[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 15:08:52 PST 2018
Looking.
On Wed, Jan 24, 2018 at 3:52 PM, Richard Trieu <rtrieu at google.com> wrote:
> Hi Eric,
>
> I am getting a build failure after this revision:
>
> llvm/projects/libcxx/include/tuple:175:27: error: no return statement in
> constexpr function
> static constexpr bool __can_bind_reference() {
> ^
> 1 error generated.
>
> It looks like if the #if in __can_bind_reference is false, the function
> will be empty. Can you take a look?
>
> Richard
>
> On Wed, Jan 24, 2018 at 2:14 PM, Eric Fiselier via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> 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.c
>> nstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp
>> libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnst
>> r/PR20855_tuple_ref_binding_diagnostics.pass.cpp
>> Removed:
>> libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnos
>> e_reference_binding.fail.cpp
>> libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/diagnos
>> e_reference_binding.pass.cpp
>> Modified:
>> libcxx/trunk/include/tuple
>>
>> Modified: libcxx/trunk/include/tuple
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/tup
>> le?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/diagnos
>> e_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/diagnos
>> e_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.c
>> nstr/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_b
>> inding_diagnostics.fail.cpp?rev=323380&view=auto
>> ============================================================
>> ==================
>> --- libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.c
>> nstr/PR20855_tuple_ref_binding_diagnostics.fail.cpp (added)
>> +++ libcxx/trunk/test/libcxx/utilities/tuple/tuple.tuple/tuple.c
>> nstr/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.cnst
>> r/PR20855_tuple_ref_binding_diagnostics.pass.cpp
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/ut
>> ilities/tuple/tuple.tuple/tuple.cnstr/PR20855_tuple_ref_bind
>> ing_diagnostics.pass.cpp?rev=323380&view=auto
>> ============================================================
>> ==================
>> --- libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnst
>> r/PR20855_tuple_ref_binding_diagnostics.pass.cpp (added)
>> +++ libcxx/trunk/test/std/utilities/tuple/tuple.tuple/tuple.cnst
>> r/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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180124/14d19df0/attachment-0001.html>
More information about the cfe-commits
mailing list