Fix Issue 22019 - case 1,: allowed by grammar but not DMD

[CyberShadow]

No description provided.

[dlang-bot]

Thanks for your pull request, @CyberShadow!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	22019	normal	`case 1,:` allowed by grammar but not DMD

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#12677"

[MoonlightSentinel]

Is there a test that verifies the error messages for case ,: and case :?

[CyberShadow]

I don't think so, though I did specifically avoid using check(TOK.comma) to avoid a bad error message.

[WalterBright]

@CyberShadow why not fix the spec instead? Is there a purpose to allowing the trailing comma?

There is a purpose in C code to allow such, because of macro expansions, but why support it in D?

[CyberShadow]

@CyberShadow why not fix the spec instead?

I publicly asked for opinions on where it would be better to make the change, and got two responses, each leaning towards making the change in the compiler.

Is there a purpose to allowing the trailing comma?

Consistency.

I would imagine that the usual reasons to allow it also apply here, to a much lesser degree.

In retrospect, I could have applied the consistency goal by adapting and calling parseArguments(). That way, the grammar would match not just the compiler's behavior but also its implementation.

[WalterBright]

I publicly asked for opinions

I'm sorry I did not notice that.

Consistency.

Consistency with what? The only thing I can think of is enums, and there's a good reason for that (minimizing diffs when appending new members). Parameter lists, argument lists, etc., do not allow trailing commas.

Since I wrote most of the grammar, it allowing a trailing comma is a mistake I made, not a choice.

[CyberShadow]

Parameter lists, argument lists, etc., do not allow trailing commas.

Yes, they do. Both in the grammar and in DMD.

[dkorpel]

I once generated code for a switch statement and encountered this limitation. I was surprised, since I knew trailing comma's are allowed in enums and argument lists and liked that. I wrote it off as a small oversight and added the extra logic to my code generation, but fixing this in the compiler seems like a good idea to me.

[CyberShadow]

Since I wrote most of the grammar, it allowing a trailing comma is a mistake I made, not a choice.

There probably are many mistakes (or differences compared to DMD) in the grammar. I am currently working on a project which attempts to consume it mechanically, which should help identify at least some of them. My progress so far (on the grammar side) is here: dlang/dlang.org#3019

[WalterBright]

Yes, they do. Both in the grammar and in DMD.

You're right. Both of those are bugs.

fixing this in the compiler seems like a good idea to me.

It just looks dirty.

[CyberShadow]

Both of those are bugs.

I think you will find that many people now see the current behavior useful, and will oppose its removal.

I suggest thinking of these as unintended features going forward. :)

It just looks dirty.

If you prefer, I could submit a PR which refactors the loop into a call to parseArguments, as mentioned above.

[CyberShadow]

FWIW, here are some reasons why I personally like [the ability to write] trailing commas:

1. Code generation (e.g. using string mixins). JSON infamously disallows trailing commas, which makes it more onerous to emit it. I ran into this problem hard when I attempted to emit JSON using DDoc macros.
2. Consistency with other languages. Newer JavaScript versions allow trailing commas.
3. Consistency with statements. We require a semicolon after the last statement in a block (unlike, say, Pascal), a comma at the end of a list is in line with that.
4. With trailing commas, it is much easier to swap two items in a list when one is at the end of the list.
5. It is less work to add an item to the end of a list, because you only need to insert one line, as opposed to also having to edit the line above.
6. Git diffs are more readable for the same reason.

[WalterBright]

1. a decent reason, except for Ddoc. Ddoc isn't used to generate code.
2. don't care about other languages
3. semicolons are a terminator, commas are a separator
4. come on
5. come on
6. no, they're not. People don't put each parameter on a separate line. Or at least it's rare.

many people

I bet 0 were affected by the case thing. I haven't seen a single example in the wild of the trailing parameter comma.

[CyberShadow]

I bet 0 were affected by the case thing.

Um, there's one in this very thread.

I haven't seen a single example in the wild of the trailing parameter comma.

OK, why not make a pull request which removes it and see how many Buildkite projects break?

[CyberShadow]

Ddoc isn't used to generate code.

I think you misunderstood what I was trying to say.

don't care about other languages

1. Even if you don't, D's users do.
2. You probably should care about other languages. :)

[WalterBright]

OK, why not make a pull request which removes it and see how many Buildkite projects break?

Good idea! Want to bet a pint on it?

[CyberShadow]

Very unusual way to offer someone a free drink, but sure :)

[WalterBright]

Have to make it worthwhile!

[WalterBright]

Found a case:

dlang/druntime#3498

But as it's not in buildkite, I haven't officially lost yet. But the next pint is on me next DConf!

[WalterBright]

There are no legacy cases of this, so it needs to be reverted.

[wilzbach]

Consistency matters.

[WalterBright]

Consistency matters.

Consistency is only one of many conflicting considerations.

[RazvanN7]

Between consistency (objective metric) and beauty (subjective metric), I would rather choose consistency. @WalterBright you have not provided any other argument besides the fact that it is ugly and you did not designed it this way. However, it's been in the language for 10 years, it's implemented consistently (parameter lists, argument lists etc.) and it's being used in our own repos. Why would we go through the trouble of changing something that is obviously not problem, instead of accepting it and moving on?