Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disallow empty struct-declaration-list #12621

Merged
merged 1 commit into from
Jun 2, 2021
Merged

Conversation

WalterBright
Copy link
Member

@thewilsonator got this right. I misread the C11 spec, conflating struct-declarator-list with struct-declaration-list

@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 + dmd#12621"

Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

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

this really ought to have a fail compilation test.

@WalterBright
Copy link
Member Author

It's best to defer fail_compilation tests for now. As the implementation is in flux, these tend to be big maintenance problems, enough to provide an incentive not to do the right thing in the code. Much better when the disruptive changes settle down.

On the other hand, we know what must compile successfully, and that isn't going to change, so writing those tests is appropriate now.

@dlang-bot dlang-bot merged commit 8949aac into dlang:master Jun 2, 2021
@ibuclaw
Copy link
Member

ibuclaw commented Jun 3, 2021

It is accepted as a GNU/Clang extension though (we have support code in D to handle empty C++ structs), but I guess we'll wait and see if it shows up in the wild.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Easy Review ImportC Pertaining to ImportC support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants