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