-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Implement base::FunctionRef<R(Args...) wrapper with stricter typing.
This provides equivalent functionality to absl::FunctionRef<R(Args...)>, but disallows implicit conversions between function types, e.g.: - a base::FunctionRef<void()> cannot bind to functors with a non-void return value - a base::FunctionRef<void(Derived*)> cannot bind to a functor with a void(Base*) signature. As a side effect, this also improves the diagnostic message when trying to use `base::BindOnce()/base::BindRepeating()` with a capturing lambda. Change-Id: Ibf341e3b15dba5a8bc0d8999e2ab1d905d8bc0e8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3768723 Reviewed-by: danakj <danakj@chromium.org> Reviewed-by: K. Moon <kmoon@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/main@{#1028595}
- Loading branch information
1 parent
e43533a
commit 2248b33
Showing
8 changed files
with
333 additions
and
42 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| // Copyright 2022 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #ifndef BASE_FUNCTIONAL_FUNCTION_REF_H_ | ||
| #define BASE_FUNCTIONAL_FUNCTION_REF_H_ | ||
|
|
||
| #include <type_traits> | ||
| #include <utility> | ||
|
|
||
| #include "base/bind_internal.h" | ||
| #include "third_party/abseil-cpp/absl/base/attributes.h" | ||
| #include "third_party/abseil-cpp/absl/functional/function_ref.h" | ||
|
|
||
| namespace base { | ||
|
|
||
| template <typename Signature> | ||
| class FunctionRef; | ||
|
|
||
| // A non-owning reference to any invocable object (e.g. function pointer, method | ||
| // pointer, functor, lambda, et cetera) suitable for use as a type-erased | ||
| // argument to ForEach-style functions or other visitor patterns that: | ||
| // | ||
| // - do not need to copy or take ownership of the argument | ||
| // - synchronously call the invocable that was passed as an argument | ||
| // | ||
| // `base::FunctionRef` makes no heap allocations: it is trivially copyable and | ||
| // should be passed by value. | ||
| // | ||
| // `base::FunctionRef` has no null/empty state: a `base::FunctionRef` is always | ||
| // valid to invoke. | ||
| // | ||
| // The usual lifetime precautions for other non-owning references types (e.g. | ||
| // `base::StringPiece`, `base::span`) also apply to `base::FunctionRef`. | ||
| // `base::FunctionRef` should typically be used as an argument; returning a | ||
| // `base::FunctionRef` or storing a `base::FunctionRef` as a field is dangerous | ||
| // and likely to result in lifetime bugs. | ||
| // | ||
| // `base::RepeatingCallback` and `base::BindRepeating()` is another common way | ||
| // to represent type-erased invocable objects. In contrast, it requires a heap | ||
| // allocation and is not trivially copyable. It should be used when there are | ||
| // ownership requirements (e.g. partial application of arguments to a function | ||
| // stored for asynchronous execution). | ||
| // | ||
| // Note: this class is very similar to `absl::FunctionRef<R(Args...)>`, but | ||
| // disallows implicit conversions between function types, e.g. functors that | ||
| // return non-void values may bind to `absl::FunctionRef<void(...)>`: | ||
| // | ||
| // ``` | ||
| // void F(absl::FunctionRef<void()>); | ||
| // ... | ||
| // F([] { return 42; }); | ||
| // ``` | ||
| // | ||
| // This compiles and silently discards the return value, but with the base | ||
| // version, the equivalent snippet: | ||
| // | ||
| // ``` | ||
| // void F(base::FunctionRef<void()>); | ||
| // ... | ||
| // F([] { return 42;}); | ||
| // ``` | ||
| // | ||
| // will not compile at all. | ||
| template <typename R, typename... Args> | ||
| class FunctionRef<R(Args...)> { | ||
| public: | ||
| // `ABSL_ATTRIBUTE_LIFETIME_BOUND` is important since `FunctionRef` retains | ||
| // only a reference to `functor`, `functor` must outlive `this`. | ||
| template <typename Functor, | ||
| typename = std::enable_if_t<std::is_same_v< | ||
| R(Args...), | ||
| typename internal::MakeFunctorTraits<Functor>::RunType>>> | ||
| // NOLINTNEXTLINE(google-explicit-constructor) | ||
| FunctionRef(const Functor& functor ABSL_ATTRIBUTE_LIFETIME_BOUND) | ||
| : wrapped_func_ref_(functor) {} | ||
|
|
||
| // Null FunctionRefs are not allowed. | ||
| FunctionRef() = delete; | ||
|
|
||
| FunctionRef(const FunctionRef&) = default; | ||
| // Reduce the likelihood of lifetime bugs by disallowing assignment. | ||
| FunctionRef& operator=(const FunctionRef&) = delete; | ||
|
|
||
| R operator()(Args... args) const { | ||
| return wrapped_func_ref_(std::forward<Args>(args)...); | ||
| } | ||
|
|
||
| absl::FunctionRef<R(Args...)> ToAbsl() const { return wrapped_func_ref_; } | ||
|
|
||
| // In Chrome, converting to `absl::FunctionRef` should be explicitly done | ||
| // through `ToAbsl()`. | ||
| template <typename Signature> | ||
| operator absl::FunctionRef<Signature>() = delete; | ||
|
|
||
| private: | ||
| absl::FunctionRef<R(Args...)> wrapped_func_ref_; | ||
| }; | ||
|
|
||
| } // namespace base | ||
|
|
||
| #endif // BASE_FUNCTIONAL_FUNCTION_REF_H_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // Copyright 2022 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "base/functional/function_ref.h" | ||
|
|
||
| #include "testing/gtest/include/gtest/gtest.h" | ||
|
|
||
| namespace base { | ||
|
|
||
| namespace { | ||
|
|
||
| char Moo(float) { | ||
| return 'a'; | ||
| } | ||
|
|
||
| struct C { | ||
| long Method() { return value; } | ||
| long value; | ||
| }; | ||
|
|
||
| } // namespace | ||
|
|
||
| TEST(FunctionRefTest, FreeFunction) { | ||
| [](FunctionRef<char(float)> ref) { EXPECT_EQ('a', ref(1.0)); }(&Moo); | ||
| } | ||
|
|
||
| TEST(FunctionRefTest, Method) { | ||
| [](FunctionRef<long(C*)> ref) { | ||
| C c = {.value = 25L}; | ||
| EXPECT_EQ(25L, ref(&c)); | ||
| }(&C::Method); | ||
| } | ||
|
|
||
| TEST(FunctionRefTest, Lambda) { | ||
| int x = 3; | ||
| auto lambda = [&x]() { return x; }; | ||
| [](FunctionRef<int()> ref) { EXPECT_EQ(3, ref()); }(lambda); | ||
| } | ||
|
|
||
| TEST(FunctionRefTest, AbslConversion) { | ||
| // Matching signatures should work. | ||
| { | ||
| bool called = false; | ||
| auto lambda = [&called](float) { | ||
| called = true; | ||
| return 'a'; | ||
| }; | ||
| FunctionRef<char(float)> ref(lambda); | ||
| [](absl::FunctionRef<char(float)> absl_ref) { | ||
| absl_ref(1.0); | ||
| }(ref.ToAbsl()); | ||
| EXPECT_TRUE(called); | ||
| } | ||
|
|
||
| // `absl::FunctionRef` should be able to adapt "similar enough" signatures. | ||
| { | ||
| bool called = false; | ||
| auto lambda = [&called](float) { | ||
| called = true; | ||
| return 'a'; | ||
| }; | ||
| FunctionRef<char(float)> ref(lambda); | ||
| [](absl::FunctionRef<void(float)> absl_ref) { | ||
| absl_ref(1.0); | ||
| }(ref.ToAbsl()); | ||
| EXPECT_TRUE(called); | ||
| } | ||
| } | ||
|
|
||
| } // namespace base |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| // Copyright 2022 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| // This is a "No Compile Test" suite. | ||
| // http://dev.chromium.org/developers/testing/no-compile-tests | ||
|
|
||
| #include "base/bind.h" | ||
| #include "base/callback.h" | ||
| #include "base/functional/function_ref.h" | ||
| #include "third_party/abseil-cpp/absl/functional/function_ref.h" | ||
|
|
||
| namespace base { | ||
|
|
||
| #if defined(NCTEST_NO_IMPLICIT_RETURN_TYPE_CONVERSIONS) // [r"note: candidate template ignored: requirement 'std::is_same_v<void \(\), int \(\)>' was not satisfied \[with Functor = \(lambda at [^)]+\)\]"] | ||
|
|
||
| void WontCompile() { | ||
| auto returns_int = [] () { return 42; }; | ||
| FunctionRef<void()> ref(returns_int); | ||
| } | ||
|
|
||
| #elif defined(NCTEST_NO_IMPLICIT_DERIVED_TO_BASE_CONVERSIONS) // [r"note: candidate template ignored: requirement 'std::is_same_v<void \(base::Derived \*\), void \(base::Base \*\)>' was not satisfied \[with Functor = \(lambda at [^)]+\)\]"] | ||
|
|
||
| class Base { }; | ||
| class Derived : public Base { }; | ||
|
|
||
| void WontCompile() { | ||
| auto takes_base = [] (Base*) { }; | ||
| FunctionRef<void(Derived*)> ref(takes_base); | ||
| } | ||
|
|
||
| #elif defined(NCTEST_BIND_ONCE_TO_ABSL_FUNCTION_REF) // [r"fatal error: static_assert failed due to requirement 'AlwaysFalse<void \(\)>': base::Bind{Once,Repeating} require strong ownership: non-owning function references may not bound as the functor due to potential lifetime issues\."] | ||
|
|
||
| void WontCompile() { | ||
| [] (absl::FunctionRef<void()> ref) { | ||
| BindOnce(ref).Run(); | ||
| }([] {}); | ||
| } | ||
|
|
||
| #elif defined(NCTEST_BIND_REPEATING_TO_ABSL_FUNCTION_REF) // [r"fatal error: static_assert failed due to requirement 'AlwaysFalse<void \(\)>': base::Bind{Once,Repeating} require strong ownership: non-owning function references may not bound as the functor due to potential lifetime issues\."] | ||
|
|
||
| void WontCompile() { | ||
| [] (FunctionRef<void()> ref) { | ||
| BindRepeating(ref).Run(); | ||
| }([] {}); | ||
| } | ||
|
|
||
| #elif defined(NCTEST_BIND_ONCE_TO_BASE_FUNCTION_REF) // [r"fatal error: static_assert failed due to requirement 'AlwaysFalse<void \(\)>': base::Bind{Once,Repeating} require strong ownership: non-owning function references may not bound as the functor due to potential lifetime issues\."] | ||
|
|
||
| void WontCompile() { | ||
| [] (FunctionRef<void()> ref) { | ||
| BindOnce(ref).Run(); | ||
| }([] {}); | ||
| } | ||
|
|
||
| #elif defined(NCTEST_BIND_REPEATING_TO_BASE_FUNCTION_REF) // [r"fatal error: static_assert failed due to requirement 'AlwaysFalse<void \(\)>': base::Bind{Once,Repeating} require strong ownership: non-owning function references may not bound as the functor due to potential lifetime issues\."] | ||
|
|
||
| void WontCompile() { | ||
| [] (FunctionRef<void()> ref) { | ||
| BindRepeating(ref).Run(); | ||
| }([] {}); | ||
| } | ||
|
|
||
| #endif | ||
|
|
||
| } // namespace base |
Oops, something went wrong.