[Bug libstdc++/94062] Cannot construct tuple from convertible types

[Bug libstdc++/94062] Cannot construct tuple from convertible types

[m.cencora at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #1 from m.cencora at gmail dot com ---
Ping

[Bug libstdc++/94062] Cannot construct tuple from convertible types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2020-03-19

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I guess you're compiling in C++17 mode, but I shouldn't have to guess the
options you're passing to the compiler. Please provide them (as required by
https://gcc.gnu.org/bugs/ which you were asked to read when creating a new bug
report).

[Bug libstdc++/94062] Cannot construct tuple from convertible types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Reduced:

#include <type_traits>
#include <tuple>

struct Bar1
{
    Bar1(const Bar1&) = delete;
    Bar1(Bar1&&) = delete;
};

struct Foo1
{
    operator Bar1() const;
};

int main()
{
    static_assert(std::is_constructible<Bar1, const Foo1&>::value, "");

    std::tuple<Bar1> tup{Foo1{}};
}

With -std=gnu++17 this fails inside the tuple(UTypes&&...) constructor:

In file included from 94062.cc:2:
/usr/include/c++/9/tuple: In instantiation of ‘constexpr std::_Head_base<_Idx,
_Head, true>::_Head_base(_UHead&&) [with _UHead = Foo1; long unsigned int _Idx
= 0; _Head = Bar1]’:
/usr/include/c++/9/tuple:349:38:   required from here
94062.cc:19:32:   in ‘constexpr’ expansion of
‘tup.std::tuple<Bar1>::tuple<Foo1>(Foo1{})’
/usr/include/c++/9/tuple:627:61:   in ‘constexpr’ expansion of
‘((std::_Tuple_impl<0, Bar1>*)this)->std::_Tuple_impl<0,
Bar1>::_Tuple_impl<Foo1>((* & std::forward<Foo1>((* & __elements#0))))’
/usr/include/c++/9/tuple:87:35: error: use of deleted function
‘Bar1::Bar1(Bar1&&)’
   87 |  : _Head(std::forward<_UHead>(__h)) { }
      |                                   ^
94062.cc:7:5: note: declared here
    7 |     Bar1(Bar1&&) = delete;
      |     ^~~~


Without -std=gnu++17 the is_constructible check is false, so there's no viable
constructor.

It seems unlikely to be a libstdc++ bug when the same libstdc++ code compiles
fine with Clang, but I'll look into it.

[Bug libstdc++/94062] Cannot construct tuple from convertible types

[m.cencora at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #4 from m.cencora at gmail dot com ---
Ping(In reply to Jonathan Wakely from comment #2)
> I guess you're compiling in C++17 mode, but I shouldn't have to guess the
> options you're passing to the compiler. Please provide them (as required by
> https://gcc.gnu.org/bugs/ which you were asked to read when creating a new
> bug report).

Yes, the only compiler flag I am passing is -std=c++17.

[Bug c++/94062] Cannot construct tuple from convertible types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|libstdc++                   |c++

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Reduced to remove any libstdc++ code:

template<typename T> T&& forward(T& t) { return static_cast<T&&>(t); }

template<typename T>
  struct tuple : T
  {
    template<typename U>
      tuple(U&& u)
      : T(forward<U>(u))
      { }
  };

struct Bar1
{
    Bar1(const Bar1&) = delete;
    Bar1(Bar1&&) = delete;
};

struct Foo1
{
    operator Bar1() const;
};

int main()
{
  tuple<Bar1> t{ Foo1{} };
}

With -std=c++17:

94062.cc: In instantiation of 'tuple<T>::tuple(U&&) [with U = Foo1; T = Bar1]':
94062.cc:25:25:   required from here
94062.cc:8:24: error: use of deleted function 'Bar1::Bar1(Bar1&&)'
    8 |       : T(forward<U>(u))
      |                        ^
94062.cc:15:5: note: declared here
   15 |     Bar1(Bar1&&) = delete;
      |     ^~~~


EDG also rejects it:

"94062.cc", line 8: error: function "Bar1::Bar1(Bar1 &&)" (declared at line 15)
          cannot be referenced -- it is a deleted function
        : T(forward<U>(u))
           ^
          detected during instantiation of "tuple<T>::tuple(U &&) [with T=Bar1,
                    U=Foo1]" at line 25

1 error detected in the compilation of "94062.cc".


But MSVC and Clang compile it in C++17 mode.

[Bug c++/94062] Cannot construct tuple from convertible types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Further reduced:

struct Bar1
{
  Bar1(Bar1&&) = delete;
};

struct Foo1
{
  operator Bar1() const;
};

struct tuple : Bar1
{
  tuple() : Bar1(Foo1{}) { }
};

tuple t;



elide.cc: In constructor 'tuple::tuple()':
elide.cc:13:24: error: use of deleted function 'Bar1::Bar1(Bar1&&)'
   13 |   tuple() : Bar1(Foo1{}) { }
      |                        ^
elide.cc:3:3: note: declared here
    3 |   Bar1(Bar1&&) = delete;
      |   ^~~~


But I think this may be another instance of PR 82113 and elision shouldn't be
done here.

[Bug c++/94062] Cannot construct tuple from convertible types

[m.cencora at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #7 from m.cencora at gmail dot com ---
(In reply to Jonathan Wakely from comment #6)
> Further reduced:
> 
> struct Bar1
> {
>   Bar1(Bar1&&) = delete;
> };
> 
> struct Foo1
> {
>   operator Bar1() const;
> };
> 
> struct tuple : Bar1
> {
>   tuple() : Bar1(Foo1{}) { }
> };
> 
> tuple t;
> 
> 
> 
> elide.cc: In constructor 'tuple::tuple()':
> elide.cc:13:24: error: use of deleted function 'Bar1::Bar1(Bar1&&)'
>    13 |   tuple() : Bar1(Foo1{}) { }
>       |                        ^
> elide.cc:3:3: note: declared here
>     3 |   Bar1(Bar1&&) = delete;
>       |   ^~~~
> 
> 
> But I think this may be another instance of PR 82113 and elision shouldn't
> be done here.

Standard says that if:
std::is_constructible_v<Bar, Foo&&> 
then
std::is_constructible_v<std::tuple<Bar>, Foo&&>

So I think this is not an instance of PR 82113, but an unfortunate consequence
of how tuple is implemented in libstdc++. If tuple elements were not stored as
base classes, but as members than elision is mandatory and it would work.

But I guess to fix this you would have to break ABI (or change C++ standard).

[Bug c++/94062] Cannot construct tuple from convertible types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to m.cencora from comment #7)
> But I guess to fix this you would have to break ABI (or change C++ standard).

No, we can switch to using a data member with the [[no_unique_address]]
attribute, which is on my TODO list for gcc-11. See PR 55713 and PR 93147

[Bug c++/94062] Cannot construct tuple from convertible types

[m.cencora at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #9 from m.cencora at gmail dot com ---
(In reply to Jonathan Wakely from comment #8)

> No, we can switch to using a data member with the [[no_unique_address]]
> attribute, which is on my TODO list for gcc-11. See PR 55713 and PR 93147

Good.

Besides this, do you plan:
 - to make the impl use 'non-recursive' base classes,
 - to guarantee LTR-ordered construction of tuple elements
?

[Bug c++/94062] Cannot construct tuple from convertible types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
No.

[Bug c++/94062] Cannot construct tuple from convertible types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
> Standard says that if:
> std::is_constructible_v<Bar, Foo&&> 
> then
> std::is_constructible_v<std::tuple<Bar>, Foo&&>

Where does it say that?

The fact a constructor participates in overload resolution doesn't mean it has
to be well-formed for all template arguments.

> So I think this is not an instance of PR 82113, but an unfortunate
> consequence of how tuple is implemented in libstdc++. If tuple elements were
> not stored as base classes, but as members than elision is mandatory and it
> would work.

Members using [[no_unique_address]] have the same restrictions.

> But I guess to fix this you would have to break ABI (or change C++ standard).

It wouldn't be an ABI break to make this compile with our std::tuple, because
it never compiled previously. But I'm a little uncomfortable with making the
ABI of std::tuple depend on whether its elements are copy constructible, rather
than just on the elements' layout.

That would mean that std::tuple<Bar> changes layout if you later add a move
constructor to Bar.

[Bug c++/94062] Cannot construct tuple from convertible types

[m.cencora at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #12 from m.cencora at gmail dot com ---
(In reply to Jonathan Wakely from comment #11)
> > Standard says that if:
> > std::is_constructible_v<Bar, Foo&&> 
> > then
> > std::is_constructible_v<std::tuple<Bar>, Foo&&>
> 
> Where does it say that?
> 
> The fact a constructor participates in overload resolution doesn't mean it
> has to be well-formed for all template arguments.

Indirectly from following paragraph 20.5.3.1 in latest C++ draft:
template<class... UTypes> constexpr explicit(see below) tuple(UTypes&&... u);
11
#
Constraints: sizeof...(Types) equals sizeof...(UTypes) and sizeof...(Types)≥1
and is_­constructible_­v<Ti, Ui> is true for all i.
12
#
Effects: Initializes the elements in the tuple with the corresponding value in
std​::​forward<UTypes>(u).
13
#
Remarks: The expression inside explicit is equivalent to:

!conjunction_v<is_convertible<UTypes, Types>...>


So unless I am missing something, I see no escape hatch for making such
constructor ill-formed for some types.

> 
> > So I think this is not an instance of PR 82113, but an unfortunate
> > consequence of how tuple is implemented in libstdc++. If tuple elements were
> > not stored as base classes, but as members than elision is mandatory and it
> > would work.
> 
> Members using [[no_unique_address]] have the same restrictions.
> 
> > But I guess to fix this you would have to break ABI (or change C++ standard).
> 
> It wouldn't be an ABI break to make this compile with our std::tuple,
> because it never compiled previously. But I'm a little uncomfortable with
> making the ABI of std::tuple depend on whether its elements are copy
> constructible, rather than just on the elements' layout.
> 
> That would mean that std::tuple<Bar> changes layout if you later add a move
> constructor to Bar.

Not sure how, because you can create std::tuple<Bar> already by using other
constructors, e.g.:

#include <tuple>

struct Bar
{
    Bar() = default;
    Bar(int i);
    Bar(const Bar&) = delete;
};

struct ConvertibleToBar
{
    operator Bar();
};

std::tuple<Bar> ok1()
{
    return {};
}

std::tuple<Bar> ok2()
{
    return {0};
}

std::tuple<Bar> fail1()
{
    return {ConvertibleToBar{}};
}

[Bug c++/94062] Cannot construct tuple from convertible types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to m.cencora from comment #12)
> So unless I am missing something, I see no escape hatch for making such
> constructor ill-formed for some types.


Consider:

#include <tuple>

struct X
{
  template<typename T>
    X(T)
    { static_assert(!std::is_same_v<T,int>); }
};

static_assert(std::is_constructible_v<X, int>);
static_assert(std::is_constructible_v<std::tuple<X>, int>);

std::tuple<X> t{1};

The fact that a type meets a function's constraints does not mean it has to
compile, it just means the function is not removed from the candidate functions
for overload resolution.

The standard is pretty clear about that:

Constraints: the conditions for the function’s participation in overload
resolution

That's it. It doesn't mean any more than that.

The constructor's effects say it initializes the element from the argument, and
if that initialization is ill-formed (e.g. because it requires a copy that
isn't elided) then the constructor is ill-formed.


> > That would mean that std::tuple<Bar> changes layout if you later add a move
> > constructor to Bar.
> 
> Not sure how, because you can create std::tuple<Bar> already by using other
> constructors, e.g.:

That's irrelevant. I'm talking about a change that would make it possible to
construct non-movable types, so if you change whether a type is movable or not,
it would affect std::tuple. The fact other constructors are already present is
irrelevant, I'm not talking about making anything depend on those.

Bar is an empty class, so we will try to make tuple<Bar> use a
potentially-overlapping subobject (storing a Bar as a base class today, or a
data member with [[no_unique_address] in the near future). That doesn't support
initialization from ConvertibleToBar because the copy cannot be elided.

I could change the tuple code to not use a potentially-overlapping subobject
for non-movable types, so that copy elision works. But now if you add a move
constructor the decision of whether to use a potentially-overlapping subobject
changes. So that means the std::tuple layout would depend on the presence of a
move constructor.

#include <type_traits>

namespace std
{
template<typename T,
         bool compress = std::is_empty_v<T> && std::is_move_constructible_v<T>>
struct Tuple_impl
{
  T t;

  template<typename U>
  Tuple_impl(U&& u) : t(static_cast<U&&>(u)) { }
};

template<typename T>
struct Tuple_impl<T, true>
{
  [[no_unique_address]] T t;

  template<typename U>
  Tuple_impl(U&& u) : t(static_cast<U&&>(u)) { }
};

template<typename... T>
struct tuple : Tuple_impl<T>...
{
  template<typename... U>
  tuple(U&&... u)
  : Tuple_impl<T>(static_cast<U&&>(u))...
  { }
};
}

struct Bar
{
    Bar() = default;
    Bar(int i);
    Bar(const Bar&) = delete;
#ifdef MAKE_IT_MOVABLE
    Bar(Bar&&) = default;
#endif
};

struct ConvertibleToBar
{
    operator Bar();
};

std::tuple<Bar> fail1()
{
    return {ConvertibleToBar{}};
}

static_assert( sizeof(std::tuple<Bar, int>) == 8 );

If you define MAKE_IT_MOVABLE then the static assertion fails. This shows that
the toy implementation above can support construction from ConvertibleToBar but
at the cost of changing layout if a move constructor is added later.

I don't think supporting guaranteed elision for non-copyable, non-movable,
empty types is a sufficiently important use case to make the layout fragile.

[Bug c++/94062] Cannot construct tuple from convertible types

[m.cencora at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #14 from m.cencora at gmail dot com ---
(In reply to Jonathan Wakely from comment #13)
> (In reply to m.cencora from comment #12)
> > So unless I am missing something, I see no escape hatch for making such
> > constructor ill-formed for some types.
> 
> 
> Consider:
> 
> #include <tuple>
> 
> struct X
> {
>   template<typename T>
>     X(T)
>     { static_assert(!std::is_same_v<T,int>); }
> };
> 
> static_assert(std::is_constructible_v<X, int>);
> static_assert(std::is_constructible_v<std::tuple<X>, int>);
> 
> std::tuple<X> t{1};
> 
> The fact that a type meets a function's constraints does not mean it has to
> compile, it just means the function is not removed from the candidate
> functions for overload resolution.
> 
> The standard is pretty clear about that:
> 
> Constraints: the conditions for the function’s participation in overload
> resolution
> 
> That's it. It doesn't mean any more than that.

Ok, but how is it relevant here?
We could provide full definition of well-formed Bar constructors and
CovertibleToBar conversion operator, and it will still fail to compile with
libstdc++'s tuple.

> 
> The constructor's effects say it initializes the element from the argument,
> and if that initialization is ill-formed (e.g. because it requires a copy
> that isn't elided) then the constructor is ill-formed.

But the user didn't introduce any ill-formed code.
Implementation did by relying on copy-elision to work in a place where it is
not guaranteed by the standard.

> 
> 
> > > That would mean that std::tuple<Bar> changes layout if you later add a move
> > > constructor to Bar.
> > 
> > Not sure how, because you can create std::tuple<Bar> already by using other
> > constructors, e.g.:
> 
> That's irrelevant. I'm talking about a change that would make it possible to
> construct non-movable types, so if you change whether a type is movable or
> not, it would affect std::tuple. The fact other constructors are already
> present is irrelevant, I'm not talking about making anything depend on those.
> 
> Bar is an empty class, so we will try to make tuple<Bar> use a
> potentially-overlapping subobject (storing a Bar as a base class today, or a
> data member with [[no_unique_address] in the near future). That doesn't
> support initialization from ConvertibleToBar because the copy cannot be
> elided.
> 
> I could change the tuple code to not use a potentially-overlapping subobject
> for non-movable types, so that copy elision works. But now if you add a move
> constructor the decision of whether to use a potentially-overlapping
> subobject changes. So that means the std::tuple layout would depend on the
> presence of a move constructor.
> 
> #include <type_traits>
> 
> namespace std
> {
> template<typename T,
>          bool compress = std::is_empty_v<T> &&
> std::is_move_constructible_v<T>>
> struct Tuple_impl
> {
>   T t;
> 
>   template<typename U>
>   Tuple_impl(U&& u) : t(static_cast<U&&>(u)) { }
> };
> 
> template<typename T>
> struct Tuple_impl<T, true>
> {
>   [[no_unique_address]] T t;
> 
>   template<typename U>
>   Tuple_impl(U&& u) : t(static_cast<U&&>(u)) { }
> };
> 
> template<typename... T>
> struct tuple : Tuple_impl<T>...
> {
>   template<typename... U>
>   tuple(U&&... u)
>   : Tuple_impl<T>(static_cast<U&&>(u))...
>   { }
> };
> }
> 
> struct Bar
> {
>     Bar() = default;
>     Bar(int i);
>     Bar(const Bar&) = delete;
> #ifdef MAKE_IT_MOVABLE
>     Bar(Bar&&) = default;
> #endif
> };
> 
> struct ConvertibleToBar
> {
>     operator Bar();
> };
> 
> std::tuple<Bar> fail1()
> {
>     return {ConvertibleToBar{}};
> }
> 
> static_assert( sizeof(std::tuple<Bar, int>) == 8 );
> 
> If you define MAKE_IT_MOVABLE then the static assertion fails. This shows
> that the toy implementation above can support construction from
> ConvertibleToBar but at the cost of changing layout if a move constructor is
> added later.
> 
> I don't think supporting guaranteed elision for non-copyable, non-movable,
> empty types is a sufficiently important use case to make the layout fragile.

Whether Bar is empty class or not is irrelevant for this bug.
Maybe this could be fixed it without breaking the ABI for empty classes
scenario, but I see no way around it for non-empty classes.

[Bug c++/94062] Cannot construct tuple from convertible types

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to m.cencora from comment #14)
> Whether Bar is empty class or not is irrelevant for this bug.

No it isn't. Your examples already work for tuples of non-empty classes.

[Bug c++/94062] Cannot construct tuple from convertible types

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:91e6226f880b048275a7ceedef716e159c7cefd9

commit r11-2720-g91e6226f880b048275a7ceedef716e159c7cefd9
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Aug 7 17:13:56 2020 +0100

    libstdc++: Remove inheritance from elements in std::tuple

    This fixes a number of std::tuple bugs by no longer making use of the
    empty base-class optimization. By using the C++20 [[no_unique_address]]
    attribute we can always store the element as a data member, while still
    compressing the layout of tuples containing empty types.

    Since we no longer use inheritance we could also apply the compression
    optimization for final types and for tuples of tuples, but doing so
    would be an ABI break.

    Using [[no_unique_address]] more liberally for the unstable std::__8
    configuration is left for a later date. There may be reasons not to
    apply the attribute unconditionally, e.g. see the discussion about
    guaranteed elision in PR 94062.

    libstdc++-v3/ChangeLog:

            PR libstdc++/55713
            PR libstdc++/71096
            PR libstdc++/93147
            * include/std/tuple [__has_cpp_attribute(no_unique_address)]
            (_Head_base<Idx, Head, true>): New definition of the partial
            specialization, using [[no_unique_address]] instead of
            inheritance.
            * testsuite/libstdc++-prettyprinters/48362.cc: Adjust expected
            output.
            * testsuite/20_util/tuple/comparison_operators/93147.cc: New test.
            * testsuite/20_util/tuple/creation_functions/55713.cc: New test.
            * testsuite/20_util/tuple/element_access/71096.cc: New test.

[Bug c++/94062] Cannot construct tuple from convertible types

[m.cencora at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94062

--- Comment #17 from m.cencora at gmail dot com ---
Ah, I see. I was under the impression, that tuple elements were always stored
as base classes, but now I see that it is only the case if element type is
empty non-final class.