From bd688258baf97e840c2bd0c4c307503042b1620c Mon Sep 17 00:00:00 2001 From: Eric Fiselier Date: Thu, 8 Dec 2016 23:57:08 +0000 Subject: [PATCH] Fix PR27374 - Remove the implicit reduced-arity-extension in tuple. This patch removes libc++'s tuple extension which allowed it to be constructed from fewer initializers than elements; with the remaining elements being default constructed. However the implicit version of this extension breaks conforming code. For example: int fun(std::string); int fun(std::tuple); int x = fun("hello"); // ambigious Because existing code may already depend on this extension it can be re-enabled by defining _LIBCPP_ENABLE_TUPLE_IMPLICIT_REDUCED_ARITY_EXTENSION. Note that the explicit version of this extension is still supported, although it's somewhat less useful than the implicit one. llvm-svn: 289158 --- libcxx/docs/UsingLibcxx.rst | 27 +++++ libcxx/include/tuple | 23 +++- ...reduced_arity_initialization_extension.pass.cpp | 108 +++++++++++++++++++ ...reduced_arity_initialization_extension.pass.cpp | 117 +++++++++++++++++++++ .../tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp | 21 ++-- 5 files changed, 286 insertions(+), 10 deletions(-) create mode 100644 libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/disable_reduced_arity_initialization_extension.pass.cpp create mode 100644 libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/enable_reduced_arity_initialization_extension.pass.cpp diff --git a/libcxx/docs/UsingLibcxx.rst b/libcxx/docs/UsingLibcxx.rst index 1d2cc9e..1fd1d49 100644 --- a/libcxx/docs/UsingLibcxx.rst +++ b/libcxx/docs/UsingLibcxx.rst @@ -155,3 +155,30 @@ thread safety annotations. Defining this macro and then building libc++ with hidden visibility gives a build of libc++ which does not export any symbols, which can be useful when building statically for inclusion into another library. + +**_LIBCPP_ENABLE_TUPLE_IMPLICIT_REDUCED_ARITY_EXTENSION**: + This macro is used to re-enable an extension in `std::tuple` which allowed + it to be implicitly constructed from fewer initializers than contained + elements. Elements without an initializer are default constructed. For example: + + .. code-block:: cpp + + std::tuple foo() { + return {"hello world", 42}; // default constructs error_code + } + + + Since libc++ 4.0 this extension has been disabled by default. This macro + may be defined to re-enable it in order to support existing code that depends + on the extension. New use of this extension should be discouraged. + See `PR 27374 `_ for more information. + + Note: The "reduced-arity-initialization" extension is still offered but only + for explicit conversions. Example: + + .. code-block:: cpp + + auto foo() { + using Tup = std::tuple; + return Tup{"hello world", 42}; // explicit constructor called. OK. + } diff --git a/libcxx/include/tuple b/libcxx/include/tuple index 251443e..553d8e5 100644 --- a/libcxx/include/tuple +++ b/libcxx/include/tuple @@ -477,6 +477,12 @@ class _LIBCPP_TYPE_VIS_ONLY tuple base base_; +#if defined(_LIBCPP_ENABLE_TUPLE_IMPLICIT_REDUCED_ARITY_EXTENSION) + static constexpr bool _EnableImplicitReducedArityExtension = true; +#else + static constexpr bool _EnableImplicitReducedArityExtension = false; +#endif + template struct _PackExpandsToThisTuple : false_type {}; @@ -703,11 +709,17 @@ public: ) {} template ::value, typename enable_if < _CheckArgsConstructor< - sizeof...(_Up) <= sizeof...(_Tp) - && !_PackExpandsToThisTuple<_Up...>::value + sizeof...(_Up) == sizeof...(_Tp) + && !_PackIsTuple + >::template __enable_implicit<_Up...>() || + _CheckArgsConstructor< + _EnableImplicitReducedArityExtension + && sizeof...(_Up) < sizeof...(_Tp) + && !_PackIsTuple >::template __enable_implicit<_Up...>(), bool >::type = false @@ -735,7 +747,12 @@ public: _CheckArgsConstructor< sizeof...(_Up) <= sizeof...(_Tp) && !_PackExpandsToThisTuple<_Up...>::value - >::template __enable_explicit<_Up...>(), + >::template __enable_explicit<_Up...>() || + _CheckArgsConstructor< + !_EnableImplicitReducedArityExtension + && sizeof...(_Up) < sizeof...(_Tp) + && !_PackExpandsToThisTuple<_Up...>() + >::template __enable_implicit<_Up...>(), bool >::type = false > diff --git a/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/disable_reduced_arity_initialization_extension.pass.cpp b/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/disable_reduced_arity_initialization_extension.pass.cpp new file mode 100644 index 0000000..4808c51 --- /dev/null +++ b/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/disable_reduced_arity_initialization_extension.pass.cpp @@ -0,0 +1,108 @@ +//===----------------------------------------------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +// + +// template class tuple; + +// template +// explicit tuple(UTypes&&... u); + +// UNSUPPORTED: c++98, c++03 + +#include +#include +#include +#include +#include + +#include "test_macros.h" +#include "test_convertible.hpp" +#include "MoveOnly.h" + +#if defined(_LIBCPP_ENABLE_TUPLE_IMPLICIT_REDUCED_ARITY_EXTENSION) +#error This macro should not be defined by default +#endif + +struct NoDefault { NoDefault() = delete; }; + + +// Make sure the _Up... constructor SFINAEs out when the types that +// are not explicitly initialized are not all default constructible. +// Otherwise, std::is_constructible would return true but instantiating +// the constructor would fail. +void test_default_constructible_extension_sfinae() +{ + typedef MoveOnly MO; + typedef NoDefault ND; + { + typedef std::tuple Tuple; + static_assert(!std::is_constructible::value, ""); + static_assert(std::is_constructible::value, ""); + static_assert(test_convertible(), ""); + } + { + typedef std::tuple Tuple; + static_assert(!std::is_constructible::value, ""); + static_assert(std::is_constructible::value, ""); + static_assert(test_convertible(), ""); + } + { + // Same idea as above but with a nested tuple type. + typedef std::tuple Tuple; + typedef std::tuple NestedTuple; + + static_assert(!std::is_constructible< + NestedTuple, MO, MO, MO, MO>::value, ""); + static_assert(std::is_constructible< + NestedTuple, MO, Tuple, MO, MO>::value, ""); + } +} + +using ExplicitTup = std::tuple; +ExplicitTup doc_example() { + return ExplicitTup{"hello world", 42}; // explicit constructor called. OK. +} + +// Test that the example given in UsingLibcxx.rst actually works. +void test_example_from_docs() { + auto tup = doc_example(); + assert(std::get<0>(tup) == "hello world"); + assert(std::get<1>(tup) == 42); + assert(std::get<2>(tup) == std::error_code{}); +} + +int main() +{ + { + using E = MoveOnly; + using Tup = std::tuple; + // Test that the reduced arity initialization extension is only + // allowed on the explicit constructor. + static_assert(test_convertible(), ""); + + Tup t(E(0), E(1)); + static_assert(std::is_constructible::value, ""); + static_assert(!test_convertible(), ""); + assert(std::get<0>(t) == 0); + assert(std::get<1>(t) == 1); + assert(std::get<2>(t) == MoveOnly()); + + Tup t2(E(0)); + static_assert(std::is_constructible::value, ""); + static_assert(!test_convertible(), ""); + assert(std::get<0>(t) == 0); + assert(std::get<1>(t) == E()); + assert(std::get<2>(t) == E()); + } + // Check that SFINAE is properly applied with the default reduced arity + // constructor extensions. + test_default_constructible_extension_sfinae(); + test_example_from_docs(); +} diff --git a/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/enable_reduced_arity_initialization_extension.pass.cpp b/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/enable_reduced_arity_initialization_extension.pass.cpp new file mode 100644 index 0000000..99b6eb7 --- /dev/null +++ b/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/enable_reduced_arity_initialization_extension.pass.cpp @@ -0,0 +1,117 @@ +//===----------------------------------------------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +// + +// template class tuple; + +// template +// explicit tuple(UTypes&&... u); + +// UNSUPPORTED: c++98, c++03 + +// MODULES_DEFINES: _LIBCPP_ENABLE_TUPLE_IMPLICIT_REDUCED_ARITY_EXTENSION +#define _LIBCPP_ENABLE_TUPLE_IMPLICIT_REDUCED_ARITY_EXTENSION +#include +#include +#include +#include +#include + +#include "test_macros.h" +#include "test_convertible.hpp" +#include "MoveOnly.h" + + +struct NoDefault { NoDefault() = delete; }; + + +// Make sure the _Up... constructor SFINAEs out when the types that +// are not explicitly initialized are not all default constructible. +// Otherwise, std::is_constructible would return true but instantiating +// the constructor would fail. +void test_default_constructible_extension_sfinae() +{ + typedef MoveOnly MO; + typedef NoDefault ND; + { + typedef std::tuple Tuple; + static_assert(!std::is_constructible::value, ""); + static_assert(std::is_constructible::value, ""); + static_assert(test_convertible(), ""); + } + { + typedef std::tuple Tuple; + static_assert(!std::is_constructible::value, ""); + static_assert(std::is_constructible::value, ""); + static_assert(test_convertible(), ""); + } + { + // Same idea as above but with a nested tuple type. + typedef std::tuple Tuple; + typedef std::tuple NestedTuple; + + static_assert(!std::is_constructible< + NestedTuple, MO, MO, MO, MO>::value, ""); + static_assert(std::is_constructible< + NestedTuple, MO, Tuple, MO, MO>::value, ""); + } + { + typedef std::tuple Tuple; + typedef std::tuple NestedTuple; + + static_assert(std::is_constructible< + NestedTuple, MO, MO, MO, MO>::value, ""); + static_assert(test_convertible< + NestedTuple, MO, MO, MO, MO>(), ""); + + static_assert(std::is_constructible< + NestedTuple, MO, Tuple, MO, MO>::value, ""); + static_assert(test_convertible< + NestedTuple, MO, Tuple, MO, MO>(), ""); + } +} + +std::tuple doc_example() { + return {"hello world", 42}; +} + +// Test that the example given in UsingLibcxx.rst actually works. +void test_example_from_docs() { + auto tup = doc_example(); + assert(std::get<0>(tup) == "hello world"); + assert(std::get<1>(tup) == 42); + assert(std::get<2>(tup) == std::error_code{}); +} + +int main() +{ + + { + using E = MoveOnly; + using Tup = std::tuple; + static_assert(test_convertible(), ""); + + Tup t = {E(0), E(1)}; + static_assert(test_convertible(), ""); + assert(std::get<0>(t) == 0); + assert(std::get<1>(t) == 1); + assert(std::get<2>(t) == MoveOnly()); + + Tup t2 = {E(0)}; + static_assert(test_convertible(), ""); + assert(std::get<0>(t) == 0); + assert(std::get<1>(t) == E()); + assert(std::get<2>(t) == E()); + } + // Check that SFINAE is properly applied with the default reduced arity + // constructor extensions. + test_default_constructible_extension_sfinae(); + test_example_from_docs(); +} diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp index 9adbc54..06284df 100644 --- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp +++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp @@ -21,6 +21,7 @@ #include #include "test_macros.h" +#include "test_convertible.hpp" #include "MoveOnly.h" #if TEST_STD_VER > 11 @@ -124,17 +125,23 @@ int main() // extensions #ifdef _LIBCPP_VERSION { - std::tuple t(MoveOnly(0), - MoveOnly(1)); + using E = MoveOnly; + using Tup = std::tuple; + // Test that the reduced arity initialization extension is only + // allowed on the explicit constructor. + static_assert(test_convertible(), ""); + + Tup t(E(0), E(1)); + static_assert(!test_convertible(), ""); assert(std::get<0>(t) == 0); assert(std::get<1>(t) == 1); assert(std::get<2>(t) == MoveOnly()); - } - { - std::tuple t(MoveOnly(0)); + + Tup t2(E(0)); + static_assert(!test_convertible(), ""); assert(std::get<0>(t) == 0); - assert(std::get<1>(t) == MoveOnly()); - assert(std::get<2>(t) == MoveOnly()); + assert(std::get<1>(t) == E()); + assert(std::get<2>(t) == E()); } #endif #if TEST_STD_VER > 11 -- 2.7.4