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

[libc++] std::variant converting constructor and operator= compile while the C++ Standard says they must not #75323

Closed
toughengineer opened this issue Dec 13, 2023 · 2 comments
Labels
c++17 invalid Resolved as invalid, i.e. not a bug libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@toughengineer
Copy link

toughengineer commented Dec 13, 2023

According to [variant.ctor] (also see cppreference) converting constructor

template<class T>
constexpr variant(T&& t) noexcept(/*...*/);

determines the alternative to construct as if by selecting an overload for F(std​::​forward<T>(​t)) from an overload set F(T_j) where T_j are types of the alternatives,
e.g. for std::variant<std::monostate, bool, int64_t, double> the overload set would be

void F(std::monostate) {}
void F(bool) {}
void F(int64_t) {}
void F(double) {}

In violation of the standard this code compiles with Clang and libc++ (https://godbolt.org/z/aah9Pq9fq):

std::variant<std::monostate, bool, int64_t, double> v = 42;

while overload resolution of F() fails:

F(42); // error: ambiguous confused_john_travolta.gif
compiler error (click/tap to expand)
<source>:14:3: error: call to 'F' is ambiguous
   14 |   F(42);
      |   ^
<source>:6:6: note: candidate function
    6 | void F(bool) {}
      |      ^
<source>:7:6: note: candidate function
    7 | void F(int64_t) {}
      |      ^
<source>:8:6: note: candidate function
    8 | void F(double) {}
      |      ^

Also the same applies to converting assignment operator

template<class T>
constexpr variant& operator=(T&& t) noexcept(/*...*/);

see [variant.assign] (also cppreference).

Among libc++, libstdc++ and MSVC standard libraries only MSVC standard library gets it according to the standard so you can look at it for reference.

See corresponding bug for libstdc++: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113007

@toughengineer toughengineer changed the title std::variant converting constructor compiles while the C++ Standard says it must not std::variant converting constructor and operator= compile while the C++ Standard says they must not Dec 13, 2023
@toughengineer toughengineer changed the title std::variant converting constructor and operator= compile while the C++ Standard says they must not [libc++] std::variant converting constructor and operator= compile while the C++ Standard says they must not Dec 13, 2023
@dtcxzyw dtcxzyw added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. c++17 and removed new issue labels Dec 13, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 13, 2023

@llvm/issue-subscribers-c-17

Author: Pavel Novikov (toughengineer)

According to [\[variant.ctor\]](https://eel.is/c++draft/variant.ctor) (also see [cppreference](https://en.cppreference.com/w/cpp/utility/variant/variant)) converting constructor ```c++ template<class T> constexpr variant(T&& t) noexcept(/*...*/); ``` determines the alternative to construct as if by selecting an overload for `F(std​::​forward<T>(​t))` from an overload set `F(T_j)` where `T_j` are types of the alternatives, e.g. for `std::variant<std::monostate, bool, int64_t, double>` the overload set would be ```c++ void F(std::monostate) {} void F(bool) {} void F(int64_t) {} void F(double) {} ```

In violation of the standard this code compiles with Clang and libc++ (https://godbolt.org/z/aah9Pq9fq):

std::variant&lt;std::monostate, bool, int64_t, double&gt; v = 42;

while overload resolution of F() fails:

F(42); // error: ambiguous

<details><summary>compiler error (click/tap to expand)</summary>

&lt;source&gt;:14:3: error: call to 'F' is ambiguous
   14 |   F(42);
      |   ^
&lt;source&gt;:6:6: note: candidate function
    6 | void F(bool) {}
      |      ^
&lt;source&gt;:7:6: note: candidate function
    7 | void F(int64_t) {}
      |      ^
&lt;source&gt;:8:6: note: candidate function
    8 | void F(double) {}
      |      ^

</details>

Also the same applies to converting assignment operator

template&lt;class T&gt;
constexpr variant&amp; operator=(T&amp;&amp; t) noexcept(/*...*/);

see [variant.assign] (also cppreference).

Among libc++, libstdc++ and MSVC standard libraries only MSVC standard library gets it according to the standard so you can look at it for reference.

@jwakely
Copy link
Contributor

jwakely commented Dec 14, 2023

Libc++ is correct here, see my explanation in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113007
(tl;dr the conversion to bool or double is considered narrowing so those conversions are not allowed, and only int64_t is viable).

@philnik777 philnik777 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 14, 2023
@philnik777 philnik777 added the invalid Resolved as invalid, i.e. not a bug label Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++17 invalid Resolved as invalid, i.e. not a bug libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

5 participants