[EXPERIMENT] trailing comma in argument list should be an error

[WalterBright]

@ibuclaw discovered that trailing commas in argument lists are allowed. This was never intended. This PR is an experiment so see if code depends on it.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#12684"

[ibuclaw]

First off, deprecation. Secondly, is there a good rationale for rejecting it after almost two decades?

I assume that this affects everything from function arguments to array initializers?

[CyberShadow]

@ibuclaw discovered

He did?

This PR is an experiment so see if code depends on it.

To save you some time, here are some more:

• In std.traits
• In std.experimental.allocator.building_blocks.stats_collector (stdx-allocator inherited this, which is used by Vibe.d)
• In DMD itself
• In Vibe.d

[ibuclaw]

@ibuclaw discovered

He did?

In my naivity, I mentally inserted a comma. :-)

[WalterBright]

dmd/src/dmd/cli.d

Line 190 in 3dda6c6

)",

I'm afraid I'm not seeing the compelling advantage of:

      "),

over:

    ")

[Geod24]

Please no. You might not find a use for it, but many people do.
Namely, it makes string-based code generation easier, and it's more git friendly as you don't have to modify an unrelated line when you add an entry to your array initializer / enum / whatnot.

Plus, what would this deprecation achieve ? What's the issue with them ?

[WalterBright]

but many people do

This seems highly unlikely. Looks to me like most uses are rare and accidental.

it makes string-based code generation easier,

That's true. But not that much easier. hdrgen.d deals with commas-as-separators all over it, and it's trivial:

https://github.com/dlang/dmd/blob/master/src/dmd/hdrgen.d#L3158

and it's more git friendly as you don't have to modify an unrelated line

See #12684 (comment)

What's the issue with them ?

Good taste. Does:

case 3,:

look like anything but crap? Code should be crafted to be pleasing to the eye. It's not a technical issue, it's a human factors issue. When I see stuff like that, it looks like someone just dented my car.

[WalterBright]

is there a good rationale for rejecting it after almost two decades?

We may be stuck with it. But that's no rationale to extend it to case statements.

[Geod24]

This seems highly unlikely. Looks to me like most uses are rare and accidental.

No, they are not. If you make that claim, please back it up. Here's what I got when I tried to compile Phobos (the error triggered in druntime, but it was while compiling Phobos):

src/rt/tracegc.d-mixin-91(132): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(175): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(218): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(261): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(304): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(347): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(390): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(433): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(476): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(519): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(562): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(605): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(648): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(691): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(734): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(777): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(820): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(863): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(906): Error: trailing comma in argument list
src/rt/tracegc.d-mixin-91(949): Error: trailing comma in argument list

Because of this:
https://github.com/dlang/druntime/blob/751f5665a31ea0f150d71b22a9852abacc2f61ec/src/rt/tracegc.d#L148-L165

So I made it a deprecation, in order to be able to build and test your claims. I got two more uses:

std/traits.d(807): Deprecation: trailing comma in argument list
std/experimental/allocator/building_blocks/stats_collector.d(295): Deprecation: trailing comma in argument list

https://github.com/dlang/phobos/blob/0b7bf8dc1fe0b035e0f3f039bdd1e365d11223bc/std/traits.d#L800-L807
https://github.com/dlang/phobos/blob/0b7bf8dc1fe0b035e0f3f039bdd1e365d11223bc/std/experimental/allocator/building_blocks/stats_collector.d#L273-L295

Then I went on to compile our code, and it triggered cases in Vibe.d:

submodules/vibe-core/source/vibe/internal/interfaceproxy.d-mixin-201(201,71): Deprecation: trailing comma in argument list
submodules/vibe-core/source/vibe/internal/interfaceproxy.d-mixin-201(201,73): Deprecation: trailing comma in argument list
submodules/vibe-core/source/vibe/internal/interfaceproxy.d-mixin-201(201,79): Deprecation: trailing comma in argument list
submodules/vibe-core/source/vibe/internal/interfaceproxy.d-mixin-201(201,83): Deprecation: trailing comma in argument list
submodules/vibe-core/source/vibe/internal/interfaceproxy.d-mixin-201(201,91): Deprecation: trailing comma in argument list
submodules/vibe-core/source/vibe/internal/interfaceproxy.d-mixin-201(201,113): Deprecation: trailing comma in argument list
submodules/vibe.d/http/vibe/http/client.d(87,2): Deprecation: trailing comma in argument list

