EricWF created this revision.
EricWF added reviewers: rsmith, mclow.lists.

See https://bugs.llvm.org/show_bug.cgi?id=20855

Libc++ goes out of it's way to diagnose `std::tuple` constructions which are UB 
due to lifetime bugs caused by reference creation. For example:

  // The 'const std::string&' is created *inside* the tuple constructor, and 
its lifetime is over before the end of the constructor call.
  std::tuple<int, const std::string&> t(std::make_tuple(42, "abc"));

However, we are over-aggressive and we incorrectly diagnose cases such as:

  void foo(std::tuple<int const&, int const&> const&);
  foo(std::make_tuple(42, 42));

This patch fixes the incorrectly diagnosed cases, as well as converting the 
diagnostic to use the newly added Clang trait `__reference_binds_to_temporary`. 
The new trait allows us to diagnose cases we previously couldn't such as:

  std::tuple<int, const std::string&> t(42, "abc");


https://reviews.llvm.org/D41977

Files:
  include/tuple
  
test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp
  
test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp

Index: test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp
===================================================================
--- /dev/null
+++ test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp
@@ -0,0 +1,47 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     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.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+// <tuple>
+
+// See llvm.org/PR20855
+
+#include <tuple>
+#include <string>
+#include "test_macros.h"
+
+#if TEST_HAS_BUILTIN_IDENTIFIER(__reference_binds_to_temporary)
+# define ASSERT_REFERENCE_BINDS_TEMPORARY(...) static_assert(__reference_binds_to_temporary(__VA_ARGS__), "")
+# define ASSERT_NOT_REFERENCE_BINDS_TEMPORARY(...) static_assert(!__reference_binds_to_temporary(__VA_ARGS__), "")
+#else
+# define ASSERT_REFERENCE_BINDS_TEMPORARY(...) static_assert(true, "")
+# define ASSERT_NOT_REFERENCE_BINDS_TEMPORARY(...) static_assert(true, "")
+#endif
+
+static_assert(std::is_same<decltype("abc"), decltype(("abc"))>::value, "");
+ASSERT_REFERENCE_BINDS_TEMPORARY(std::string const&, decltype("abc"));
+ASSERT_REFERENCE_BINDS_TEMPORARY(std::string const&, decltype(("abc")));
+ASSERT_REFERENCE_BINDS_TEMPORARY(std::string const&, const char*&&);
+
+template <class ...Args, class Tup = std::tuple<Args...>>
+void F(Tup const&) {}
+
+int main() {
+  {
+    F<int, int const&>(std::make_tuple<const int&, const int&>(42, 42));
+    std::tuple<int, int const&> t(std::make_tuple<const int&, const int&>(42, 42));
+  }
+  {
+    auto fn = &F<int, std::string const&>;
+    fn(std::tuple<int, std::string const&>(42, std::string("a")));
+    fn(std::make_tuple(42, std::string("a")));
+  }
+}
Index: test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp
===================================================================
--- /dev/null
+++ test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp
@@ -0,0 +1,54 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+//                     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.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+// <tuple>
+
+// See llvm.org/PR20855
+
+#ifdef __clang__
+#pragma clang diagnostic ignored "-Wdangling-field"
+#endif
+
+#include <tuple>
+#include <string>
+#include "test_macros.h"
+
+
+#if !TEST_HAS_BUILTIN_IDENTIFIER(__reference_binds_to_temporary)
+#else
+# define ASSERT_REFERENCE_BINDS_TEMPORARY(...) static_assert(true, "")
+# define ASSERT_NOT_REFERENCE_BINDS_TEMPORARY(...) static_assert(true, "")
+#endif
+
+template <class T> struct CannotDeduce {
+ using type = T;
+};
+
+template <class ...Args>
+void F(typename CannotDeduce<std::tuple<Args...>>::type const&) {}
+
+
+int main() {
+  // expected-error@tuple:* 1 {{"Attempted to construct a reference element in a tuple with an rvalue"}}
+  {
+    F<int, const std::string&>(std::make_tuple(1, "abc")); // expected-note 1 {{requested here}}
+  }
+#if TEST_HAS_BUILTIN_IDENTIFIER(__reference_binds_to_temporary)
+  // expected-error@tuple:* 2 {{"Attempted to construct a reference element in a tuple with an rvalue"}}
+  {
+    std::tuple<int, const std::string&> t(1, "a"); // expected-note 1 {{requested here}}
+  }
+  {
+    F<int, const std::string&>(std::tuple<int, const std::string&>(1, "abc")); // expected-note 1 {{requested here}}
+  }
+#endif
+}
Index: include/tuple
===================================================================
--- include/tuple
+++ include/tuple
@@ -173,16 +173,24 @@
 
     template <class _Tp>
     static constexpr bool __can_bind_reference() {
+#if !__has_keyword(__reference_binds_to_temporary)
         using _RawTp = typename remove_reference<_Tp>::type;
         using _RawHp = typename remove_reference<_Hp>::type;
+
         using _CheckLValueArg = integral_constant<bool,
             is_lvalue_reference<_Tp>::value
         ||  is_same<_RawTp, reference_wrapper<_RawHp>>::value
         ||  is_same<_RawTp, reference_wrapper<typename remove_const<_RawHp>::type>>::value
+        // Allow "int&&" to bind to 'int const&'
+        ||  (is_rvalue_reference<_Tp>::value && is_const<_RawHp>::value &&
+            is_same<_RawHp, const _RawTp>::value)
         >;
         return  !is_reference<_Hp>::value
             || (is_lvalue_reference<_Hp>::value && _CheckLValueArg::value)
             || (is_rvalue_reference<_Hp>::value && !is_lvalue_reference<_Tp>::value);
+#else
+      return !__reference_binds_to_temporary(_Hp, _Tp);
+#endif
     }
 
     __tuple_leaf& operator=(const __tuple_leaf&);
@@ -224,14 +232,14 @@
         _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
         explicit __tuple_leaf(_Tp&& __t) _NOEXCEPT_((is_nothrow_constructible<_Hp, _Tp>::value))
             : __value_(_VSTD::forward<_Tp>(__t))
-        {static_assert(__can_bind_reference<_Tp>(),
+        {static_assert(__can_bind_reference<_Tp&&>(),
        "Attempted to construct a reference element in a tuple with an rvalue");}
 
     template <class _Tp, class _Alloc>
         _LIBCPP_INLINE_VISIBILITY
         explicit __tuple_leaf(integral_constant<int, 0>, const _Alloc&, _Tp&& __t)
             : __value_(_VSTD::forward<_Tp>(__t))
-        {static_assert(__can_bind_reference<_Tp>(),
+        {static_assert(__can_bind_reference<_Tp&&>(),
        "Attempted to construct a reference element in a tuple with an rvalue");}
 
     template <class _Tp, class _Alloc>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to