Nondeterminism with conflicting definitions

[anordal]

If I give Gersemi conflicting definitions of a function, Gersemi (0.9.2) formats the calls to it nondeterministically, in accordance with either one or the other definition.

That's of course an invalid configuration, but I suppose as a feature request, it would be useful if Gersemi could detect this, blame the user and fail in such a case. This because not every CMake contributor is going to be aware of this pitfall (speaking firstly about myself here).

Example:

# cmake/poem_parsearg_edition.cmake
function(poem)
    cmake_parse_arguments("ARG" "" "" "LINES" ${ARGN})
    message(FATAL_ERROR ${ARG_LINES})
endfunction()

# cmake/poem_straightarg_edition.cmake
function(poem)
    message(FATAL_ERROR ${ARGN})
endfunction()

# CMakeLists.txt
poem(LINES "foo" "bar")

gersemi --list-expansion=favour-expansion --definitions cmake/ --diff CMakeLists.txt

Result 1:

-poem(LINES "foo" "bar")
+poem(
+    LINES
+    "foo"
+    "bar"
+)

Result 2:

-poem(LINES "foo" "bar")
+poem(
+    LINES
+        "foo"
+        "bar"
+)

---

Thank you for writing Gersemi! I could write long and wide about properties of good and bad formatters, but for one thing: Predictability. It was always a surprise where cmake-format would break the line, but Gersemi's format is very consistent and easy to follow, which I think is praiseworthy and underrated.

[BlankSpruce]

Nice idea. I'll make it both deterministic and helpful. Let me know if output like that on stderr would be enough:

Warning: conflicting definitions for 'foo':
(used)    /tmp/tmp55zzobdn/conflicting_definitions/foo1.cmake:1:10
(ignored) /tmp/tmp55zzobdn/conflicting_definitions/foo2.cmake:1:10
(ignored) /tmp/tmp55zzobdn/conflicting_definitions/foo2.cmake:5:10
(ignored) /tmp/tmp55zzobdn/conflicting_definitions/foo2.cmake:9:10

[anordal]

Cool! Yes, if you can make it deterministic, that essentially solves the problem. Then, it becomes a question of helpfulness and usefulness whether you choose to fail, warn or allow.

I would think that a warning is bit of a weird middleground between a feature and a failure, though: Although helpful and perhaps the safest default, it's not where a user wants to be. If you make it a failure, you aren't breaking anything that really works now. But making it allowed could potentially be more useful if the user can control the precedence of definitions by let's say the order. Then, your diagnostic above could be something like the --verbose output.

[BlankSpruce]

Definitely I don't intend to have it as a feature that can be controlled unless good example where it matters is shown. I will consider whether it should be failure or just warning. My first thought about how it would occur in the wild is something along the lines "user just wanted to have nice formatting, they didn't do anything silly on purpose" which would convince me towards warning.

[BlankSpruce]

0.9.3 has it fixed. I've chosen the warning solution since I intend for this tool to be friendly to use even if user might have done something slightly out of bounds of expected usage patterns. The behaviour will be now deterministic in a way that given following sorted order of definitions:

(used)    /tmp/tmp55zzobdn/conflicting_definitions/foo1.cmake:1:10
(ignored) /tmp/tmp55zzobdn/conflicting_definitions/foo2.cmake:1:10
(ignored) /tmp/tmp55zzobdn/conflicting_definitions/foo2.cmake:5:10
(ignored) /tmp/tmp55zzobdn/conflicting_definitions/foo2.cmake:9:10

the first one is chosen because of alphabetic order of path:line:column.

[anordal]

Thanks, this fixes the most important thing for me, which is to be deterministic. But it seems we meant different things with "conflicting" definitions. As such, I'm glad you didn't make it an error. I should have clarified that first:

There can be multiple definitions, but what I mean by conflicting is that they also differ in something that matters (like different signature), so that they result in different formatting. If you would say that there are legit or at least unproblematic uses, like duplicates and overloads, then I completely agree.

[BlankSpruce]

You're absolutely right that all conflicting definitions might introduce different signatures (it was even the case in your provided example). In fact there could be a case, very hostile for users of such function in my opinion, to have different definitions in if-else clauses. However I have to put the stop somewhere because while I might be able to write parser of function/macro signatures I definitely can't write one for programmer's intent. Let's hope that warning will be enough to nudge people towards some cleanups.