[libcxx-commits] [libcxx] r365960 - Add option to disable variant narrowing conversion changes.
Eric Fiselier via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Jul 12 14:32:11 PDT 2019
Author: ericwf
Date: Fri Jul 12 14:32:11 2019
New Revision: 365960
URL: http://llvm.org/viewvc/llvm-project?rev=365960&view=rev
Log:
Add option to disable variant narrowing conversion changes.
The paper P0608R3 - "A sane variant converting constructor" disallows
narrowing conversions in variant. It was meant to address this
surprising problem:
std::variant<std::string, bool> v = "abc";
assert(v.index() == 1); // constructs a bool.
However, it also disables every potentially narrowing conversion. For
example:
variant<unsigned> v = 0; // ill-formed
variant<string, double> v2 = 42; // ill-formed (int -> double narrows)
These latter changes break code. A lot of code. Within Google it broke
on the order of a hundred thousand target with thousands of root causes
responsible for the breakages.
Of the breakages related to the narrowing restrictions, none of them
exposed outstanding bugs. However, the breakages caused by boolean
conversions (~13 root causes), all but one of them were bugs.
For this reasons, I am adding a flag to disable the narrowing conversion
changes but not the boolean conversions one.
One purpose of this flag is to allow users to opt-out of breaking changes
in variant until the offending code can be cleaned up. For non-trivial
variant usages the amount of cleanup may be significant.
This flag is also required to support automated tooling, such as
clang-tidy, that can automatically fix code broken by this change.
In order for clang-tidy to know the correct alternative to construct,
it must know what alternative was being constructed previously, which
means running it over the old version of std::variant.
Because this change breaks so much code, I will be implementing the
aforementioned clang-tidy check in the very near future.
Additionally I'm plan present this new information to the committee so they can
re-consider if this is a breaking change we want to make.
I think libc++ should very seriously consider pulling this change
before the 9.0 release branch is cut. But that's a separate discussion
that I will start on the lists.
For now this is the minimal first step.
Added:
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp
libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp
Removed:
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/test/support/variant_test_helpers.hpp
Modified: libcxx/trunk/include/variant
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/variant?rev=365960&r1=365959&r2=365960&view=diff
==============================================================================
--- libcxx/trunk/include/variant (original)
+++ libcxx/trunk/include/variant Fri Jul 12 14:32:11 2019
@@ -1103,7 +1103,11 @@ struct __overload<_Tp, _Types...> : __ov
template <class _Up>
auto operator()(_Tp, _Up&& __t) const
+#ifndef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
-> decltype(__test({ _VSTD::forward<_Up>(__t) }));
+#else
+ -> __identity<_Tp>;
+#endif
};
template <class _Base, class _Tp>
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=365960&r1=365959&r2=365960&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 Fri Jul 12 14:32:11 2019
@@ -136,7 +136,8 @@ void test_T_assignment_sfinae() {
}
{
using V = std::variant<std::string, float>;
- static_assert(!std::is_assignable<V, int>::value, "no matching operator=");
+ static_assert(std::is_assignable<V, int>::value == VariantAllowsNarrowingConversions,
+ "no matching operator=");
}
{
using V = std::variant<std::unique_ptr<int>, bool>;
@@ -187,6 +188,7 @@ void test_T_assignment_basic() {
assert(v.index() == 1);
assert(std::get<1>(v) == 43);
}
+#ifndef TEST_VARIANT_ALLOWS_NARROWING_CONVERSIONS
{
std::variant<unsigned, long> v;
v = 42;
@@ -196,6 +198,7 @@ void test_T_assignment_basic() {
assert(v.index() == 0);
assert(std::get<0>(v) == 43);
}
+#endif
{
std::variant<std::string, bool> v = true;
v = "bar";
Removed: 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=365959&view=auto
==============================================================================
--- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp (original)
+++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp (removed)
@@ -1,52 +0,0 @@
-// -*- 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 '='}}
-}
Added: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp?rev=365960&view=auto
==============================================================================
--- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp (added)
+++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.assign/conv.pass.cpp Fri Jul 12 14:32:11 2019
@@ -0,0 +1,43 @@
+// -*- 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>
+
+#include "variant_test_helpers.hpp"
+
+int main(int, char**)
+{
+ static_assert(!std::is_assignable<std::variant<int, int>, int>::value, "");
+ static_assert(!std::is_assignable<std::variant<long, long long>, int>::value, "");
+ static_assert(std::is_assignable<std::variant<char>, int>::value == VariantAllowsNarrowingConversions, "");
+
+ static_assert(std::is_assignable<std::variant<std::string, float>, int>::value == VariantAllowsNarrowingConversions, "");
+ static_assert(std::is_assignable<std::variant<std::string, double>, int>::value == VariantAllowsNarrowingConversions, "");
+ static_assert(!std::is_assignable<std::variant<std::string, bool>, int>::value, "");
+
+ static_assert(!std::is_assignable<std::variant<int, bool>, decltype("meow")>::value, "");
+ static_assert(!std::is_assignable<std::variant<int, const bool>, decltype("meow")>::value, "");
+ static_assert(!std::is_assignable<std::variant<int, const volatile bool>, decltype("meow")>::value, "");
+
+ static_assert(!std::is_assignable<std::variant<bool>, std::true_type>::value, "");
+ static_assert(!std::is_assignable<std::variant<bool>, std::unique_ptr<char> >::value, "");
+ static_assert(!std::is_assignable<std::variant<bool>, decltype(nullptr)>::value, "");
+
+}
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=365960&r1=365959&r2=365960&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 Fri Jul 12 14:32:11 2019
@@ -69,7 +69,7 @@ void test_T_ctor_sfinae() {
}
{
using V = std::variant<std::string, float>;
- static_assert(!std::is_constructible<V, int>::value,
+ static_assert(std::is_constructible<V, int>::value == VariantAllowsNarrowingConversions,
"no matching constructor");
}
{
@@ -127,11 +127,13 @@ void test_T_ctor_basic() {
static_assert(v.index() == 1, "");
static_assert(std::get<1>(v) == 42, "");
}
+#ifndef TEST_VARIANT_ALLOWS_NARROWING_CONVERSIONS
{
constexpr std::variant<unsigned, long> v(42);
static_assert(v.index() == 1, "");
static_assert(std::get<1>(v) == 42, "");
}
+#endif
{
std::variant<std::string, bool const> v = "foo";
assert(v.index() == 0);
Removed: 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=365959&view=auto
==============================================================================
--- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp (original)
+++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp (removed)
@@ -1,39 +0,0 @@
-// -*- 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}}
-}
Added: libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp?rev=365960&view=auto
==============================================================================
--- libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp (added)
+++ libcxx/trunk/test/std/utilities/variant/variant.variant/variant.ctor/conv.pass.cpp Fri Jul 12 14:32:11 2019
@@ -0,0 +1,42 @@
+// -*- 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>
+
+#include "variant_test_helpers.hpp"
+
+int main(int, char**)
+{
+ static_assert(!std::is_constructible<std::variant<int, int>, int>::value, "");
+ static_assert(!std::is_constructible<std::variant<long, long long>, int>::value, "");
+ static_assert(std::is_constructible<std::variant<char>, int>::value == VariantAllowsNarrowingConversions, "");
+
+ static_assert(std::is_constructible<std::variant<std::string, float>, int>::value == VariantAllowsNarrowingConversions, "");
+ static_assert(std::is_constructible<std::variant<std::string, double>, int>::value == VariantAllowsNarrowingConversions, "");
+ static_assert(!std::is_constructible<std::variant<std::string, bool>, int>::value, "");
+
+ static_assert(!std::is_constructible<std::variant<int, bool>, decltype("meow")>::value, "");
+ static_assert(!std::is_constructible<std::variant<int, const bool>, decltype("meow")>::value, "");
+ static_assert(!std::is_constructible<std::variant<int, const volatile bool>, decltype("meow")>::value, "");
+
+ static_assert(!std::is_constructible<std::variant<bool>, std::true_type>::value, "");
+ static_assert(!std::is_constructible<std::variant<bool>, std::unique_ptr<char> >::value, "");
+ static_assert(!std::is_constructible<std::variant<bool>, decltype(nullptr)>::value, "");
+
+}
Modified: libcxx/trunk/test/support/variant_test_helpers.hpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/support/variant_test_helpers.hpp?rev=365960&r1=365959&r2=365960&view=diff
==============================================================================
--- libcxx/trunk/test/support/variant_test_helpers.hpp (original)
+++ libcxx/trunk/test/support/variant_test_helpers.hpp Fri Jul 12 14:32:11 2019
@@ -21,6 +21,15 @@
// FIXME: Currently the variant<T&> tests are disabled using this macro.
#define TEST_VARIANT_HAS_NO_REFERENCES
+#ifdef _LIBCPP_ENABLE_NARROWING_CONVERSIONS_IN_VARIANT
+# define TEST_VARIANT_ALLOWS_NARROWING_CONVERSIONS
+#endif
+
+#ifdef TEST_VARIANT_ALLOWS_NARROWING_CONVERSIONS
+constexpr bool VariantAllowsNarrowingConversions = true;
+#else
+constexpr bool VariantAllowsNarrowingConversions = false;
+#endif
#ifndef TEST_HAS_NO_EXCEPTIONS
struct CopyThrows {
More information about the libcxx-commits
mailing list