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

undefined boolean note clarification #2542

Merged
merged 2 commits into from Dec 7, 2018
Merged

Conversation

jfbastien
Copy link
Contributor

Addresses #2483.

@jensmaurer
Copy link
Member

jensmaurer commented Nov 30, 2018

The commit comment should have a [section.label] prefix.

Other than that, I'm not sure what this footnote delivers us in the first place: Reading uninitialized data is undefined behavior, and attempting to list the shades and variations of undefined behavior leads nowhere at best and is seriously misleading otherwise. So, let's remove that footnote altogether.

See [dcl.init] p12 for the detailed treatise. I'd actually be in favor of moving most of this paragraph into [expr.prop] as a new subclause "Indeterminate values". Except that an indeterminate value is not a compile-time property (like the other properties mentioned there), but a runtime property. @zygoloid, what do you think?

@zygoloid
Copy link
Member

zygoloid commented Dec 7, 2018

I think this change makes matters a little worse, since it makes this note sound even more like it's describing what will actually happen. I think this note is problematic for other reasons, as (by omission) it suggests that the same might not be true for other types. (Eg, reading an uninitialized int is undefined even if there's a one-to-one correspondence between object representations and value representations.)

I'm in favor of deleting this footnote. Moving the description of uninitialized values into its own subclause seems very reasonable to me, but it's not a property of an expression, so I don't think [expr.prop] is an ideal home. Maybe [intro.object], perhaps just after [basic.life], would be better? I think we should move all but the first sentence of [dcl.init]p12 to the new subclause, and add a paragraph break after the first note.

@jfbastien
Copy link
Contributor Author

The note is now gone. 👋

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

Looks good. @tkoeppe, do you feel like applying after @zygoloid's comments above?
(The commit message needs fixing, though.)

@tkoeppe tkoeppe merged commit a5603f0 into cplusplus:master Dec 7, 2018
@jfbastien jfbastien deleted the patch-4 branch December 7, 2018 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants