[libcxx] r323380 - [libc++] Fix PR20855 -- libc++ incorrectly diagnoses illegal reference binding in std::tuple.
Eric Fiselier via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 24 14:14:01 PST 2018
Author: ericwf
Date: Wed Jan 24 14:14:01 2018
New Revision: 323380
URL: http://llvm.org/viewvc/llvm-project?rev=323380&view=rev
Log:
[libc++] Fix PR20855 -- libc++ incorrectly diagnoses illegal reference binding in std::tuple.
Summary:
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");
```
Reviewers: rsmith, mclow.lists
Reviewed By: rsmith
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D41977
Added:
libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp
libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp
Removed:
libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.fail.cpp
libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.pass.cpp
Modified:
libcxx/trunk/include/tuple
Modified: libcxx/trunk/include/tuple
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/tuple?rev=323380&r1=323379&r2=323380&view=diff
==============================================================================
--- libcxx/trunk/include/tuple (original)
+++ libcxx/trunk/include/tuple Wed Jan 24 14:14:01 2018
@@ -173,16 +173,9 @@ class __tuple_leaf
template <class _Tp>
static constexpr bool __can_bind_reference() {
- 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
- >;
- return !is_reference<_Hp>::value
- || (is_lvalue_reference<_Hp>::value && _CheckLValueArg::value)
- || (is_rvalue_reference<_Hp>::value && !is_lvalue_reference<_Tp>::value);
+#if __has_keyword(__reference_binds_to_temporary)
+ return !__reference_binds_to_temporary(_Hp, _Tp);
+#endif
}
__tuple_leaf& operator=(const __tuple_leaf&);
@@ -224,15 +217,15 @@ public:
_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>(),
- "Attempted to construct a reference element in a tuple with an rvalue");}
+ {static_assert(__can_bind_reference<_Tp&&>(),
+ "Attempted construction of reference element binds to a temporary whose lifetime has ended");}
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>(),
- "Attempted to construct a reference element in a tuple with an rvalue");}
+ {static_assert(__can_bind_reference<_Tp&&>(),
+ "Attempted construction of reference element binds to a temporary whose lifetime has ended");}
template <class _Tp, class _Alloc>
_LIBCPP_INLINE_VISIBILITY
Removed: libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.fail.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.fail.cpp?rev=323379&view=auto
==============================================================================
--- libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.fail.cpp (original)
+++ libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.fail.cpp (removed)
@@ -1,40 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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>
-
-// Test the diagnostics libc++ generates for invalid reference binding.
-// Libc++ attempts to diagnose the following cases:
-// * Constructing an lvalue reference from an rvalue.
-// * Constructing an rvalue reference from an lvalue.
-
-#include <tuple>
-#include <string>
-
-int main() {
- std::allocator<void> alloc;
-
- // expected-error-re at tuple:* 4 {{static_assert failed{{.*}} "Attempted to construct a reference element in a tuple with an rvalue"}}
-
- // bind lvalue to rvalue
- std::tuple<int const&> t(42); // expected-note {{requested here}}
- std::tuple<int const&> t1(std::allocator_arg, alloc, 42); // expected-note {{requested here}}
- // bind rvalue to constructed non-rvalue
- std::tuple<std::string &&> t2("hello"); // expected-note {{requested here}}
- std::tuple<std::string &&> t3(std::allocator_arg, alloc, "hello"); // expected-note {{requested here}}
-
- // FIXME: The below warnings may get emitted as an error, a warning, or not emitted at all
- // depending on the flags used to compile this test.
- {
- // expected-warning at tuple:* 0+ {{binding reference member '__value_' to a temporary value}}
- // expected-error at tuple:* 0+ {{binding reference member '__value_' to a temporary value}}
- }
-}
Removed: libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.pass.cpp?rev=323379&view=auto
==============================================================================
--- libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.pass.cpp (original)
+++ libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.pass.cpp (removed)
@@ -1,71 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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>
-
-// Test the diagnostics libc++ generates for invalid reference binding.
-// Libc++ attempts to diagnose the following cases:
-// * Constructing an lvalue reference from an rvalue.
-// * Constructing an rvalue reference from an lvalue.
-
-#include <tuple>
-#include <string>
-#include <functional>
-#include <cassert>
-
-static_assert(std::is_constructible<int&, std::reference_wrapper<int>>::value, "");
-static_assert(std::is_constructible<int const&, std::reference_wrapper<int>>::value, "");
-
-
-int main() {
- std::allocator<void> alloc;
- int x = 42;
- {
- std::tuple<int&> t(std::ref(x));
- assert(&std::get<0>(t) == &x);
- std::tuple<int&> t1(std::allocator_arg, alloc, std::ref(x));
- assert(&std::get<0>(t1) == &x);
- }
- {
- auto r = std::ref(x);
- auto const& cr = r;
- std::tuple<int&> t(r);
- assert(&std::get<0>(t) == &x);
- std::tuple<int&> t1(cr);
- assert(&std::get<0>(t1) == &x);
- std::tuple<int&> t2(std::allocator_arg, alloc, r);
- assert(&std::get<0>(t2) == &x);
- std::tuple<int&> t3(std::allocator_arg, alloc, cr);
- assert(&std::get<0>(t3) == &x);
- }
- {
- std::tuple<int const&> t(std::ref(x));
- assert(&std::get<0>(t) == &x);
- std::tuple<int const&> t2(std::cref(x));
- assert(&std::get<0>(t2) == &x);
- std::tuple<int const&> t3(std::allocator_arg, alloc, std::ref(x));
- assert(&std::get<0>(t3) == &x);
- std::tuple<int const&> t4(std::allocator_arg, alloc, std::cref(x));
- assert(&std::get<0>(t4) == &x);
- }
- {
- auto r = std::ref(x);
- auto cr = std::cref(x);
- std::tuple<int const&> t(r);
- assert(&std::get<0>(t) == &x);
- std::tuple<int const&> t2(cr);
- assert(&std::get<0>(t2) == &x);
- std::tuple<int const&> t3(std::allocator_arg, alloc, r);
- assert(&std::get<0>(t3) == &x);
- std::tuple<int const&> t4(std::allocator_arg, alloc, cr);
- assert(&std::get<0>(t4) == &x);
- }
-}
Added: libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp?rev=323380&view=auto
==============================================================================
--- libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp (added)
+++ libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp Wed Jan 24 14:14:01 2018
@@ -0,0 +1,80 @@
+// -*- 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"
+
+template <class Tp>
+struct ConvertsTo {
+ using RawTp = typename std::remove_cv< typename std::remove_reference<Tp>::type>::type;
+
+ operator Tp() const {
+ return static_cast<Tp>(value);
+ }
+
+ mutable RawTp value;
+};
+
+struct Base {};
+struct Derived : Base {};
+
+template <class T> struct CannotDeduce {
+ using type = T;
+};
+
+template <class ...Args>
+void F(typename CannotDeduce<std::tuple<Args...>>::type const&) {}
+
+
+int main() {
+#if TEST_HAS_BUILTIN_IDENTIFIER(__reference_binds_to_temporary)
+ // expected-error at tuple:* 8 {{"Attempted construction of reference element binds to a temporary whose lifetime has ended"}}
+ {
+ F<int, const std::string&>(std::make_tuple(1, "abc")); // expected-note 1 {{requested here}}
+ }
+ {
+ 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}}
+ }
+ {
+ ConvertsTo<int&> ct;
+ std::tuple<const long&, int> t(ct, 42); // expected-note {{requested here}}
+ }
+ {
+ ConvertsTo<int> ct;
+ std::tuple<int const&, void*> t(ct, nullptr); // expected-note {{requested here}}
+ }
+ {
+ ConvertsTo<Derived> ct;
+ std::tuple<Base const&, int> t(ct, 42); // expected-note {{requested here}}
+ }
+ {
+ std::allocator<void> alloc;
+ std::tuple<std::string &&> t2("hello"); // expected-note {{requested here}}
+ std::tuple<std::string &&> t3(std::allocator_arg, alloc, "hello"); // expected-note {{requested here}}
+ }
+#else
+#error force failure
+// expected-error at -1 {{force failure}}
+#endif
+}
Added: libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp?rev=323380&view=auto
==============================================================================
--- libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp (added)
+++ libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp Wed Jan 24 14:14:01 2018
@@ -0,0 +1,136 @@
+// -*- 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
+
+template <class Tp>
+struct ConvertsTo {
+ using RawTp = typename std::remove_cv< typename std::remove_reference<Tp>::type>::type;
+
+ operator Tp() const {
+ return static_cast<Tp>(value);
+ }
+
+ mutable RawTp value;
+};
+
+struct Base {};
+struct Derived : Base {};
+
+
+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*&&);
+
+ASSERT_NOT_REFERENCE_BINDS_TEMPORARY(int&, const ConvertsTo<int&>&);
+ASSERT_NOT_REFERENCE_BINDS_TEMPORARY(const int&, ConvertsTo<int&>&);
+ASSERT_NOT_REFERENCE_BINDS_TEMPORARY(Base&, Derived&);
+
+
+static_assert(std::is_constructible<int&, std::reference_wrapper<int>>::value, "");
+static_assert(std::is_constructible<int const&, std::reference_wrapper<int>>::value, "");
+
+template <class T> struct CannotDeduce {
+ using type = T;
+};
+
+template <class ...Args>
+void F(typename CannotDeduce<std::tuple<Args...>>::type const&) {}
+
+void compile_tests() {
+ {
+ F<int, int const&>(std::make_tuple(42, 42));
+ }
+ {
+ 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")));
+ }
+ {
+ Derived d;
+ std::tuple<Base&, Base const&> t(d, d);
+ }
+ {
+ ConvertsTo<int&> ct;
+ std::tuple<int, int&> t(42, ct);
+ }
+}
+
+void allocator_tests() {
+ std::allocator<void> alloc;
+ int x = 42;
+ {
+ std::tuple<int&> t(std::ref(x));
+ assert(&std::get<0>(t) == &x);
+ std::tuple<int&> t1(std::allocator_arg, alloc, std::ref(x));
+ assert(&std::get<0>(t1) == &x);
+ }
+ {
+ auto r = std::ref(x);
+ auto const& cr = r;
+ std::tuple<int&> t(r);
+ assert(&std::get<0>(t) == &x);
+ std::tuple<int&> t1(cr);
+ assert(&std::get<0>(t1) == &x);
+ std::tuple<int&> t2(std::allocator_arg, alloc, r);
+ assert(&std::get<0>(t2) == &x);
+ std::tuple<int&> t3(std::allocator_arg, alloc, cr);
+ assert(&std::get<0>(t3) == &x);
+ }
+ {
+ std::tuple<int const&> t(std::ref(x));
+ assert(&std::get<0>(t) == &x);
+ std::tuple<int const&> t2(std::cref(x));
+ assert(&std::get<0>(t2) == &x);
+ std::tuple<int const&> t3(std::allocator_arg, alloc, std::ref(x));
+ assert(&std::get<0>(t3) == &x);
+ std::tuple<int const&> t4(std::allocator_arg, alloc, std::cref(x));
+ assert(&std::get<0>(t4) == &x);
+ }
+ {
+ auto r = std::ref(x);
+ auto cr = std::cref(x);
+ std::tuple<int const&> t(r);
+ assert(&std::get<0>(t) == &x);
+ std::tuple<int const&> t2(cr);
+ assert(&std::get<0>(t2) == &x);
+ std::tuple<int const&> t3(std::allocator_arg, alloc, r);
+ assert(&std::get<0>(t3) == &x);
+ std::tuple<int const&> t4(std::allocator_arg, alloc, cr);
+ assert(&std::get<0>(t4) == &x);
+ }
+}
+
+
+int main() {
+ compile_tests();
+ allocator_tests();
+}
More information about the cfe-commits
mailing list