Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

dwarf.d: remove spurious trailing comma#3498

Closed
WalterBright wants to merge 1 commit intodlang:masterfrom
WalterBright:trailingComma
Closed

dwarf.d: remove spurious trailing comma#3498
WalterBright wants to merge 1 commit intodlang:masterfrom
WalterBright:trailingComma

Conversation

@WalterBright
Copy link
Member

Bad style exploiting what appears to be a bug in the D front end.

@WalterBright WalterBright added the Refactoring No semantic changes label Jun 14, 2021
@dlang-bot
Copy link
Contributor

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 + druntime#3498"

@WalterBright
Copy link
Member Author

std/stdio.d(299): Error: undefined identifier `flockfile` in module `core.sys.posix.stdio`

What the heck? That has nothing to do with this trivial PR.

@wilzbach
Copy link
Contributor

It's not a bug, it's a feature.

@WalterBright
Copy link
Member Author

If it's used exactly once, it's not much of a feature.

@CyberShadow
Copy link
Member

More uses here: dlang/dmd#12684 (comment)

@thewilsonator
Copy link
Contributor

This might be bad style for just two arguments, but as noted this is a feature not a bug.
It has the same use as trailing comma for enum declarations.

@thewilsonator
Copy link
Contributor

What the heck? That has nothing to do with this trivial PR.

How many commits between trailingComma~1 and dlang/master? Maybe try rebasing?

Copy link
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an oversight, very much on purpose, as there is another usage of it in the commit that introduced it:
f653441#diff-1b94c982760aac40515b2b539895bf36e83f45159711f983f09290277a996587R91-R96

And we established it's not a bug, but a feature introduced 10 years ago as a fix to issue 2477.

@DoctorNoobingstoneIPresume
Copy link

DoctorNoobingstoneIPresume commented Jun 21, 2021

FWMOIW (For Whatever My Opinion Is Worth):

This feature makes it easier to write some code-generating (mixin) code.

Also: If code is indented like this:

f
(
    very-long-arg-1,
    not-long-but-anyway-similarly-indented-arg-2,
    very-long-arg-3,
);

then that trailing comma ensures we can add very-long-arg-4 without affecting the (git blame etc.) history of the very-long-arg-3 line.

I love this bug^H^H^Hfeature.

Therefore, I vote against removing code which (even accidentally) exploits the feature. Otherwise, others (including @WalterBright) might remove the ahem bug and actually get away with it !

@Geod24
Copy link
Member

Geod24 commented Jun 21, 2021

The DMD PR has been closed, so closing this too.

@Geod24 Geod24 closed this Jun 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Refactoring No semantic changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants