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

[spec/struct] Add 'Union Literals' & 'Anonymous Structs & Unions' #3355

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

ntrel
Copy link
Contributor

@ntrel ntrel commented Jul 25, 2022

The docs for a struct literal with a union field apply for non-anonymous unions too.
Fix: Non-overlapping union field data is not default initialized, it is
zeroed.
Fixes Issues 21188, 18855.

@dlang-bot
Copy link
Contributor

dlang-bot commented Jul 25, 2022

Thanks for your pull request and interest in making D better, @ntrel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
18855 minor Behavior of Anonymous Union is Undocumented
21188 normal Anonymous structs - not described

Copy link
Contributor

@dkorpel dkorpel left a comment

Choose a reason for hiding this comment

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

I think the text could be more precise in what an anonymous struct/union does instead of what it's used for. It doesn't cover arbitrarily complex / redundant usages: struct S { struct { struct { union { struct { int x; }}}}} Still, it's a good start.

@PetarKirov
Copy link
Member

@ntrel please update the commit message, so that all of the issues will be automatically closed by @dlang-bot. See: https://github.com/dlang/dlang-bot#referencing-multiple-issues

@RazvanN7
Copy link
Contributor

RazvanN7 commented Jul 27, 2022

Hmm, the commit message does contain both issues, I wonder why the bot doesn't catch that. @ntrel the quick fix would be to just break your commit into 2, each fixing one of the issues.

@PetarKirov
Copy link
Member

I think the commit message should start with the issues that are fixed. Try this:

Fix issues 18855 and 21188 - [spec/struct] Add 'Union Literals' & 'Anonymous Structs & Unions' 

Fix Issue 21188 - Anonymous structs - not described.
Fix Issue 18855 - Behavior of Anonymous Union is Undocumented.

Fix: Non-overlapping union field data is not default initialized, it is
zeroed.

See: https://regex101.com/r/C7q7eT/1

…s & Unions'

Fix Issue 21188 - Anonymous structs - not described.
Fix Issue 18855 - Behavior of Anonymous Union is Undocumented.

Fix: Non-overlapping union field data is not default initialized, it is
zeroed.
@ntrel
Copy link
Contributor Author

ntrel commented Jul 27, 2022

@PetarKirov @RazvanN7 Updated commit message as suggested. No changes to diff.

@ntrel
Copy link
Contributor Author

ntrel commented Jul 27, 2022

@PetarKirov Do you know which file in dlang-bot has the regex to detect fixed issues? I'd like to have a look.

@PetarKirov
Copy link
Member

Do you know which file in dlang-bot has the regex to detect fixed issues? I'd like to have a look.

I think this is what you're looking for: https://github.com/dlang/dlang-bot/blob/750bb49423033c0bc52d2515fde2b29238668265/source/dlangbot/bugzilla.d#L21

@RazvanN7 RazvanN7 merged commit ef3f740 into dlang:master Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants