Disallow `a, b = val` in routine arguments & maybe more

[metagn]

This code:

proc foo(a, b = 1) = echo (a, b)
foo()

Currently compiles, and outputs:

(1, 1)

This is counterintuitive and can undesirably affect runtime behavior. From what I've seen this syntax isn't used often either, probably because it's unintuitive.

So I think it's fine to error in situations like this. To be specific, when = val can be applied to multiple arguments in a routine declaration (as is done currently), an error should be given.

Similar syntax can be used in var/let sections as well, but this seems to have somewhat common use, so I am not sure about what should be done there. Perhaps it should be disallowed with pragmas in those situations:

var a {.global.}, b = foo() # disallowed regardless of the pragma used

If default values for object fields are allowed (don't have link to RFC on hand) then this syntax should probably be disallowed as well.

[ghost]

Just for information (on possibly opening issues if this gets approved):

List of Nimble packages that seem to use this feature in their code

https://github.com/krisppurg/dimscord/blob/master/dimscord/constants.nim#L546 endpointGuildVoiceStatesUser, endpointGuildTemplates https://github.com/nepeckman/nerve-rpc/blob/master/tests/services/main.nim#L21 proc add, also tests/utests/tRouter proc sub and other ones https://github.com/OpenSystemsLab/q.nim/blob/master/q.nim#L76 (also line 37) https://github.com/liquidev/rapid/blob/master/src/rapid/physics/chipmunk.nim#L422 https://github.com/onelivesleft/simple_parseopt/blob/master/src/simple_parseopt.nim#L163 https://github.com/stisa/snail/blob/master/snail/graphic.nim#L80 https://github.com/mratsim/Arraymancer/blob/master/arraymancer.nimble#L142 https://github.com/h3rald/fae/blob/master/fae.nimble#L41 (also litestore) https://github.com/enthus1ast/nimja/blob/master/tests/basic/test_proc.nim#L65 https://github.com/hamidb80/packedArgs/blob/main/tests/test.nim#L4 https://github.com/ba0f3/xml.nim/blob/master/src/xml/selector.nim#L29 https://github.com/nimterop/nimterop/blob/master/nimterop/build/tools.nim#L91 (also `newConanPackage`) https://github.com/jlp765/seqmath/blob/master/src/seqmath/util.nim#L24 https://github.com/bung87/scorper/blob/devel/src/scorper/http/cookies.nim#L9 This is just from a quick regex search over all nimble package repos, of course I might've missed some. Doesn't look like it's widely used anywhere, mostly just tests or small procs that can be easily patched. For reference - the regex was run in VSCode's serach (which uses ripgrep), the regexes (I don't know them well) ```re proc \S*\(([^=:\s]*)\s*,\s*([^\s]*)\s*=\s*([^\s]*)\s* proc \S*\(([^=:\s]*)\s*,\s*([^\s]*)\s*,\s*([^\s]*)\s*=\s*([^\s]*)\s* ```

[metagn]

Some of those are probably even unintentional (dimscord, fae, seqmath, scorper). My guess is some of them are copy pasted function signatures from Python, and the type annotation for the previous parameters are forgotten because they compile.

[zedeus]

This seems consistent with the difference between , and ; in signatures.

template foo(a, b: int)
template foo(a; b: int)

In the second case, a is untyped since , denotes a list of arguments potentially of a single type. So, I would also expect a, b = 2 or a, b: int = 2 to assign the default value to both arguments.

[mratsim]

Trick question, what is printed in C

#include <stdio.h>

int main() {
  int a, b = 2;

  printf("a = %d\n", a);
  printf("b = %d\n", b);
}

I personally always have to stop and think or test "uh, does that set both or not to that value" every time I read this.

Now in Nim, in function signatures, since you're forced to declare types unless there is a default value, it does sound reasonable and consistent to allow that. Furthermore, if there was an issue with type/default value, it would be a compile-time error.

Regarding var/let I would agree, besides the C int a, b = 2;, there is also Python a, b = b, a and the convenience for the writer doesn't trump the readability and maintenance and clarity for the readers, code reviewers, collaborators

[metagn]

you're forced to declare types unless there is a default value

This is true and there are more reasons why this will rarely cause issues. The way I see it though, this adds extra possibilities of human error for benefit that isn't there, similar to what you said. I can definitely understand nitpicking harmless stuff can get out of hand (style insensitivity) but sometimes acting in advance is helpful.

[Araq]

I think making the compiler warn about it in order to phase it out would work well enough.

[demotomohiro]

So proc foo(a, b = 1) = echo (a, b) should be written as proc foo(a = 1, b = 1) = echo (a, b), and there is no alternative way to write the one default value for multiple arguments at same time because there is not much use case? I think writing default value with type like proc foo(a, b: int(=1)) = echo (a, b) looks more intuitive. But regardless of the syntax, writing the one default value for multiple arguments is counterintuitive or bad practice?

[metagn]

It's just more confusing than it is useful, I wouldn't call it bad practice necessarily.

On that note, thinking about it again, it might be more of an effort to remove than to keep this. And it does technically have unique behavior in that it compiles (but not runs) the given expression only once.

proc foo(x, y = (; static: echo "abc"; 1)) = discard

foo()
foo()
# only one "abc" in compile logs

Maybe this could be made a style check instead like in #486. The feature still exists, but using it is bad style. Stuff like var a {.global.}, b = val should still be an error though.

[Zectbumo]

Trick question, what is echoed in nim?

var x = 0
proc foo(): int =
  x.inc
  x

var a, b = foo()

echo a
echo b

[metagn]

It might be misleading but its behavior is consistent, it's basically the same as var a = foo(); var b = foo() (but not really because foo() is only compiled once as said above). Maybe the whole feature should be a style check.

[Varriount]

I agree that this behavior should be kept, but with a warning emitted by the compiler. I think my secondary concern is that, as things currently stand, the compiler is far too verbose by default - warnings tend to get drowned out by additional information output by the compiler.