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

iterators that return move-only types by value do not satisfy Readable #399

Closed
ericniebler opened this issue Apr 4, 2017 · 3 comments
Closed

Comments

@ericniebler
Copy link
Owner

ericniebler commented Apr 4, 2017

The problem is with CommonReference<reference_t<I>, value_type_t<I>&>(). Imagine an iterator I such that reference_t<I> is std::unique_ptr<int>. The CommonReference first computes the common reference type to be std::unique_ptr<int>, then it tests that std::unique_ptr<int>& is ConvertibleTo std::unique_ptr<int>, which is of course false.

The fix is to instead be testing CommonReference<reference_t<I>&&, value_type_t<I>&>(). That causes the common reference to be computed as const std::unique_ptr<int>&, and std::unique_ptr<int>& is indeed convertible to that.

Proposed Resolution

(Includes the resolution for #339, accepted by LWG in Kona but not yet moved in full committee).

Change the Readable concept [iterators.readable] as follows:

template <class I>
concept bool Readable() {
  return requires {
      typename value_type_t<I>;
      typename reference_t<I>;
      typename rvalue_reference_t<I>;
    } &&
-   CommonReference<reference_t<I>, value_type_t<I>&>() &&
-   CommonReference<reference_t<I>, rvalue_reference_t<I>>() &&
-   CommonReference<rvalue_reference_t<I>, const value_type_t<I>&>();
+   CommonReference<reference_t<I>&&, value_type_t<I>&>() &&
+   CommonReference<reference_t<I>&&, rvalue_reference_t<I>&&>() &&
+   CommonReference<rvalue_reference_t<I>&&, const value_type_t<I>&>();
}
@ericniebler
Copy link
Owner Author

@CaseyCarter comments?

@CaseyCarter
Copy link
Collaborator

(A "this wording is relative to" would be good - looks like N4651 to me, but reviewers won't know.)

This seems plausible. I'm concerned that I haven't yet developed an intuition for CommonReference: it seems like I should be able to describe why the old requirement was wrong, and why the new requirement is right, in a sentence. The fact that I cannot - when we're supposedly the experts on this thing - makes me concerned that CommonReference needs "sharpening."

@ericniebler
Copy link
Owner Author

The way I think of it is that:

  1. common_reference is the least-qualified reference(-ish) type to which all arguments bind, and
  2. If any of the arguments are not references, the result of common_reference is also not a reference.

I could imagine treating non-reference types as if they were rvalue reference types. Then, common_reference_t<int, int> would be int&& since an expression e such that decltype(e) is int can bind to an int&&. That wouldn't mean that common_reference always evaluates to a reference, though. common_reference_t<int, short> would still be int.

I considered this resolution, but treating common_reference_t<Ts...> as if the user had written common_reference_t<Ts&&...> seems lossy. If that's what the user wants, he/she can type that.

@CaseyCarter CaseyCarter added ready and removed new labels Jul 10, 2017
CaseyCarter added a commit that referenced this issue Jul 18, 2017
CaseyCarter added a commit that referenced this issue Jul 18, 2017
CaseyCarter added a commit that referenced this issue Jul 18, 2017
CaseyCarter added a commit that referenced this issue Jul 18, 2017
CaseyCarter added a commit that referenced this issue Jul 18, 2017
CaseyCarter added a commit that referenced this issue Jul 18, 2017
CaseyCarter added a commit that referenced this issue Jul 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants