diff options
author | Eric Fiselier <eric@efcs.ca> | 2018-01-24 22:14:01 +0000 |
---|---|---|
committer | Eric Fiselier <eric@efcs.ca> | 2018-01-24 22:14:01 +0000 |
commit | 292617e7003efdbc4dd84bc49423dc662f856fb6 (patch) | |
tree | 653c72b1f4c5e7b84c7842f812cc0ebeb69a6dec | |
parent | 67abf5396124384be2ad484abd8660bb10dcd94d (diff) | |
download | bcm5719-llvm-292617e7003efdbc4dd84bc49423dc662f856fb6.tar.gz bcm5719-llvm-292617e7003efdbc4dd84bc49423dc662f856fb6.zip |
[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
llvm-svn: 323380
-rw-r--r-- | libcxx/include/tuple | 21 | ||||
-rw-r--r-- | libcxx/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.fail.cpp | 40 | ||||
-rw-r--r-- | libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp | 80 | ||||
-rw-r--r-- | libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp (renamed from libcxx/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.pass.cpp) | 79 |
4 files changed, 159 insertions, 61 deletions
diff --git a/libcxx/include/tuple b/libcxx/include/tuple index 31578d1557a..1accb9c67a7 100644 --- a/libcxx/include/tuple +++ b/libcxx/include/tuple @@ -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 diff --git a/libcxx/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.fail.cpp b/libcxx/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.fail.cpp deleted file mode 100644 index 1cc3c73d10f..00000000000 --- a/libcxx/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.fail.cpp +++ /dev/null @@ -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@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@tuple:* 0+ {{binding reference member '__value_' to a temporary value}} - // expected-error@tuple:* 0+ {{binding reference member '__value_' to a temporary value}} - } -} diff --git a/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp b/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp new file mode 100644 index 00000000000..3300f7e831a --- /dev/null +++ b/libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp @@ -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@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@-1 {{force failure}} +#endif +} diff --git a/libcxx/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp index 78446976431..4680635441d 100644 --- a/libcxx/test/libcxx/utilities/tuple/tuple.tuple/diagnose_reference_binding.pass.cpp +++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_binding_diagnostics.pass.cpp @@ -1,3 +1,4 @@ +// -*- C++ -*- //===----------------------------------------------------------------------===// // // The LLVM Compiler Infrastructure @@ -11,21 +12,79 @@ // <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. +// See llvm.org/PR20855 #include <tuple> #include <string> -#include <functional> -#include <cassert> +#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; +}; -int main() { +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; { @@ -69,3 +128,9 @@ int main() { assert(&std::get<0>(t4) == &x); } } + + +int main() { + compile_tests(); + allocator_tests(); +} |