Skip to content

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

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions source/ranges.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@CaseyCarter CaseyCarter Oct 20, 2022 •
edited
Loading

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

@frederick-vs-ja frederick-vs-ja Oct 20, 2022 •
edited
Loading

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

@jensmaurer jensmaurer Oct 21, 2022 •
edited
Loading

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).

Bound @\exposid{bound_}@ = Bound(); // \expos

public:
Expand Down Expand Up @@ -13301,7 +13301,7 @@
requires @\libconcept{view}@<V> && is_object_v<Pred>
class chunk_by_view : public view_interface<chunk_by_view<V, Pred>> {
V @\exposid{base_}@ = V(); // \expos
@\exposidnc{movable-box}@<Pred> @\exposid{pred_}@ = Pred(); // \expos
@\exposidnc{movable-box}@<Pred> @\exposid{pred_}@; // \expos

// \ref{range.chunk.by.iter}, class \tcode{chunk_by_view::\exposid{iterator}}
class @\exposidnc{iterator}@; // \expos
Expand Down