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

Tagged_getter names must hide names in Base #363

Closed
CaseyCarter opened this issue Mar 4, 2017 · 11 comments
Closed

Tagged_getter names must hide names in Base #363

CaseyCarter opened this issue Mar 4, 2017 · 11 comments

Comments

@CaseyCarter
Copy link
Collaborator

CaseyCarter commented Mar 4, 2017

tagged needs to inject the tagged getter types "between" itself and Base in the inheritance hierarchy, so that tagged getter names hide (instead of ambiguate) members with the same name in Base. This is necessary so that given e.g. tagged_tuple<tag::in(T)> f(); the expression f().in() results in a reference to the tagged_tuple's member T even when T has a member function named in and tuple<T> derives from T to take advantage of the empty base optimization.

The existing complexity around tagged getters seems unwarranted since users cannot define their own, and I would rather not exacerbate the problem by introducing additional type parameters and constructor inheritance to fix this issue. The simplest solution seems to be to remove the notion of tagged getters altogether and instead specify the named accessor functions as being expression-equivalent-to a call to get with the appropriate index.

Proposed Resolution

Wording relative to N4671 + #172, also resolves #364, and #418.

-template <class Base, TagSpecifier... Tags>
-  requires sizeof...(Tags) <= tuple_size<Base>::value
+template <Destructible Base, TagSpecifier... Tags>
+  requires std::is_object<Base>::value &&
+    sizeof...(Tags) <= tuple_size<Base>::value
 struct tagged :
