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@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}} - } -} 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@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 +} 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(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
