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

[ranges] Remove unnecessary copy-list-initialization LWG3796 #5911

Closed
wants to merge 1 commit into from

Conversation

hewillk
Copy link
Contributor

@hewillk hewillk commented Oct 20, 2022

The two should be equivalent. This is consistent with the rest of <ranges>.

@@ -3411,7 +3411,7 @@
// \ref{range.repeat.iterator}, class \tcode{repeat_view::\exposid{iterator}}
struct @\exposidnc{iterator}@; // \expos

@\exposidnc{movable-box}@<W> @\exposid{value_}@ = W(); // \expos, see \ref{range.move.wrap}
@\exposidnc{movable-box}@<W> @\exposid{value_}@; // \expos, see \ref{range.move.wrap}
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't copy-list-initialization, there's no {} here - it's plain "copy-initialization". The proposed change isn't editorial, it's normative since it causes the default constructor to default-initialize value_ instead of value-initializing it. These comments apply to line 13304 as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the correction. I'm going to submit an LWG since it actually caused some issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

it causes the default constructor to default-initialize value_ instead of value-initializing it

I believe default-initialization and value-initialization are equivalent for movable-box. Its default ctor, whenever provided, always value-initializes the contained value.

However, the propose change seems non-editorial to me. The current wording does require move construction in these default ctors (as specified via optional's corresponding ctor), which is probably wrong, and perhaps an LWG issue is needed to correct them.

Copy link
Member

Choose a reason for hiding this comment

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

@CaseyCarter , "it causes the default constructor to default-initialize value_ instead of value-initializing it."

This is not quite accurate. We're initializing a movable-box<W> with a value-initialized value of type W, which may or may not be equivalent to value-initializing the movable-box<W> itself (this issue claims it isn't equivalent, and actually harmful).

@jensmaurer
Copy link
Member

Please leave a comment with the LWG issue number once you have it.

@hewillk
Copy link
Contributor Author

hewillk commented Oct 22, 2022

Please leave a comment with the LWG issue number once you have it.

LWG3796.

@jensmaurer jensmaurer changed the title [ranges] Remove unnecessary copy-list-initialization [ranges] Remove unnecessary copy-list-initialization LWG3796 Oct 22, 2022
@tkoeppe tkoeppe added the lwg Issue must be reviewed by LWG. label Nov 8, 2022
@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 8, 2022

Can we close this editorial issue now that this is tracked elsewhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lwg Issue must be reviewed by LWG.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants