<div dir="ltr">Hi Zhihao,<div><br></div><div>This commit has caused a large number of compiler failures in Google's codebase.</div><div>I don't think all of them were intended. For example: <br>```<br>variant<uint32_t> source6(42);<br>```</div><div><br></div><div>Could you please roll back so we can further evaluate the impact of this change?</div><div>Additionally, could you review the examples I provided and confirm that they</div><div>should break under the new wording?</div><div><br>/Eric</div><div><br>[1] <a href="https://godbolt.org/z/oU9v6U">https://godbolt.org/z/oU9v6U</a> (Abseil's std/absl variant tests)</div><div>[2] <a href="https://godbolt.org/z/-YAaTc">https://godbolt.org/z/-YAaTc</a> (user code in Google)<br>[3] <a href="https://godbolt.org/z/YW_aSU">https://godbolt.org/z/YW_aSU</a> (another example of broken user code)</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 18, 2019 at 11:23 AM Zhihao Yuan via libcxx-commits <<a href="mailto:libcxx-commits@lists.llvm.org">libcxx-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: lichray<br>
Date: Tue Jun 18 08:26:50 2019<br>
New Revision: 363692<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=363692&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=363692&view=rev</a><br>
Log:<br>
[libc++] Implement P0608R3 - A sane variant converting constructor<br>
<br>
Summary:<br>
Prefer user-defined conversions over narrowing conversions and conversions to bool.<br>
<br>
References:<br>
<a href="http://wg21.link/p0608" rel="noreferrer" target="_blank">http://wg21.link/p0608</a><br>
<br>
Reviewers: EricWF, mpark, mclow.lists<br>
<br>
Reviewed By: mclow.lists<br>
<br>
Subscribers: zoecarver, ldionne, libcxx-commits, cfe-commits, christof<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D44865" rel="noreferrer" target="_blank">https://reviews.llvm.org/D44865</a><br>
<br>
Added:<br>
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp<br>
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp<br>
Modified:<br>
libcxx/trunk/include/variant<br>
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp<br>
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp<br>
libcxx/trunk/www/cxx2a_status.html<br>
<br>
Modified: libcxx/trunk/include/variant<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/variant?rev=363692&r1=363691&r2=363692&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/variant?rev=363692&r1=363691&r2=363692&view=diff</a><br>
==============================================================================<br>
--- libcxx/trunk/include/variant (original)<br>
+++ libcxx/trunk/include/variant Tue Jun 18 08:26:50 2019<br>
@@ -1098,11 +1098,39 @@ struct __overload<> { void operator()()<br>
template <class _Tp, class... _Types><br>
struct __overload<_Tp, _Types...> : __overload<_Types...> {<br>
using __overload<_Types...>::operator();<br>
- __identity<_Tp> operator()(_Tp) const;<br>
+<br>
+ static auto __test(_Tp (&&)[1]) -> __identity<_Tp>;<br>
+<br>
+ template <class _Up><br>
+ auto operator()(_Tp, _Up&& __t) const<br>
+ -> decltype(__test({ _VSTD::forward<_Up>(__t) }));<br>
+};<br>
+<br>
+template <class _Base, class _Tp><br>
+struct __overload_bool : _Base {<br>
+ using _Base::operator();<br>
+<br>
+ template <class _Up, class _Ap = __uncvref_t<_Up>><br>
+ auto operator()(bool, _Up&&) const<br>
+ -> enable_if_t<is_same_v<_Ap, bool>, __identity<_Tp>>;<br>
};<br>
<br>
+template <class... _Types><br>
+struct __overload<bool, _Types...><br>
+ : __overload_bool<__overload<_Types...>, bool> {};<br>
+template <class... _Types><br>
+struct __overload<bool const, _Types...><br>
+ : __overload_bool<__overload<_Types...>, bool const> {};<br>
+template <class... _Types><br>
+struct __overload<bool volatile, _Types...><br>
+ : __overload_bool<__overload<_Types...>, bool volatile> {};<br>
+template <class... _Types><br>
+struct __overload<bool const volatile, _Types...><br>
+ : __overload_bool<__overload<_Types...>, bool const volatile> {};<br>
+<br>
template <class _Tp, class... _Types><br>
-using __best_match_t = typename result_of_t<__overload<_Types...>(_Tp&&)>::type;<br>
+using __best_match_t =<br>
+ typename invoke_result_t<__overload<_Types...>, _Tp, _Tp>::type;<br>
<br>
} // __variant_detail<br>
<br>
<br>
Modified: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp?rev=363692&r1=363691&r2=363692&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp?rev=363692&r1=363691&r2=363692&view=diff</a><br>
==============================================================================<br>
--- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp (original)<br>
+++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp Tue Jun 18 08:26:50 2019<br>
@@ -22,6 +22,7 @@<br>
#include <string><br>
#include <type_traits><br>
#include <variant><br>
+#include <memory><br>
<br>
#include "test_macros.h"<br>
#include "variant_test_helpers.hpp"<br>
@@ -122,7 +123,7 @@ void test_T_assignment_noexcept() {<br>
<br>
void test_T_assignment_sfinae() {<br>
{<br>
- using V = std::variant<long, unsigned>;<br>
+ using V = std::variant<long, long long>;<br>
static_assert(!std::is_assignable<V, int>::value, "ambiguous");<br>
}<br>
{<br>
@@ -133,6 +134,31 @@ void test_T_assignment_sfinae() {<br>
using V = std::variant<std::string, void *>;<br>
static_assert(!std::is_assignable<V, int>::value, "no matching operator=");<br>
}<br>
+ {<br>
+ using V = std::variant<std::string, float>;<br>
+ static_assert(!std::is_assignable<V, int>::value, "no matching operator=");<br>
+ }<br>
+ {<br>
+ using V = std::variant<std::unique_ptr<int>, bool>;<br>
+ static_assert(!std::is_assignable<V, std::unique_ptr<char>>::value,<br>
+ "no explicit bool in operator=");<br>
+ struct X {<br>
+ operator void*();<br>
+ };<br>
+ static_assert(!std::is_assignable<V, X>::value,<br>
+ "no boolean conversion in operator=");<br>
+ static_assert(!std::is_assignable<V, std::false_type>::value,<br>
+ "no converted to bool in operator=");<br>
+ }<br>
+ {<br>
+ struct X {};<br>
+ struct Y {<br>
+ operator X();<br>
+ };<br>
+ using V = std::variant<X>;<br>
+ static_assert(std::is_assignable<V, Y>::value,<br>
+ "regression on user-defined conversions in operator=");<br>
+ }<br>
#if !defined(TEST_VARIANT_HAS_NO_REFERENCES)<br>
{<br>
using V = std::variant<int, int &&>;<br>
@@ -161,6 +187,37 @@ void test_T_assignment_basic() {<br>
assert(v.index() == 1);<br>
assert(std::get<1>(v) == 43);<br>
}<br>
+ {<br>
+ std::variant<unsigned, long> v;<br>
+ v = 42;<br>
+ assert(v.index() == 1);<br>
+ assert(std::get<1>(v) == 42);<br>
+ v = 43u;<br>
+ assert(v.index() == 0);<br>
+ assert(std::get<0>(v) == 43);<br>
+ }<br>
+ {<br>
+ std::variant<std::string, bool> v = true;<br>
+ v = "bar";<br>
+ assert(v.index() == 0);<br>
+ assert(std::get<0>(v) == "bar");<br>
+ }<br>
+ {<br>
+ std::variant<bool, std::unique_ptr<int>> v;<br>
+ v = nullptr;<br>
+ assert(v.index() == 1);<br>
+ assert(std::get<1>(v) == nullptr);<br>
+ }<br>
+ {<br>
+ std::variant<bool volatile, int> v = 42;<br>
+ v = false;<br>
+ assert(v.index() == 0);<br>
+ assert(!std::get<0>(v));<br>
+ bool lvt = true;<br>
+ v = lvt;<br>
+ assert(v.index() == 0);<br>
+ assert(std::get<0>(v));<br>
+ }<br>
#if !defined(TEST_VARIANT_HAS_NO_REFERENCES)<br>
{<br>
using V = std::variant<int &, int &&, long>;<br>
<br>
Added: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp?rev=363692&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp?rev=363692&view=auto</a><br>
==============================================================================<br>
--- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp (added)<br>
+++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp Tue Jun 18 08:26:50 2019<br>
@@ -0,0 +1,52 @@<br>
+// -*- C++ -*-<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.<br>
+// See <a href="https://llvm.org/LICENSE.txt" rel="noreferrer" target="_blank">https://llvm.org/LICENSE.txt</a> for license information.<br>
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+// UNSUPPORTED: c++98, c++03, c++11, c++14<br>
+<br>
+// <variant><br>
+<br>
+// template <class ...Types> class variant;<br>
+<br>
+// template <class T><br>
+// variant& operator=(T&&) noexcept(see below);<br>
+<br>
+#include <variant><br>
+#include <string><br>
+#include <memory><br>
+<br>
+int main(int, char**)<br>
+{<br>
+ std::variant<int, int> v1;<br>
+ std::variant<long, long long> v2;<br>
+ std::variant<char> v3;<br>
+ v1 = 1; // expected-error {{no viable overloaded '='}}<br>
+ v2 = 1; // expected-error {{no viable overloaded '='}}<br>
+ v3 = 1; // expected-error {{no viable overloaded '='}}<br>
+<br>
+ std::variant<std::string, float> v4;<br>
+ std::variant<std::string, double> v5;<br>
+ std::variant<std::string, bool> v6;<br>
+ v4 = 1; // expected-error {{no viable overloaded '='}}<br>
+ v5 = 1; // expected-error {{no viable overloaded '='}}<br>
+ v6 = 1; // expected-error {{no viable overloaded '='}}<br>
+<br>
+ std::variant<int, bool> v7;<br>
+ std::variant<int, bool const> v8;<br>
+ std::variant<int, bool volatile> v9;<br>
+ v7 = "meow"; // expected-error {{no viable overloaded '='}}<br>
+ v8 = "meow"; // expected-error {{no viable overloaded '='}}<br>
+ v9 = "meow"; // expected-error {{no viable overloaded '='}}<br>
+<br>
+ std::variant<bool> v10;<br>
+ std::variant<bool> v11;<br>
+ std::variant<bool> v12;<br>
+ v10 = std::true_type(); // expected-error {{no viable overloaded '='}}<br>
+ v11 = std::unique_ptr<char>(); // expected-error {{no viable overloaded '='}}<br>
+ v12 = nullptr; // expected-error {{no viable overloaded '='}}<br>
+}<br>
<br>
Modified: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp?rev=363692&r1=363691&r2=363692&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp?rev=363692&r1=363691&r2=363692&view=diff</a><br>
==============================================================================<br>
--- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp (original)<br>
+++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp Tue Jun 18 08:26:50 2019<br>
@@ -20,8 +20,8 @@<br>
#include <string><br>
#include <type_traits><br>
#include <variant><br>
+#include <memory><br>
<br>
-#include "test_convertible.hpp"<br>
#include "test_macros.h"<br>
#include "variant_test_helpers.hpp"<br>
<br>
@@ -39,6 +39,8 @@ struct NoThrowT {<br>
<br>
struct AnyConstructible { template <typename T> AnyConstructible(T&&) {} };<br>
struct NoConstructible { NoConstructible() = delete; };<br>
+template <class T><br>
+struct RValueConvertibleFrom { RValueConvertibleFrom(T&&) {} };<br>
<br>
void test_T_ctor_noexcept() {<br>
{<br>
@@ -53,7 +55,7 @@ void test_T_ctor_noexcept() {<br>
<br>
void test_T_ctor_sfinae() {<br>
{<br>
- using V = std::variant<long, unsigned>;<br>
+ using V = std::variant<long, long long>;<br>
static_assert(!std::is_constructible<V, int>::value, "ambiguous");<br>
}<br>
{<br>
@@ -66,6 +68,32 @@ void test_T_ctor_sfinae() {<br>
"no matching constructor");<br>
}<br>
{<br>
+ using V = std::variant<std::string, float>;<br>
+ static_assert(!std::is_constructible<V, int>::value,<br>
+ "no matching constructor");<br>
+ }<br>
+ {<br>
+ using V = std::variant<std::unique_ptr<int>, bool>;<br>
+ static_assert(!std::is_constructible<V, std::unique_ptr<char>>::value,<br>
+ "no explicit bool in constructor");<br>
+ struct X {<br>
+ operator void*();<br>
+ };<br>
+ static_assert(!std::is_constructible<V, X>::value,<br>
+ "no boolean conversion in constructor");<br>
+ static_assert(!std::is_constructible<V, std::false_type>::value,<br>
+ "no converted to bool in constructor");<br>
+ }<br>
+ {<br>
+ struct X {};<br>
+ struct Y {<br>
+ operator X();<br>
+ };<br>
+ using V = std::variant<X>;<br>
+ static_assert(std::is_constructible<V, Y>::value,<br>
+ "regression on user-defined conversions in constructor");<br>
+ }<br>
+ {<br>
using V = std::variant<AnyConstructible, NoConstructible>;<br>
static_assert(<br>
!std::is_constructible<V, std::in_place_type_t<NoConstructible>>::value,<br>
@@ -99,6 +127,34 @@ void test_T_ctor_basic() {<br>
static_assert(v.index() == 1, "");<br>
static_assert(std::get<1>(v) == 42, "");<br>
}<br>
+ {<br>
+ constexpr std::variant<unsigned, long> v(42);<br>
+ static_assert(v.index() == 1, "");<br>
+ static_assert(std::get<1>(v) == 42, "");<br>
+ }<br>
+ {<br>
+ std::variant<std::string, bool const> v = "foo";<br>
+ assert(v.index() == 0);<br>
+ assert(std::get<0>(v) == "foo");<br>
+ }<br>
+ {<br>
+ std::variant<bool volatile, std::unique_ptr<int>> v = nullptr;<br>
+ assert(v.index() == 1);<br>
+ assert(std::get<1>(v) == nullptr);<br>
+ }<br>
+ {<br>
+ std::variant<bool volatile const, int> v = true;<br>
+ assert(v.index() == 0);<br>
+ assert(std::get<0>(v));<br>
+ }<br>
+ {<br>
+ std::variant<RValueConvertibleFrom<int>> v1 = 42;<br>
+ assert(v1.index() == 0);<br>
+<br>
+ int x = 42;<br>
+ std::variant<RValueConvertibleFrom<int>, AnyConstructible> v2 = x;<br>
+ assert(v2.index() == 1);<br>
+ }<br>
#if !defined(TEST_VARIANT_HAS_NO_REFERENCES)<br>
{<br>
using V = std::variant<const int &, int &&, long>;<br>
<br>
Added: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp?rev=363692&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp?rev=363692&view=auto</a><br>
==============================================================================<br>
--- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp (added)<br>
+++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp Tue Jun 18 08:26:50 2019<br>
@@ -0,0 +1,39 @@<br>
+// -*- C++ -*-<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.<br>
+// See <a href="https://llvm.org/LICENSE.txt" rel="noreferrer" target="_blank">https://llvm.org/LICENSE.txt</a> for license information.<br>
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+// UNSUPPORTED: c++98, c++03, c++11, c++14<br>
+<br>
+// <variant><br>
+<br>
+// template <class ...Types> class variant;<br>
+<br>
+// template <class T> constexpr variant(T&&) noexcept(see below);<br>
+<br>
+#include <variant><br>
+#include <string><br>
+#include <memory><br>
+<br>
+int main(int, char**)<br>
+{<br>
+ std::variant<int, int> v1 = 1; // expected-error {{no viable conversion}}<br>
+ std::variant<long, long long> v2 = 1; // expected-error {{no viable conversion}}<br>
+ std::variant<char> v3 = 1; // expected-error {{no viable conversion}}<br>
+<br>
+ std::variant<std::string, float> v4 = 1; // expected-error {{no viable conversion}}<br>
+ std::variant<std::string, double> v5 = 1; // expected-error {{no viable conversion}}<br>
+ std::variant<std::string, bool> v6 = 1; // expected-error {{no viable conversion}}<br>
+<br>
+ std::variant<int, bool> v7 = "meow"; // expected-error {{no viable conversion}}<br>
+ std::variant<int, bool const> v8 = "meow"; // expected-error {{no viable conversion}}<br>
+ std::variant<int, bool volatile> v9 = "meow"; // expected-error {{no viable conversion}}<br>
+<br>
+ std::variant<bool> v10 = std::true_type(); // expected-error {{no viable conversion}}<br>
+ std::variant<bool> v11 = std::unique_ptr<char>(); // expected-error {{no viable conversion}}<br>
+ std::variant<bool> v12 = nullptr; // expected-error {{no viable conversion}}<br>
+}<br>
<br>
Modified: libcxx/trunk/www/cxx2a_status.html<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx2a_status.html?rev=363692&r1=363691&r2=363692&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx2a_status.html?rev=363692&r1=363691&r2=363692&view=diff</a><br>
==============================================================================<br>
--- libcxx/trunk/www/cxx2a_status.html (original)<br>
+++ libcxx/trunk/www/cxx2a_status.html Tue Jun 18 08:26:50 2019<br>
@@ -115,7 +115,7 @@<br>
<tr><td><a href="<a href="https://wg21.link/P0591R4" rel="noreferrer" target="_blank">https://wg21.link/P0591R4</a>">P0591R4</a></td><td>LWG</td><td>Utility functions to implement uses-allocator construction</td><td>San Diego</td><td><i> </i></td><td></td></tr><br>
<tr><td><a href="<a href="https://wg21.link/P0595R2" rel="noreferrer" target="_blank">https://wg21.link/P0595R2</a>">P0595R2</a></td><td>CWG</td><td>P0595R2 std::is_constant_evaluated()</td><td>San Diego</td><td>Complete</td><td>9.0</td></tr><br>
<tr><td><a href="<a href="https://wg21.link/P0602R4" rel="noreferrer" target="_blank">https://wg21.link/P0602R4</a>">P0602R4</a></td><td>LWG</td><td>variant and optional should propagate copy/move triviality</td><td>San Diego</td><td>Complete</td><td>8.0</td></tr><br>
- <tr><td><a href="<a href="https://wg21.link/P0608R3" rel="noreferrer" target="_blank">https://wg21.link/P0608R3</a>">P0608R3</a></td><td>LWG</td><td>A sane variant converting constructor</td><td>San Diego</td><td><i> </i></td><td></td></tr><br>
+ <tr><td><a href="<a href="https://wg21.link/P0608R3" rel="noreferrer" target="_blank">https://wg21.link/P0608R3</a>">P0608R3</a></td><td>LWG</td><td>A sane variant converting constructor</td><td>San Diego</td><td>Complete</td><td>9.0</td></tr><br>
<tr><td><a href="<a href="https://wg21.link/P0655R1" rel="noreferrer" target="_blank">https://wg21.link/P0655R1</a>">P0655R1</a></td><td>LWG</td><td>visit<R>: Explicit Return Type for visit</td><td>San Diego</td><td><i> </i></td><td></td></tr><br>
<tr><td><a href="<a href="https://wg21.link/P0771R1" rel="noreferrer" target="_blank">https://wg21.link/P0771R1</a>">P0771R1</a></td><td>LWG</td><td>std::function move constructor should be noexcept</td><td>San Diego</td><td>Complete</td><td>6.0</td></tr><br>
<tr><td><a href="<a href="https://wg21.link/P0896R4" rel="noreferrer" target="_blank">https://wg21.link/P0896R4</a>">P0896R4</a></td><td>LWG</td><td>The One Ranges Proposal</td><td>San Diego</td><td><i> </i></td><td></td></tr><br>
<br>
<br>
_______________________________________________<br>
libcxx-commits mailing list<br>
<a href="mailto:libcxx-commits@lists.llvm.org" target="_blank">libcxx-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits</a><br>
</blockquote></div>