[libcxx] r289158 - Fix PR27374 - Remove the implicit reduced-arity-extension in tuple.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 8 15:57:09 PST 2016


Author: ericwf
Date: Thu Dec  8 17:57:08 2016
New Revision: 289158

URL: http://llvm.org/viewvc/llvm-project?rev=289158&view=rev
Log:
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<std::string, int>);
    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.

Added:
    libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/
    libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/disable_reduced_arity_initialization_extension.pass.cpp
    libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/enable_reduced_arity_initialization_extension.pass.cpp
Modified:
    libcxx/trunk/docs/UsingLibcxx.rst
    libcxx/trunk/include/tuple
    libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp

Modified: libcxx/trunk/docs/UsingLibcxx.rst
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/UsingLibcxx.rst?rev=289158&r1=289157&r2=289158&view=diff
==============================================================================
--- libcxx/trunk/docs/UsingLibcxx.rst (original)
+++ libcxx/trunk/docs/UsingLibcxx.rst Thu Dec  8 17:57:08 2016
@@ -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<std::string, int, std::error_code> 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 <http://llvm.org/PR27374>`_ 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<std::string, int, std::error_code>;
+      return Tup{"hello world", 42}; // explicit constructor called. OK.
+    }

Modified: libcxx/trunk/include/tuple
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/tuple?rev=289158&r1=289157&r2=289158&view=diff
==============================================================================
--- libcxx/trunk/include/tuple (original)
+++ libcxx/trunk/include/tuple Thu Dec  8 17:57:08 2016
@@ -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 <class ..._Args>
     struct _PackExpandsToThisTuple : false_type {};
 
@@ -703,11 +709,17 @@ public:
                ) {}
 
     template <class ..._Up,
+              bool _PackIsTuple = _PackExpandsToThisTuple<_Up...>::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
              >

Added: libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/disable_reduced_arity_initialization_extension.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/disable_reduced_arity_initialization_extension.pass.cpp?rev=289158&view=auto
==============================================================================
--- libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/disable_reduced_arity_initialization_extension.pass.cpp (added)
+++ libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/disable_reduced_arity_initialization_extension.pass.cpp Thu Dec  8 17:57:08 2016
@@ -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.
+//
+//===----------------------------------------------------------------------===//
+
+// <tuple>
+
+// template <class... Types> class tuple;
+
+// template <class... UTypes>
+//   explicit tuple(UTypes&&... u);
+
+// UNSUPPORTED: c++98, c++03
+
+#include <tuple>
+#include <cassert>
+#include <type_traits>
+#include <string>
+#include <system_error>
+
+#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<MO, ND> Tuple;
+        static_assert(!std::is_constructible<Tuple, MO>::value, "");
+        static_assert(std::is_constructible<Tuple, MO, ND>::value, "");
+        static_assert(test_convertible<Tuple, MO, ND>(), "");
+    }
+    {
+        typedef std::tuple<MO, MO, ND> Tuple;
+        static_assert(!std::is_constructible<Tuple, MO, MO>::value, "");
+        static_assert(std::is_constructible<Tuple, MO, MO, ND>::value, "");
+        static_assert(test_convertible<Tuple, MO, MO, ND>(), "");
+    }
+    {
+        // Same idea as above but with a nested tuple type.
+        typedef std::tuple<MO, ND> Tuple;
+        typedef std::tuple<MO, Tuple, MO, MO> 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<std::string, int, std::error_code>;
+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<E, E, E>;
+        // Test that the reduced arity initialization extension is only
+        // allowed on the explicit constructor.
+        static_assert(test_convertible<Tup, E, E, E>(), "");
+
+        Tup t(E(0), E(1));
+        static_assert(std::is_constructible<Tup, E, E>::value, "");
+        static_assert(!test_convertible<Tup, E, E>(), "");
+        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<Tup, E>::value, "");
+        static_assert(!test_convertible<Tup, E>(), "");
+        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();
+}

Added: libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/enable_reduced_arity_initialization_extension.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/enable_reduced_arity_initialization_extension.pass.cpp?rev=289158&view=auto
==============================================================================
--- libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/enable_reduced_arity_initialization_extension.pass.cpp (added)
+++ libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/enable_reduced_arity_initialization_extension.pass.cpp Thu Dec  8 17:57:08 2016
@@ -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.
+//
+//===----------------------------------------------------------------------===//
+
+// <tuple>
+
+// template <class... Types> class tuple;
+
+// template <class... UTypes>
+//   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 <tuple>
+#include <cassert>
+#include <type_traits>
+#include <string>
+#include <system_error>
+
+#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<MO, ND> Tuple;
+        static_assert(!std::is_constructible<Tuple, MO>::value, "");
+        static_assert(std::is_constructible<Tuple, MO, ND>::value, "");
+        static_assert(test_convertible<Tuple, MO, ND>(), "");
+    }
+    {
+        typedef std::tuple<MO, MO, ND> Tuple;
+        static_assert(!std::is_constructible<Tuple, MO, MO>::value, "");
+        static_assert(std::is_constructible<Tuple, MO, MO, ND>::value, "");
+        static_assert(test_convertible<Tuple, MO, MO, ND>(), "");
+    }
+    {
+        // Same idea as above but with a nested tuple type.
+        typedef std::tuple<MO, ND> Tuple;
+        typedef std::tuple<MO, Tuple, MO, MO> 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<MO, int> Tuple;
+        typedef std::tuple<MO, Tuple, MO, MO> 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<std::string, int, std::error_code> 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<E, E, E>;
+        static_assert(test_convertible<Tup, E, E, E>(), "");
+
+        Tup t = {E(0), E(1)};
+        static_assert(test_convertible<Tup, E, E>(), "");
+        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<Tup, E>(), "");
+        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();
+}

Modified: libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp?rev=289158&r1=289157&r2=289158&view=diff
==============================================================================
--- libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp (original)
+++ libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/UTypes.pass.cpp Thu Dec  8 17:57:08 2016
@@ -21,6 +21,7 @@
 #include <type_traits>
 
 #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<MoveOnly, MoveOnly, MoveOnly> t(MoveOnly(0),
-                                                   MoveOnly(1));
+        using E = MoveOnly;
+        using Tup = std::tuple<E, E, E>;
+        // Test that the reduced arity initialization extension is only
+        // allowed on the explicit constructor.
+        static_assert(test_convertible<Tup, E, E, E>(), "");
+
+        Tup t(E(0), E(1));
+        static_assert(!test_convertible<Tup, E, E>(), "");
         assert(std::get<0>(t) == 0);
         assert(std::get<1>(t) == 1);
         assert(std::get<2>(t) == MoveOnly());
-    }
-    {
-        std::tuple<MoveOnly, MoveOnly, MoveOnly> t(MoveOnly(0));
+
+        Tup t2(E(0));
+        static_assert(!test_convertible<Tup, E>(), "");
         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




More information about the cfe-commits mailing list