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

DE282 24.04.4 Make enable_view less greedy LWG 3326 #278

Closed
wg21bot opened this issue Oct 24, 2019 · 9 comments · Fixed by cplusplus/draft#3748
Closed

DE282 24.04.4 Make enable_view less greedy LWG 3326 #278

wg21bot opened this issue Oct 24, 2019 · 9 comments · Fixed by cplusplus/draft#3748
Labels
accepted LWG Library ranges std::ranges
Milestone

Comments

@wg21bot
Copy link
Collaborator

wg21bot commented Oct 24, 2019

Since the difference between range and view is largely semantic, the
two are differentiated with the help of enable_view." (§3)

enable_view is designed as on opt-in trait to specify that a type is a
view. It defaults to true for types derived from view_base (§4.2) which
is clearly a form of opt-in. But it also employs a heuristic assuming
that anything with iterator == const_iterator is also view (§4.3).

This
is a very poor heuristic, the same paragraph already needs to define six
exceptions from this rule for standard library types (§4.2).

Experience in working with range-v3 has revealed multiple of our own
library types as being affected from needing to opt-out from the
"auto-opt-in", as well. This is counter-intuitive: something that was
never designed to be a view shouldn't go through hoops so that it isn't
treated as a view.

Proposed change:
Make enable_view truly be opt-in by relying only on explicit
specialisation or inheritance from view_base. This means removing 24.4.4
§4.2 - §4.4 and introducing new §4.2 "Otherwise, false".

Double-check if existing standard library types like basic_string_view
and span need to opt-in to being a view now.

@wg21bot wg21bot added the LEWG Library Evolution label Oct 24, 2019
@wg21bot wg21bot changed the title DE282 24.04.4 DE282 24.04.4 Make enable_view less greedy Oct 24, 2019
@jensmaurer jensmaurer added the ranges std::ranges label Oct 25, 2019
@h-2
Copy link

h-2 commented Nov 5, 2019

Wording change:

  • Remove 24.4.4 §4.2 - §4.4 and introduce new §4.2 "Otherwise, false".
  • In §21.4, specialise enable_view for string_view to be true.
  • In §22.7, specialise enable_view for span to be true.
  • In §24.7.11.4, for split_­view​::​outer_­iterator​::​value_­type, specialise enable_view to be true unless it is changed to inherit view_base directly or indirectly.
  • All other views in the ranges library derive from view_interface which inherits from view_base and are thus covered already.

@CaseyCarter do you agree with this?

@tituswinters
Copy link

LEWG in Belfast: Approve this NB comment suggestion. Forward to LWG for C++20, pending detailed wording. (Which appears to have been provided by @h-2 earlier this evening).

@tituswinters tituswinters added LWG Library and removed LEWG Library Evolution labels Nov 5, 2019
@CaseyCarter
Copy link

CaseyCarter commented Nov 6, 2019

Some wording clarifications:

  • We can simplify enable_view a bit more since "If [condition] true; Otherwise false" is equivalent to "[condition]".
  • We need to cover all specializations of basic_string_view in addition to string_view.
  • "specialize [meow] to true for [woof] in <header>" could be made more precise.
  • LWG 3276 fixes split_view::outer_iterator::value_type. It will be moved in plenary this week with the other "Ready" LWG issues, we simply need to make it clear that the correctness of this change depends on that issue's P/R.

Technical Specifications

Change [range.view] as follows:

 template<class T>
-  inline constexpr bool enable_view = see below;
+  inline constexpr bool enable_view = derived_from<T, view_base>;

-4 Remarks: For a type T, the default value of enable_view<T> is:
-(4.1) - If derived_from<T, view_base> is true, true.
-(4.2) - Otherwise, if T is a specialization of class template
-        initializer_list ([support.initlist]), set ([set]), multiset ([multiset]),
-        unordered_set ([unord.set]), unordered_multiset ([unord.multiset]), or
-        match_results (30.10), false.
-(4.3) - Otherwise, if both T and const T model range and range_reference_t<T> is not the same
-        type as range_reference_t<const T>, false . [Note: Deep const-ness implies element
-        ownership, whereas shallow const-ness implies reference semantics. —end note]
-(4.4) - Otherwise, true.

Change [string.view.synop] as follows:

 template<class charT, class traits = char_traits<charT>>
   class basic_string_view;

+template<class charT, class traits>
+  inline constexpr bool ranges::enable_view<basic_string_view<charT, traits>> = true;

 // 21.4.3, non-member comparison functions
 template<class charT, class traits>
   constexpr bool operator==(basic_string_view<charT, traits> x,
                             basic_string_view<charT, traits> y) noexcept;

Change [span.syn] as follows:

 // 22.7.3, class template span
 template<class ElementType, size_t Extent = dynamic_extent>
   class span;

+template<class ElementType, size_t Extent>
+  inline constexpr bool ranges::enable_view<span<ElementType, Extent>> = Extent == 0 || Extent == dynamic_extent;

 // 22.7.3.7, views of object representation
 template<class ElementType, size_t Extent>
   span<const byte, Extent == dynamic_extent ? dynamic_extent : sizeof(ElementType) * Extent>
     as_bytes(span<ElementType, Extent> s) noexcept;

Apply the Proposed Resolution of LWG 3276.

@JeffGarland
Copy link
Member

Belfast LWG small group - recommend a new LWG issue to resolve. There's a missing bracket on string_view in the wording above.

@CaseyCarter
Copy link

There's a missing bracket on string_view in the wording above.

Edited in place to correct. Also, since two different people have asked: span<T, N> is default constructible - and therefore can model view - only when N == 0 or N == dynamic_extent. Consequently I constrained the partial specialization of enable_view to be consistent with the rules the working draft applies to program-defined specializations of enable_view. We don't really have to follow those rules, but it seems to provide a better example for users if we do.

@Dani-Hub
Copy link
Member

Dani-Hub commented Nov 6, 2019

The associated issue is LWG 3326

@JeffGarland
Copy link
Member

Issue moved to ready during Wed night issue processing in Belfast.

@jensmaurer jensmaurer changed the title DE282 24.04.4 Make enable_view less greedy DE282 24.04.4 Make enable_view less greedy LWG 3326 Nov 7, 2019
@JeffGarland
Copy link
Member

Will be in the issues to be approved in Prague.

@jensmaurer
Copy link
Member

Accepted with modification. See library issue 3326.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted LWG Library ranges std::ranges
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants