[Bug libstdc++/113060] New: std::variant converting constructor/assignment is non-conforming after P2280?

[Bug libstdc++/113060] New: std::variant converting constructor/assignment is non-conforming after P2280?

[dangelog at gmail dot com]

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

            Bug ID: 113060
           Summary: std::variant converting constructor/assignment is
                    non-conforming after P2280?
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dangelog at gmail dot com
  Target Milestone: ---

GCC 14 implements P2280 (see #106650). 

As a side effect of that, the "narrowing detector" used in std::variant's
converting constructor/assignment is now too restrictive. This code:


    // IC is a type with a constexpr conversion operator
    using IC = std::integral_constant<int, 42>;
    std::variant<float> v( IC{} );

should work after P2280 (libstdc++ says ill-formed).

---

https://eel.is/c++draft/variant.ctor#14 says:

> Let Tj be a type that is determined as follows: build an imaginary function FUN(Ti) for each alternative type Ti for which Ti x[] = {std​::​forward<T>(t)}; is well-formed for some invented variable x.
> The overload FUN(Tj) selected by overload resolution for the expression FUN(std​::​forward<T>(​t)) defines the alternative Tj which is the type of the contained value after construction.


Right now libstdc++ implements this by means of SFINAE:

> https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/variant#L787-L823


IC is convertible to float, and given the constexpr nature of its conversion
operator (to int), it wouldn't be a narrowing conversion:

  IC ic;
  float f{ic}; // not narrowing

However, using SFINAE means that std::declval<IC>() is used to build the
"candidate" argument of FUN. declval is not a constexpr function so its
returned value is not considered to be usable in constant expressions. 

The net effect is that `Ti x[] = {std​::​forward<T>(t)};` is considered to be a
narrowing conversion ([dcl.init.list], int->float, source is not a constant
expression), and FUN(float) rejected.

But nowhere does [variant.ctor] talk about using std::declval; if one
reimplements the same check with a constraint, then GCC 14 accepts the
conversion:

    template <typename Tj, typename T>
    concept FUN_constraint = requires(T &&t) {
        { std::type_identity_t<Tj[]>{ std::forward<T>(t) } };
    };

    template <typename T>
    requires FUN_constraint<float, T>
    void FUN(float);

    FUN<IC>( IC{} ); // OK

https://gcc.godbolt.org/z/xP9z97v35

P2280 is necessary to make this work, because otherwise the usage of a
reference in the requires-expression would make `t` again not usable in
constant expressions. In conclusion, it seems that variant<float> should indeed
accept construction from IC.

(I can't cross-check this report with other compilers as none of them
implements P2280 yet.)

[Bug libstdc++/113060] std::variant converting constructor/assignment is non-conforming after P2280?

[redi at gcc dot gnu.org]

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

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Giuseppe D'Angelo from comment #0)
> GCC 14 implements P2280 (see #106650). 

N.B. if you say "Bug 106650" or "PR 106650" or "bug #106650" or pretty much
anything except just #106650 then bugzilla makes it a clickable link :)

> it seems that variant<float> should indeed accept construction from IC.

I'm not convinced that this change to the semantics of std::variant was an
intended side effect of https://wg21.link/P2280 -- I think I'd prefer if the
committee confirmed it.

The standard doesn't say that it _should_ work in a constant expression, it
seems like you're assuming that because it's now possible to implement it so
that it works, that it's required to work.

My reading of [variant.ctor] p14 is that "some invented variable x" is not a
constant expression, so using std::declval as a stand-in for "some invented
variable" is fine.

If we made this change, then some cases that compile today would become
ill-formed, e.g. see PR 113007. Under your reading, conversion from the
constant 42 would be valid for both int64_t and double, and so the
initialization would be ill-formed. But it would be well-formed when using a
non-constant int equal to 42.

Apart from the fact it would be a breaking change, the difference in behaviour
between runtime and compile time would be surprising. It would also mean that
std::is_constructible would be misleading: it would say you can construct
variant<long, double> from 42, but when you try to do that it would fail to
compile.

[Bug libstdc++/113060] std::variant converting constructor/assignment is non-conforming after P2280?

[dangelog at gmail dot com]

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

--- Comment #2 from Giuseppe D'Angelo <dangelog at gmail dot com> ---
(In reply to Jonathan Wakely from comment #1)
> (In reply to Giuseppe D'Angelo from comment #0)
> > GCC 14 implements P2280 (see #106650). 
> 
> N.B. if you say "Bug 106650" or "PR 106650" or "bug #106650" or pretty much
> anything except just #106650 then bugzilla makes it a clickable link :)

D'oh! 


> > it seems that variant<float> should indeed accept construction from IC.
> 
> I'm not convinced that this change to the semantics of std::variant was an
> intended side effect of https://wg21.link/P2280 -- I think I'd prefer if the
> committee confirmed it.

Ok, I'll raise the question to LEWG. Or maybe EWG?


> The standard doesn't say that it _should_ work in a constant expression, it
> seems like you're assuming that because it's now possible to implement it so
> that it works, that it's required to work.
> 
> My reading of [variant.ctor] p14 is that "some invented variable x" is not a
> constant expression, so using std::declval as a stand-in for "some invented
> variable" is fine.


But it does say how you get that x, you get it from the statement:

  Tj x[] = {std​::​forward<T>(t)};

Nowhere in there's a call to declval. My reasoning is simply that if you spell
FUN out for Ti = float, something like this (although the standard doesn't
really specify the signature of FUN, which is not ideal):

  template <typename T>
  auto FUN_float(T &&t) {
      float x[] = {std​::​forward<T>(t)};
  }

then FUN_float(IC{}) is well-formed after P2280.

Note that in general

  IC c;       // not const!!!
  float f{c}; // OK

is well-formed, and this is even before P2280. What seems to have changed with
P2280 is that references do not disqualify expressions from being constant, and
thus we can now detect this case a requires-expression that uses references.


> If we made this change, then some cases that compile today would become
> ill-formed, e.g. see PR 113007. Under your reading, conversion from the
> constant 42 would be valid for both int64_t and double, and so the
> initialization would be ill-formed. But it would be well-formed when using a
> non-constant int equal to 42.

Are you sure it would? FUN_double(42) should be still ill-formed (!), due to
narrowing. 

My reasoning, in the context of P0870 (is_convertible_without_narrowing), is
that int->double requires a lvalue-to-rvalue conversion, and then this kicks in
https://eel.is/c++draft/expr.const#5.9 , disqualifying `t` from being treated
as a constant expression. So then the list-initialization of `x` triggers
narrowing.

This is different from IC->double, where there's no lvalue-to-rvalue conversion
(!), it's described here
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p0870r5.html#ch5.9

Testcase: https://gcc.godbolt.org/z/rfvnfvYzs


> Apart from the fact it would be a breaking change, the difference in
> behaviour between runtime and compile time would be surprising. It would
> also mean that std::is_constructible would be misleading: it would say you
> can construct variant<long, double> from 42, but when you try to do that it
> would fail to compile.

Sure, I'm really struggling at squaring the P2280 rules with detecting
narrowing conversions, but I'm not sure if this would actually break. (And I'm
not sure if something _else_ would break instead.)

[Bug libstdc++/113060] std::variant converting constructor/assignment is non-conforming after P2280?

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |SUSPENDED
   Last reconfirmed|                            |2024-02-16
     Ever confirmed|0                           |1

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Suspending until
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3146r0.html is
handled.

[Bug libstdc++/113060] std::variant converting constructor/assignment is non-conforming after P2280?

[de34 at live dot cn]

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

Jiang An <de34 at live dot cn> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |de34 at live dot cn

--- Comment #4 from Jiang An <de34 at live dot cn> ---
Hi, I found that we can support std::variant<float>(IC{}) with C++17 core
language rules (as modified by P2280R4), and thus P3146 should be updated.

Proof-of-concept example (https://gcc.godbolt.org/z/174sx397f):

```
#include <type_traits>
#include <utility>

template<class Ti>
struct array_element_initialization_tester {
    Ti elems[1];
};

template<class Ti, class Tp>
auto test_array_element_initializable(Tp&& t)
    -> decltype((void)
array_element_initialization_tester<Ti>{{std::forward<Tp>(t)}});

template<class Ti, class Tp, class = void>
constexpr bool is_array_element_initializable_from = false;
template<class Ti, class Tp>
constexpr bool is_array_element_initializable_from<Ti, Tp,
    decltype(::test_array_element_initializable<Ti>(std::declval<Tp>()))> =
true;

template <typename T,
std::enable_if_t<is_array_element_initializable_from<float, T>, int> = 0>
void FUN(float);

int main() {
    using IC = std::integral_constant<int, 42>;
    FUN<IC>(IC{});
    IC ic;
    FUN<IC&>(ic);
}
```

Note that this example adds a mediate function template
(test_array_element_initializable) to "reduce" the non-constexpr-ness of
std::declval.

[Bug libstdc++/113060] std::variant converting constructor/assignment is non-conforming after P2280?

[de34 at live dot cn]

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

--- Comment #5 from Jiang An <de34 at live dot cn> ---
Function pointers seem working (https://gcc.godbolt.org/z/Mbvfafdof).

```
template<class Ti, class Tp, class = void>
constexpr bool is_array_element_initializable_from = false;
template<class Ti, class Tp>
constexpr bool is_array_element_initializable_from<Ti, Tp,
    decltype(std::declval<auto (*)(Tp&& t)
        -> decltype((void)
array_element_initialization_tester<Ti>{{std::forward<Tp>(t)}})>()
    (std::declval<Tp>()))> = true;
```

So perhaps we can simply change `decltype(_Arr<_Ti>{{std::declval<_Tp>()}})` to
`decltype(std::declval<auto (*)(_Tp&& __t) ->
decltype(_Arr<_Ti>{{std::forward<_Tp>(__t)}})>(std::declval<_Tp>()))` to
resolve this "bug".

[Bug libstdc++/113060] std::variant converting constructor/assignment is non-conforming after P2280?

[de34 at live dot cn]

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

--- Comment #6 from Jiang An <de34 at live dot cn> ---
(In reply to Jiang An from comment #5)
> `decltype(std::declval<auto (*)(_Tp&& __t) ->
> decltype(_Arr<_Ti>{{std::forward<_Tp>(__t)}})>(std::declval<_Tp>()))`

Typo, this should be

`decltype(std::declval<auto (*)(_Tp&& __t) ->
decltype(_Arr<_Ti>{{std::forward<_Tp>(__t)}})>()(std::declval<_Tp>()))`.

[Bug libstdc++/113060] std::variant converting constructor/assignment is non-conforming after P2280?

[dangelog at gmail dot com]

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

--- Comment #7 from Giuseppe D'Angelo <dangelog at gmail dot com> ---
Hi,

> Note that this example adds a mediate function template (test_array_element_initializable) to "reduce" the non-constexpr-ness of std::declval.

That's very clever, thank you! 

Is it _supposed_ to work, though? I had imagined (possibly erroneusly) that
once one places the call to `test_array_element_initializable` using `declval`
as an argument, it would disqualify the whole thing from being usable in
constant expressions.

(It would help to have another compiler that implements P2280, so to do more
tests...)

[Bug libstdc++/113060] std::variant converting constructor/assignment is non-conforming after P2280?

[de34 at live dot cn]

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

--- Comment #8 from Jiang An <de34 at live dot cn> ---
(In reply to Giuseppe D'Angelo from comment #7)
> Hi,
> 
> > Note that this example adds a mediate function template (test_array_element_initializable) to "reduce" the non-constexpr-ness of std::declval.
> 
> That's very clever, thank you! 
> 
> Is it _supposed_ to work, though? I had imagined (possibly erroneusly) that
> once one places the call to `test_array_element_initializable` using
> `declval` as an argument, it would disqualify the whole thing from being
> usable in constant expressions.
> 
> (It would help to have another compiler that implements P2280, so to do more
> tests...)

I think it's supposed to work. Enclosing std::declval calls don't matter
because only the constantness in the trailing return type would affect overload
resolution.

Ah, we don't even need to call the function template or function pointers -
it's sufficient to only detect the well-formedness of the function type.

The simplest "fix" I found is changing
`void_t<decltype(_Arr<_Ti>{{std::declval<_Tp>()}})>`
to
`void_t<auto (_Tp&& __t) -> decltype(_Arr<_Ti>{{std::forward<_Tp>(__t)}})>`
, which seemingly works (https://godbolt.org/z/8M85zre5P).

[Bug libstdc++/113060] std::variant converting constructor/assignment is non-conforming after P2280?

[dangelog at gmail dot com]

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

--- Comment #9 from Giuseppe D'Angelo <dangelog at gmail dot com> ---
Thank you, I'll amend P3146 with this new information, and try and make sure
that LEWG/LWG see the updated draft (if they discuss this before the next
mailing).