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