Annotate some Abseil container methods with [[clang::lifetime_capture_by(...)]] This will allow catching some lifetime issues such as the following: ``` absl::flat_hash_set<absl::string_view> s; s.add(std::string(...)); // dangling ``` PiperOrigin-RevId: 714123078 Change-Id: I67f382e97fa8d5e2dc2b58a1ccb8cc013819e4b3
diff --git a/absl/base/attributes.h b/absl/base/attributes.h index 0b94ae4..95b102e 100644 --- a/absl/base/attributes.h +++ b/absl/base/attributes.h
@@ -831,6 +831,17 @@ #define ABSL_ATTRIBUTE_LIFETIME_BOUND #endif +// Internal attribute; name and documentation TBD. +// +// See the upstream documentation: +// https://clang.llvm.org/docs/AttributeReference.html#lifetime_capture_by +#if ABSL_HAVE_CPP_ATTRIBUTE(clang::lifetime_capture_by) +#define ABSL_INTERNAL_ATTRIBUTE_CAPTURED_BY(Owner) \ + [[clang::lifetime_capture_by(Owner)]] +#else +#define ABSL_INTERNAL_ATTRIBUTE_CAPTURED_BY(Owner) +#endif + // ABSL_ATTRIBUTE_VIEW indicates that a type is solely a "view" of data that it // points to, similarly to a span, string_view, or other non-owning reference // type.
diff --git a/absl/container/BUILD.bazel b/absl/container/BUILD.bazel index 776a516..19dc91d 100644 --- a/absl/container/BUILD.bazel +++ b/absl/container/BUILD.bazel
@@ -682,6 +682,7 @@ linkopts = ABSL_DEFAULT_LINKOPTS, deps = [ ":common", + ":common_policy_traits", ":compressed_tuple", ":container_memory", ":hash_function_defaults",
diff --git a/absl/container/CMakeLists.txt b/absl/container/CMakeLists.txt index b9152d2..85a4ce2 100644 --- a/absl/container/CMakeLists.txt +++ b/absl/container/CMakeLists.txt
@@ -749,6 +749,7 @@ ${ABSL_DEFAULT_COPTS} DEPS absl::bits + absl::common_policy_traits absl::compressed_tuple absl::config absl::container_common
diff --git a/absl/container/internal/common_policy_traits.h b/absl/container/internal/common_policy_traits.h index c521f61..bbf5475 100644 --- a/absl/container/internal/common_policy_traits.h +++ b/absl/container/internal/common_policy_traits.h
@@ -28,6 +28,15 @@ ABSL_NAMESPACE_BEGIN namespace container_internal { +template <class Policy, class = void> +struct policy_trait_element_is_owner : std::false_type {}; + +template <class Policy> +struct policy_trait_element_is_owner< + Policy, + std::enable_if_t<!std::is_void<typename Policy::element_is_owner>::value>> + : Policy::element_is_owner {}; + // Defines how slots are initialized/destroyed/moved. template <class Policy, class = void> struct common_policy_traits {
diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 5cfa98b..24f6011 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h
@@ -209,6 +209,7 @@ #include "absl/base/port.h" #include "absl/base/prefetch.h" #include "absl/container/internal/common.h" // IWYU pragma: export // for node_handle +#include "absl/container/internal/common_policy_traits.h" #include "absl/container/internal/compressed_tuple.h" #include "absl/container/internal/container_memory.h" #include "absl/container/internal/hash_function_defaults.h" @@ -2436,23 +2437,14 @@ static_assert(std::is_lvalue_reference<reference>::value, "Policy::element() must return a reference"); - template <typename T> - struct SameAsElementReference - : std::is_same<typename std::remove_cv< - typename std::remove_reference<reference>::type>::type, - typename std::remove_cv< - typename std::remove_reference<T>::type>::type> {}; - // An enabler for insert(T&&): T must be convertible to init_type or be the // same as [cv] value_type [ref]. - // Note: we separate SameAsElementReference into its own type to avoid using - // reference unless we need to. MSVC doesn't seem to like it in some - // cases. template <class T> - using RequiresInsertable = typename std::enable_if< - absl::disjunction<std::is_convertible<T, init_type>, - SameAsElementReference<T>>::value, - int>::type; + using Insertable = absl::disjunction< + std::is_same<absl::remove_cvref_t<reference>, absl::remove_cvref_t<T>>, + std::is_convertible<T, init_type>>; + template <class T> + using IsNotBitField = std::is_pointer<T*>; // RequiresNotInit is a workaround for gcc prior to 7.1. // See https://godbolt.org/g/Y4xsUh. @@ -2463,6 +2455,17 @@ template <class... Ts> using IsDecomposable = IsDecomposable<void, PolicyTraits, Hash, Eq, Ts...>; + template <class T> + using IsDecomposableAndInsertable = + IsDecomposable<std::enable_if_t<Insertable<T>::value, T>>; + + // Evaluates to true if an assignment from the given type would require the + // source object to remain alive for the life of the element. + template <class U> + using IsLifetimeBoundAssignmentFrom = std::conditional_t< + policy_trait_element_is_owner<Policy>::value, std::false_type, + type_traits_internal::IsLifetimeBoundAssignment<init_type, U>>; + public: static_assert(std::is_same<pointer, value_type*>::value, "Allocators with custom pointer types are not supported"); @@ -2706,7 +2709,8 @@ // absl::flat_hash_set<int> a, b{a}; // // RequiresNotInit<T> is a workaround for gcc prior to 7.1. - template <class T, RequiresNotInit<T> = 0, RequiresInsertable<T> = 0> + template <class T, RequiresNotInit<T> = 0, + std::enable_if_t<Insertable<T>::value, int> = 0> raw_hash_set(std::initializer_list<T> init, size_t bucket_count = 0, const hasher& hash = hasher(), const key_equal& eq = key_equal(), const allocator_type& alloc = allocator_type()) @@ -2717,7 +2721,8 @@ const allocator_type& alloc = allocator_type()) : raw_hash_set(init.begin(), init.end(), bucket_count, hash, eq, alloc) {} - template <class T, RequiresNotInit<T> = 0, RequiresInsertable<T> = 0> + template <class T, RequiresNotInit<T> = 0, + std::enable_if_t<Insertable<T>::value, int> = 0> raw_hash_set(std::initializer_list<T> init, size_t bucket_count, const hasher& hash, const allocator_type& alloc) : raw_hash_set(init, bucket_count, hash, key_equal(), alloc) {} @@ -2726,7 +2731,8 @@ const hasher& hash, const allocator_type& alloc) : raw_hash_set(init, bucket_count, hash, key_equal(), alloc) {} - template <class T, RequiresNotInit<T> = 0, RequiresInsertable<T> = 0> + template <class T, RequiresNotInit<T> = 0, + std::enable_if_t<Insertable<T>::value, int> = 0> raw_hash_set(std::initializer_list<T> init, size_t bucket_count, const allocator_type& alloc) : raw_hash_set(init, bucket_count, hasher(), key_equal(), alloc) {} @@ -2735,7 +2741,8 @@ const allocator_type& alloc) : raw_hash_set(init, bucket_count, hasher(), key_equal(), alloc) {} - template <class T, RequiresNotInit<T> = 0, RequiresInsertable<T> = 0> + template <class T, RequiresNotInit<T> = 0, + std::enable_if_t<Insertable<T>::value, int> = 0> raw_hash_set(std::initializer_list<T> init, const allocator_type& alloc) : raw_hash_set(init, 0, hasher(), key_equal(), alloc) {} @@ -2951,15 +2958,26 @@ // // flat_hash_map<std::string, int> m; // m.insert(std::make_pair("abc", 42)); - // TODO(cheshire): A type alias T2 is introduced as a workaround for the nvcc - // bug. - template <class T, RequiresInsertable<T> = 0, class T2 = T, - typename std::enable_if<IsDecomposable<T2>::value, int>::type = 0, - T* = nullptr> + template <class T, + std::enable_if_t<IsDecomposableAndInsertable<T>::value && + IsNotBitField<T>::value && + !IsLifetimeBoundAssignmentFrom<T>::value, + int> = 0> std::pair<iterator, bool> insert(T&& value) ABSL_ATTRIBUTE_LIFETIME_BOUND { return emplace(std::forward<T>(value)); } + template <class T, + std::enable_if_t<IsDecomposableAndInsertable<T>::value && + IsNotBitField<T>::value && + IsLifetimeBoundAssignmentFrom<T>::value, + int> = 0> + std::pair<iterator, bool> insert( + T&& value ABSL_INTERNAL_ATTRIBUTE_CAPTURED_BY(this)) + ABSL_ATTRIBUTE_LIFETIME_BOUND { + return emplace(std::forward<T>(value)); + } + // This overload kicks in when the argument is a bitfield or an lvalue of // insertable and decomposable type. // @@ -2971,13 +2989,23 @@ // const char* p = "hello"; // s.insert(p); // - template < - class T, RequiresInsertable<const T&> = 0, - typename std::enable_if<IsDecomposable<const T&>::value, int>::type = 0> + template <class T, std::enable_if_t< + IsDecomposableAndInsertable<const T&>::value && + !IsLifetimeBoundAssignmentFrom<const T&>::value, + int> = 0> std::pair<iterator, bool> insert(const T& value) ABSL_ATTRIBUTE_LIFETIME_BOUND { return emplace(value); } + template <class T, + std::enable_if_t<IsDecomposableAndInsertable<const T&>::value && + IsLifetimeBoundAssignmentFrom<const T&>::value, + int> = 0> + std::pair<iterator, bool> insert( + const T& value ABSL_INTERNAL_ATTRIBUTE_CAPTURED_BY(this)) + ABSL_ATTRIBUTE_LIFETIME_BOUND { + return emplace(value); + } // This overload kicks in when the argument is an rvalue of init_type. Its // purpose is to handle brace-init-list arguments. @@ -2985,22 +3013,43 @@ // flat_hash_map<std::string, int> s; // s.insert({"abc", 42}); std::pair<iterator, bool> insert(init_type&& value) - ABSL_ATTRIBUTE_LIFETIME_BOUND { + ABSL_ATTRIBUTE_LIFETIME_BOUND +#if __cplusplus >= 202002L + requires(!IsLifetimeBoundAssignmentFrom<init_type>::value) +#endif + { return emplace(std::move(value)); } +#if __cplusplus >= 202002L + std::pair<iterator, bool> insert( + init_type&& value ABSL_INTERNAL_ATTRIBUTE_CAPTURED_BY(this)) + ABSL_ATTRIBUTE_LIFETIME_BOUND + requires(IsLifetimeBoundAssignmentFrom<init_type>::value) + { + return emplace(std::move(value)); + } +#endif - // TODO(cheshire): A type alias T2 is introduced as a workaround for the nvcc - // bug. - template <class T, RequiresInsertable<T> = 0, class T2 = T, - typename std::enable_if<IsDecomposable<T2>::value, int>::type = 0, - T* = nullptr> + template <class T, + std::enable_if_t<IsDecomposableAndInsertable<T>::value && + IsNotBitField<T>::value && + !IsLifetimeBoundAssignmentFrom<T>::value, + int> = 0> iterator insert(const_iterator, T&& value) ABSL_ATTRIBUTE_LIFETIME_BOUND { return insert(std::forward<T>(value)).first; } + template <class T, + std::enable_if_t<IsDecomposableAndInsertable<T>::value && + IsNotBitField<T>::value && + IsLifetimeBoundAssignmentFrom<T>::value, + int> = 0> + iterator insert(const_iterator, T&& value ABSL_INTERNAL_ATTRIBUTE_CAPTURED_BY( + this)) ABSL_ATTRIBUTE_LIFETIME_BOUND { + return insert(std::forward<T>(value)).first; + } - template < - class T, RequiresInsertable<const T&> = 0, - typename std::enable_if<IsDecomposable<const T&>::value, int>::type = 0> + template <class T, std::enable_if_t< + IsDecomposableAndInsertable<const T&>::value, int> = 0> iterator insert(const_iterator, const T& value) ABSL_ATTRIBUTE_LIFETIME_BOUND { return insert(value).first; @@ -3016,7 +3065,8 @@ for (; first != last; ++first) emplace(*first); } - template <class T, RequiresNotInit<T> = 0, RequiresInsertable<const T&> = 0> + template <class T, RequiresNotInit<T> = 0, + std::enable_if_t<Insertable<const T&>::value, int> = 0> void insert(std::initializer_list<T> ilist) { insert(ilist.begin(), ilist.end()); } @@ -3055,8 +3105,8 @@ // flat_hash_map<std::string, std::string> m = {{"abc", "def"}}; // // Creates no std::string copies and makes no heap allocations. // m.emplace("abc", "xyz"); - template <class... Args, typename std::enable_if< - IsDecomposable<Args...>::value, int>::type = 0> + template <class... Args, + std::enable_if_t<IsDecomposable<Args...>::value, int> = 0> std::pair<iterator, bool> emplace(Args&&... args) ABSL_ATTRIBUTE_LIFETIME_BOUND { return PolicyTraits::apply(EmplaceDecomposable{*this}, @@ -3066,8 +3116,8 @@ // This overload kicks in if we cannot deduce the key from args. It constructs // value_type unconditionally and then either moves it into the table or // destroys. - template <class... Args, typename std::enable_if< - !IsDecomposable<Args...>::value, int>::type = 0> + template <class... Args, + std::enable_if_t<!IsDecomposable<Args...>::value, int> = 0> std::pair<iterator, bool> emplace(Args&&... args) ABSL_ATTRIBUTE_LIFETIME_BOUND { alignas(slot_type) unsigned char raw[sizeof(slot_type)]; @@ -3259,9 +3309,8 @@ return node; } - template < - class K = key_type, - typename std::enable_if<!std::is_same<K, iterator>::value, int>::type = 0> + template <class K = key_type, + std::enable_if_t<!std::is_same<K, iterator>::value, int> = 0> node_type extract(const key_arg<K>& key) { auto it = find(key); return it == end() ? node_type() : extract(const_iterator{it});
diff --git a/absl/meta/type_traits.h b/absl/meta/type_traits.h index d52c49e..02da067 100644 --- a/absl/meta/type_traits.h +++ b/absl/meta/type_traits.h
@@ -658,9 +658,9 @@ // Until then, we consider an assignment from an "owner" (such as std::string) // to a "view" (such as std::string_view) to be a lifetime-bound assignment. template <typename T, typename U> -using IsLifetimeBoundAssignment = - std::integral_constant<bool, IsView<absl::remove_cvref_t<T>>::value && - IsOwner<absl::remove_cvref_t<U>>::value>; +using IsLifetimeBoundAssignment = absl::conjunction< + std::integral_constant<bool, !std::is_lvalue_reference<U>::value>, + IsOwner<absl::remove_cvref_t<U>>, IsView<absl::remove_cvref_t<T>>>; } // namespace type_traits_internal