Skip to content

Commit e15f8e7

Browse files
author
Michal Klocek
committed
Revert "Disallow constructing FunctionRefs from others w/compatible signatures."
Revert as it does not work with msvc22. This reverts: https://chromium-review.googlesource.com/c/chromium/src/+/5150374 Change-Id: I4ec5d9a762445b448b1fe988bdfe3e9e80357323 Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/559262 Reviewed-by: Peter Varga <pvarga@inf.u-szeged.hu>
1 parent bfb686c commit e15f8e7

File tree

2 files changed

+1
-97
lines changed

2 files changed

+1
-97
lines changed

chromium/base/functional/function_ref.h

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
#include "base/compiler_specific.h"
1414
#include "base/functional/bind_internal.h"
15-
#include "base/types/is_instantiation.h"
1615
#include "third_party/abseil-cpp/absl/functional/function_ref.h"
1716

1817
namespace base {
@@ -74,27 +73,7 @@ class FunctionRef<R(Args...)> {
7473
// `LIFETIME_BOUND` is important; since `FunctionRef` retains
7574
// only a reference to `functor`, `functor` must outlive `this`.
7675
template <typename Functor>
77-
requires kCompatibleFunctor<Functor> &&
78-
// Prevent this constructor from participating in overload
79-
// resolution if the callable is itself an instantiation of the
80-
// `FunctionRef` template.
81-
//
82-
// If the callable is a `FunctionRef` with exactly the same
83-
// signature as us, then the copy constructor will be used instead,
84-
// so this has no effect. (Note that if the constructor argument
85-
// were `Functor&&`, this exclusion would be necessary to force the
86-
// choice of the copy constructor over this one for non-const ref
87-
// args; see https://stackoverflow.com/q/57909923.)
88-
//
89-
// If the callable is a `FunctionRef` with some other signature
90-
// then we choose not to support binding to it at all. Conceivably
91-
// we could teach our trampoline to deal with this, but this may be
92-
// the sign of an object lifetime bug, and again it's not clear
93-
// that this isn't just a mistake on the part of the user.
94-
(!is_instantiation<FunctionRef, std::decay_t<Functor>>) &&
95-
// For the same reason as the second case above, prevent
96-
// construction from `absl::FunctionRef`.
97-
(!is_instantiation<absl::FunctionRef, std::decay_t<Functor>>)
76+
requires kCompatibleFunctor<Functor>
9877
// NOLINTNEXTLINE(google-explicit-constructor)
9978
FunctionRef(const Functor& functor LIFETIME_BOUND)
10079
: wrapped_func_ref_(functor) {}

chromium/base/functional/function_ref_unittest.cc

Lines changed: 0 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
#include <stdint.h>
88

99
#include <concepts>
10-
#include <optional>
1110

12-
#include "base/compiler_specific.h"
1311
#include "testing/gtest/include/gtest/gtest.h"
1412
#include "third_party/abseil-cpp/absl/functional/function_ref.h"
1513

@@ -70,62 +68,6 @@ TEST(FunctionRef, Method) {
7068
[&s](FunctionRef<int(const S*)> ref) { EXPECT_EQ(25, ref(&s)); }(&S::Method);
7169
}
7270

73-
// If we construct from another `FunctionRef`, that should work fine, even if
74-
// the input is destroyed before we call the output. In other words, we should
75-
// reference the underlying callable, not the `FunctionRef`.
76-
//
77-
// We construct in a `noinline` function to maximize the chance that ASAN
78-
// notices the use-after-free if we get this wrong.
79-
NOINLINE void ConstructFromLValue(std::optional<FunctionRef<int()>>& ref) {
80-
const auto return_17 = [] { return 17; };
81-
FunctionRef<int()> other = return_17;
82-
ref.emplace(other);
83-
}
84-
NOINLINE void ConstructFromConstLValue(std::optional<FunctionRef<int()>>& ref) {
85-
const auto return_17 = [] { return 17; };
86-
const FunctionRef<int()> other = return_17;
87-
ref.emplace(other);
88-
}
89-
NOINLINE void ConstructFromRValue(std::optional<FunctionRef<int()>>& ref) {
90-
const auto return_17 = [] { return 17; };
91-
using Ref = FunctionRef<int()>;
92-
ref.emplace(Ref(return_17));
93-
}
94-
NOINLINE void ConstructFromConstRValue(std::optional<FunctionRef<int()>>& ref) {
95-
const auto return_17 = [] { return 17; };
96-
using Ref = const FunctionRef<int()>;
97-
ref.emplace(Ref(return_17));
98-
}
99-
TEST(FunctionRef, ConstructionFromOtherFunctionRefObjects) {
100-
using Ref = FunctionRef<int()>;
101-
std::optional<Ref> ref;
102-
103-
ConstructFromLValue(ref);
104-
EXPECT_EQ(17, (*ref)());
105-
106-
ConstructFromConstLValue(ref);
107-
EXPECT_EQ(17, (*ref)());
108-
109-
ConstructFromRValue(ref);
110-
EXPECT_EQ(17, (*ref)());
111-
112-
ConstructFromConstRValue(ref);
113-
EXPECT_EQ(17, (*ref)());
114-
115-
// It shouldn't be possible to construct from `FunctionRef` objects with
116-
// differing signatures, even if they are compatible with `int()`.
117-
static_assert(!std::constructible_from<Ref, FunctionRef<void()>>);
118-
static_assert(!std::constructible_from<Ref, FunctionRef<int(int)>>);
119-
static_assert(!std::constructible_from<Ref, FunctionRef<int64_t()>>);
120-
121-
// Check again with various qualifiers.
122-
static_assert(!std::constructible_from<Ref, const FunctionRef<void()>>);
123-
static_assert(!std::constructible_from<Ref, FunctionRef<void()>&>);
124-
static_assert(!std::constructible_from<Ref, FunctionRef<void()>&&>);
125-
static_assert(!std::constructible_from<Ref, const FunctionRef<void()>&>);
126-
static_assert(!std::constructible_from<Ref, const FunctionRef<void()>&&>);
127-
}
128-
12971
// `FunctionRef` allows functors with convertible return types to be adapted.
13072
TEST(FunctionRef, ConvertibleReturnTypes) {
13173
{
@@ -200,21 +142,4 @@ TEST(FunctionRef, ConstructionFromInexactMatches) {
200142
decltype(&functor)>);
201143
}
202144

203-
TEST(FunctionRef, ConstructionFromAbslFunctionRef) {
204-
// It shouldn't be possible to construct a `FunctionRef` from an
205-
// `absl::FunctionRef`, whether the signatures are compatible or not.
206-
using Ref = FunctionRef<int(int)>;
207-
static_assert(!std::is_constructible_v<Ref, absl::FunctionRef<void()>>);
208-
static_assert(!std::is_constructible_v<Ref, absl::FunctionRef<void(int)>>);
209-
static_assert(!std::is_constructible_v<Ref, absl::FunctionRef<int(int)>>);
210-
211-
// Check again with various qualifiers.
212-
using AbslRef = absl::FunctionRef<int(int)>;
213-
static_assert(!std::is_constructible_v<Ref, const AbslRef>);
214-
static_assert(!std::is_constructible_v<Ref, AbslRef&>);
215-
static_assert(!std::is_constructible_v<Ref, AbslRef&&>);
216-
static_assert(!std::is_constructible_v<Ref, const AbslRef&>);
217-
static_assert(!std::is_constructible_v<Ref, const AbslRef&&>);
218-
}
219-
220145
} // namespace base

0 commit comments

Comments
 (0)