summaryrefslogtreecommitdiff
path: root/absl/functional
diff options
context:
space:
mode:
authorGravatar Abseil Team <absl-team@google.com>2022-10-11 10:27:17 -0700
committerGravatar Copybara-Service <copybara-worker@google.com>2022-10-11 10:28:00 -0700
commit73789eb0689071b0b02b00cb3daed9f224ff1a10 (patch)
treeb6fec5c27f8d6044655b85b021d5702e5f3b73e8 /absl/functional
parent845610e80b66aa3d834f4d1b401133919bf7fadb (diff)
Detects accidental multiple invocations of AnyInvocable<R(...)&&>::operator()&& by producing an error in debug mode, and clarifies that the behavior is undefined in the general case.
PiperOrigin-RevId: 480392976 Change-Id: I2d4c6f213fa7c8747f125c9735272a8e47b9214b
Diffstat (limited to 'absl/functional')
-rw-r--r--absl/functional/any_invocable.h3
-rw-r--r--absl/functional/any_invocable_test.cc49
-rw-r--r--absl/functional/internal/any_invocable.h24
3 files changed, 64 insertions, 12 deletions
diff --git a/absl/functional/any_invocable.h b/absl/functional/any_invocable.h
index 040418d4..3e783c87 100644
--- a/absl/functional/any_invocable.h
+++ b/absl/functional/any_invocable.h
@@ -148,6 +148,9 @@ ABSL_NAMESPACE_BEGIN
// // rvalue-reference qualified.
// std::move(continuation)(result_of_foo);
// }
+//
+// Attempting to call `absl::AnyInvocable` multiple times in such a case
+// results in undefined behavior.
template <class Sig>
class AnyInvocable : private internal_any_invocable::Impl<Sig> {
private:
diff --git a/absl/functional/any_invocable_test.cc b/absl/functional/any_invocable_test.cc
index dabaae9b..1ed85407 100644
--- a/absl/functional/any_invocable_test.cc
+++ b/absl/functional/any_invocable_test.cc
@@ -16,6 +16,7 @@
#include <cstddef>
#include <initializer_list>
+#include <memory>
#include <numeric>
#include <type_traits>
@@ -1156,9 +1157,6 @@ TYPED_TEST_P(AnyInvTestMovable, ConversionConstructionUserDefinedType) {
EXPECT_TRUE(static_cast<bool>(fun));
EXPECT_EQ(29, TypeParam::ToThisParam(fun)(7, 8, 9).value);
-
- EXPECT_TRUE(static_cast<bool>(fun));
- EXPECT_EQ(38, TypeParam::ToThisParam(fun)(10, 11, 12).value);
}
TYPED_TEST_P(AnyInvTestMovable, ConversionConstructionVoidCovariance) {
@@ -1179,9 +1177,6 @@ TYPED_TEST_P(AnyInvTestMovable, ConversionAssignUserDefinedTypeEmptyLhs) {
EXPECT_TRUE(static_cast<bool>(fun));
EXPECT_EQ(29, TypeParam::ToThisParam(fun)(7, 8, 9).value);
-
- EXPECT_TRUE(static_cast<bool>(fun));
- EXPECT_EQ(38, TypeParam::ToThisParam(fun)(10, 11, 12).value);
}
TYPED_TEST_P(AnyInvTestMovable, ConversionAssignUserDefinedTypeNonemptyLhs) {
@@ -1193,9 +1188,6 @@ TYPED_TEST_P(AnyInvTestMovable, ConversionAssignUserDefinedTypeNonemptyLhs) {
EXPECT_TRUE(static_cast<bool>(fun));
EXPECT_EQ(29, TypeParam::ToThisParam(fun)(7, 8, 9).value);
-
- EXPECT_TRUE(static_cast<bool>(fun));
- EXPECT_EQ(38, TypeParam::ToThisParam(fun)(10, 11, 12).value);
}
TYPED_TEST_P(AnyInvTestMovable, ConversionAssignVoidCovariance) {
@@ -1414,6 +1406,41 @@ TYPED_TEST_P(AnyInvTestRvalue, ConversionAssignReferenceWrapper) {
std::is_assignable<AnyInvType&, std::reference_wrapper<AddType>>::value));
}
+TYPED_TEST_P(AnyInvTestRvalue, NonConstCrashesOnSecondCall) {
+ using AnyInvType = typename TypeParam::AnyInvType;
+ using AddType = typename TypeParam::AddType;
+
+ AnyInvType fun(absl::in_place_type<AddType>, 5);
+
+ EXPECT_TRUE(static_cast<bool>(fun));
+ std::move(fun)(7, 8, 9);
+
+ // Ensure we're still valid
+ EXPECT_TRUE(static_cast<bool>(fun)); // NOLINT(bugprone-use-after-move)
+
+#if !defined(NDEBUG) || ABSL_OPTION_HARDENED == 1
+ EXPECT_DEATH_IF_SUPPORTED(std::move(fun)(7, 8, 9), "");
+#endif
+}
+
+// Ensure that any qualifiers (in particular &&-qualifiers) do not affect
+// when the destructor is actually run.
+TYPED_TEST_P(AnyInvTestRvalue, QualifierIndependentObjectLifetime) {
+ using AnyInvType = typename TypeParam::AnyInvType;
+
+ auto refs = std::make_shared<std::nullptr_t>();
+ {
+ AnyInvType fun([refs](auto&&...) noexcept { return 0; });
+ EXPECT_FALSE(refs.unique());
+
+ std::move(fun)(7, 8, 9);
+
+ // Ensure destructor hasn't run even if rref-qualified
+ EXPECT_FALSE(refs.unique());
+ }
+ EXPECT_TRUE(refs.unique());
+}
+
// NOTE: This test suite originally attempted to enumerate all possible
// combinations of type properties but the build-time started getting too large.
// Instead, it is now assumed that certain parameters are orthogonal and so
@@ -1670,7 +1697,9 @@ INSTANTIATE_TYPED_TEST_SUITE_P(NonRvalueCallNothrow, AnyInvTestNonRvalue,
REGISTER_TYPED_TEST_SUITE_P(AnyInvTestRvalue,
ConversionConstructionReferenceWrapper,
NonMoveableResultType,
- ConversionAssignReferenceWrapper);
+ ConversionAssignReferenceWrapper,
+ NonConstCrashesOnSecondCall,
+ QualifierIndependentObjectLifetime);
INSTANTIATE_TYPED_TEST_SUITE_P(RvalueCallMayThrow, AnyInvTestRvalue,
TestParameterListRvalueQualifiersCallMayThrow);
diff --git a/absl/functional/internal/any_invocable.h b/absl/functional/internal/any_invocable.h
index 35b389d1..8fce4bf6 100644
--- a/absl/functional/internal/any_invocable.h
+++ b/absl/functional/internal/any_invocable.h
@@ -809,11 +809,31 @@ using CanAssignReferenceWrapper = TrueAlias<
: Core(absl::in_place_type<absl::decay_t<T> inv_quals>, \
std::forward<Args>(args)...) {} \
\
+ InvokerType<noex, ReturnType, P...>* ExtractInvoker() cv { \
+ using QualifiedTestType = int cv ref; \
+ auto* invoker = this->invoker_; \
+ if (!std::is_const<QualifiedTestType>::value && \
+ std::is_rvalue_reference<QualifiedTestType>::value) { \
+ ABSL_HARDENING_ASSERT([this]() { \
+ /* We checked that this isn't const above, so const_cast is safe */ \
+ const_cast<Impl*>(this)->invoker_ = \
+ [](TypeErasedState*, \
+ ForwardedParameterType<P>...) noexcept(noex) -> ReturnType { \
+ ABSL_HARDENING_ASSERT(false && "AnyInvocable use-after-move"); \
+ std::terminate(); \
+ }; \
+ return this->HasValue(); \
+ }()); \
+ } \
+ return invoker; \
+ } \
+ \
/*The actual invocation operation with the proper signature*/ \
ReturnType operator()(P... args) cv ref noexcept(noex) { \
assert(this->invoker_ != nullptr); \
- return this->invoker_(const_cast<TypeErasedState*>(&this->state_), \
- static_cast<ForwardedParameterType<P>>(args)...); \
+ return this->ExtractInvoker()( \
+ const_cast<TypeErasedState*>(&this->state_), \
+ static_cast<ForwardedParameterType<P>>(args)...); \
} \
}