And in our code:

source/agora/common/Config.d(317,9): Deprecation: trailing comma in argument list
source/agora/consensus/state/ValidatorSet.d(350,21): Deprecation: trailing comma in argument list
source/agora/consensus/state/ValidatorSet.d(351,17): Deprecation: trailing comma in argument list
source/agora/flash/UpdateSigner.d(437,83): Deprecation: trailing comma in argument list
source/agora/node/FullNode.d(303,9): Deprecation: trailing comma in argument list
source/agora/node/admin/AdminInterface.d(192,9): Deprecation: trailing comma in argument list

https://github.com/bosagora/agora/blob/490b18e32e87e39de68094de549f2e93d5fa04d0/source/agora/common/Config.d#L290-L317
https://github.com/bosagora/agora/blob/490b18e32e87e39de68094de549f2e93d5fa04d0/source/agora/consensus/state/ValidatorSet.d#L343-L351
https://github.com/bosagora/agora/blob/490b18e32e87e39de68094de549f2e93d5fa04d0/source/agora/node/FullNode.d#L295-L303
https://github.com/bosagora/agora/blob/490b18e32e87e39de68094de549f2e93d5fa04d0/source/agora/node/admin/AdminInterface.d#L187-L192

There are two patterns that emerge from this:

• People use trailing comma for statements that spawns multiple lines;
• People use trailing comma for code generation;

That's true. But not that much easier. hdrgen.d deals with commas-as-separators all over it, and it's trivial:

The fact you can do it differently with "a bit more work" doesn't negate the point.

See #12684 (comment)

My point was about being more "git friendly". That's not a matter of style or aesthetics. Without trailing commas, you have to change surrounding lines when moving an entry / adding a new one. With trailing commas you don't. There's nothing subjective about this.

Does [...] look like anything but crap? Code should be crafted to be pleasing to the eye. It's not a technical issue, it's a human factors issue. When I see stuff like that, it looks like someone just dented my car.

This makes absolutely no sense. Since when is it acceptable to deprecate a tiny but convenient feature used by many projects out there because you suddenly don't like how it looks ? If I want my code to be formatted according to someone else's questionable taste, I'd be using Golang.

[thewilsonator]

Looks to me like most uses are rare and accidental.

They would be very useful if C++ had them for LDC as one example, what with LLVM interfaces changing frequently. See also Mathis' post above.

But that's no rationale to extend it to case statements.

Then you should open a reversion PR on that PR. Closing this.

[WalterBright]

Since when is it acceptable to deprecate a tiny but convenient feature used by many projects out there because you suddenly don't like how it looks ?

The case 1,; is a new feature. #12677

suddenly

I never did. I made a mistake implementing the parser.

[Geod24]

No you did not, you willingly made this change.

[WalterBright]

No you did not, you willingly made this change.

So I did 10 years ago. Mea Culpa.

[Geod24]

So I did 10 years ago. Mea Culpa.

I think you made the right call back then. I don't remember anyone complaining about trailing commas, ever, before today.
If anything, people have asked to extend the ability to use them (this one is "only" 5 years ago).

[CyberShadow]

No you did not, you willingly made this change.

And here is the spec change which introduced it in the grammar:
dlang/dlang.org@5b47199

[CyberShadow]

This makes absolutely no sense. Since when is it acceptable to deprecate a tiny but convenient feature used by many projects out there because you suddenly don't like how it looks ?

For clarity, this pull request was an experiment to check the CI fallout - see #12677 (comment) . @WalterBright's initiative to remove it from the language was based on the hypothesis that this feature isn't actually used by anyone (which has now been debunked). I will update the title accordingly to avoid further confusion.

[MoonlightSentinel]

The only argument against this feature is style and "it's ugly". But the right place to enforce style guidelines is dscanner, not dmd.