-  Base, TAGGET (tagged<Base, Tags...>, Ti, i)... { // see below
-  using Base::Base;
+  Base { // see below
+  using Base::Base; // see below
   tagged() = default;
   tagged(tagged&&) = default;
   tagged(const tagged&) = default;
+  tagged(Base&&) noexcept(see below)
+    requires MoveConstructible<Base>();
+  tagged(const Base&) noexcept(see below)
+    requires CopyConstructible<Base>();
   tagged &operator=(tagged&&) = default;
   tagged &operator=(const tagged&) = default;

 [...]

+-?- It is unspecified whether Base is a direct or indirect base class of tagged<Base, Tags...>.
+  tagged derives publicly and unambiguously from Base, inherits constructors (perhaps
+  indirectly) from Base, and has special member functions as-if it defines no data members and
+  Base is its direct and only base class. It shall not be possible to delete an instance of
+  class template tagged through a pointer to any base other than Base.

-3 A tagged getter is an empty trivial class type that [...]

-4 A tagged getter class with DerivedCharacteristic [...]

 5 A tag specifier is a type that facilitates a mapping from a
-  tuple-like type and an element index into a tagged getter that gives named access to the
-  element at that index.
+  human-readable name to the index of an element of a tuple-like type.
   TagSpecifier<T>() is satisfied if and only if T is a tag specifier. The tag specifiers in the
   Tags parameter pack shall be unique.
-  [Note: The mapping mechanism from tag specifier to tagged getter is unspecified.—end note ]
+  [Note: This document specifies several tag specifiers that are defined in the namespace
+  std::experimental::ranges::tag (\ref{alg.tagspec}).-end note]

-6 Let TAGGET(D, T, N) name a tagged getter type that gives named [...]

-7 It shall not be possible to delete an instance of class template tagged [...]

+-?- Let tag::NAME be any tag specifier defined in this document, and let ADL::GET be the
+  exposition-only function template defined in an exposition-only sub-namespace:
+
+  namespace ADL {
+    using std::get;
+
+    template<size_t I, class T>
+    constexpr decltype(auto) GET(T&& t)
+      noexcept(noexcept(get<N>(std::forward<T>(t))))
+   {
+      return get<N>(std::forward<T>(t));
+   }
+ }
+
+  tagged has additional public member functions - possibly inherited from unspecified base
+  classes - such that for an lvalue or rvalue t of type (possibly-const) tagged<Base, Tags...>
+  t.NAME() is expression equivalent to ADL::GET<N>(t) where N is the 0-based index in Tags... of
+  the tag specifier tag::NAME.

+-?- [Example: Given the definition tagged<std::pair<int, double>, tag::in, tag::out> t;
+  t.in() is an lvalue denoting the int element of the underlying pair; std::move(t).out() is an
+  rvalue denoting the double element.-end example]

After [taggedtup.tagged]/9, add new paragraphs:

tagged(Base &&that) noexcept(see below )
  requires MoveConstructible<Base>();

-?- Remarks: The expression in the noexcept is equivalent to:

is_nothrow_move_constructible<Base>::value

-?- Effects: Initializes Base with std::move(that).


tagged(const Base &that) noexcept(see below )
  requires CopyConstructible<Base>();

-?- Remarks: The expression in the noexcept is equivalent to:

is_nothrow_copy_constructible<Base>::value

-?- Effects: Initializes Base with that.

Change [alg.tagspec] para 1 to 3 as follows:

-1 In the following description, let X [...]
-
-2 tag::X is a tag specifier (8.5.2) such that [...]
-
-3 [Example: tag::in is a type such that TAGGET(D, tag::in, N) names a type [...]
+1 The types defined in the tag namespace above are tag specifiers (\ref{taggedtup.tagged}).
@CaseyCarter
Copy link
Collaborator Author

CaseyCarter commented Jun 9, 2017

If we envision the current design as "fan inheritance":

                Tagged
                 ^  ^
        collect--/  \---Base
        ^     ^
getter1-/ ... \-getterN

we want to change the structure to "chain inheritance":

Tagged
^
\-getter1
  ^
  \-getter2
    ^
   ...
    \-getterN
      ^
      \--Base

And chain inheritance? Why should there be hiding instead of ambiguity? Seems to me that ambiguity is an acceptable result.

Given:

template <class T, class U>
tagged_tuple<tag::in(T), tag::out(U), tag::end(int)> f() { /*...*/ }

Should f().in() always return a reference to the temporary's T member, or should it be allowed to break when T or U have a member function named in, but only on implementations that use EBO for std::tuple? (To be clear, the non-portability of the error concerns me as much as the error itself.)

@CaseyCarter CaseyCarter self-assigned this Jun 16, 2017
CaseyCarter added a commit to CaseyCarter/cmcstl2 that referenced this issue Jul 1, 2017
CaseyCarter added a commit to CaseyCarter/cmcstl2 that referenced this issue Jul 1, 2017
CaseyCarter added a commit to CaseyCarter/cmcstl2 that referenced this issue Jul 2, 2017
* Implement ericniebler/stl2#299
* Implement ericniebler/stl2#363
* Implement ericniebler/stl2#364
* Implement ericniebler/stl2#418
* Remove tuple_find that was dropped from variant ages ago
* Implement extension 'structible object concepts
* tighten up constrains in the tagged impl
@CaseyCarter
Copy link
Collaborator Author

CaseyCarter commented Jul 5, 2017

I'm considering dropping the "tagged getter" notion altogether instead of making it even more complicated. The existing complexity around tagged getters seems unwarranted since users cannot define their own, and I would rather not exacerbate the problem. Getting rid of tagged getters requires something like:

-template <class Base, TagSpecifier... Tags>
-  requires sizeof...(Tags) <= tuple_size<Base>::value
+template <Destructible Base, TagSpecifier... Tags>
+  requires std::is_object<Base>::value &&
+    sizeof...(Tags) <= tuple_size<Base>::value
 struct tagged :
-  Base, TAGGET (tagged<Base, Tags...>, Ti, i)... { // see below
-  using Base::Base;
+  Base { // see below
+  using Base::Base; // see below
   tagged() = default;
   tagged(tagged&&) = default;
   tagged(const tagged&) = default;
   tagged &operator=(tagged&&) = default;
   tagged &operator=(const tagged&) = default;

 [...]

+? It is unspecified whether Base is a direct or indirect base of tagged<Base, Tags...>. tagged
+  derives publicly and unambiguously from Base, inherits constructors (perhaps indirectly) from
+  Base, and has special member functions as-if it defines no data members and Base was its
+  direct and only base class.

-3 A tagged getter is an empty trivial class type that [...]

-4 A tagged getter class with DerivedCharacteristic [...]

 5 A tag specifier is a type that facilitates a mapping from a
-  tuple-like type and an element index into a tagged getter that gives named access to the
-  element at that index.
+  human-readable name to the index of an element of a tuple-like type.
   TagSpecifier<T>() is satisfied if and only if T is a tag specifier. The tag specifiers in the 
   Tags parameter pack shall be unique.
-  [Note: The mapping mechanism from tag specifier to tagged getter is unspecified.—end note ]
+  [Note: This document specifies several tag specifiers that are defined in the namespace
+  std::experimental::ranges::tag (\ref{alg.tagspec}).-end note]

-6 Let TAGGET(D, T, N) name a tagged getter type that gives named [...]

+? Let tag::NAME be any tag specifier defined in this document, and let ADL::GET be the
+  exposition-only function:
+
+    (see wording for ADL::GET in #364)
+
+  tagged has additional public member functions - possibly inherited from unspecified base
+  classes - such that for an lvalue or rvalue t of type (possibly-const) tagged<Base, Tags...>
+  t.NAME() is expression equivalent to ADL::GET<N>(t) where N is the 0-based index in Tags... of
+  the tag specifier tag::NAME.

+? [Example: Given the definition tagged<std::pair<int, double>, tag::in, tag::out> t{42, 3.14};
+  t.in() is an lvalue denoting the int element of the underlying pair with value 42.
+  std::move(t).out() is an rvalue denoting the double element of the underlying pair with value
+  3.14.-end example]

Thoughts before I get too far down this road?

@ericniebler
Copy link
Owner

as-if it defines no data members and Base was its direct and only base class.

s/was/were/. English's subjunctive case is not yet dead.

Thoughts before I get too far down this road?

I guess so. It's somewhat unfortunate that users can't create their own tagged getters. I suspect users are going to ask for that, and we'll have to revert to "tagged getters" before long. But for the TS, this is probably fine.

@timsong-cpp
Copy link
Contributor

s/was/were/. English's subjunctive case is not yet dead.

This is a weird one. Do we really want to imply that half of the condition is counterfactual ("were") and half isn't ("defines")?

The tag specifiers in the Tags parameter pack shall be unique.

Maybe an "otherwise the program is ill-formed" here?

@CaseyCarter
Copy link
Collaborator Author

s/was/were/. English's subjunctive case is not yet dead.

This is a weird one. Do we really want to imply that half of the condition is counterfactual ("were") and half isn't ("defines")?

I've decided on "...as-if it defines no data members and Base is its direct and only base class." as being the least confusing option.

The tag specifiers in the Tags parameter pack shall be unique.

Maybe an "otherwise the program is ill-formed" here?

Yuck: $O(N^2)$ metaprogramming. For our purposes $N$ is never greater than 3, IIRC, so I suppose we could do this.

@ericniebler
Copy link
Owner

ericniebler commented Jul 5, 2017

Yuck: $O(N^2)$ metaprogramming

I can force an error for this in O(1):

struct RequireUniqueTags : Tags... {};

@CaseyCarter
Copy link
Collaborator Author

I can force an error on for this in O(1)

Yes, but the diagnostic is not exactly pretty. (Aside: can anyone get this into the immediate context so it SFINAEs?)

@timsong-cpp
Copy link
Contributor

The PR needs to remove more stuff from [alg.tagspec]. The entire three-paragraph text after the declarations can be replaced with something like "Each type in the tag namespace above is a tag specifier ([taggedtup.tagged])."

Incidentally, I'm not a big fan of the fact that tagged_pair<tag::min(const int), tag::max(int)> is derived from pair<int, int>. But that's a problem for another day...

@CaseyCarter
Copy link
Collaborator Author

Incidentally, I'm not a big fan of the fact that tagged_pair<tag::min(const int), tag::max(int)> is derived from pair<int, int>. But that's a problem for another day...

Please open an issue and note your concerns; we can use one of the post-TS tags if it's a design concern that needn't be resolved before publishing the TS.

@CaseyCarter
Copy link
Collaborator Author

The PR needs to remove more stuff from [alg.tagspec].

Done.

@CaseyCarter CaseyCarter removed their assignment Jul 6, 2017
@CaseyCarter CaseyCarter added TS and removed PDTS labels Jul 16, 2017
@CaseyCarter CaseyCarter added NAD and removed new labels Jun 15, 2018
@CaseyCarter
Copy link
Collaborator Author

LEWG has killed tagged for C++20 - closing this as NAD.

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

3 participants