pw_sync: Fixed InlineBorrowable constructor ambiguity
There is ambiguity about which constructor to invoke when constructing
an inline object that has a single construtor argument
using the in-place syntax with an lvalue parameter. Added enable_if
guards so that the factory-function constructors are only instantiated
for compatible callables.
Change-Id: I7db859bf4b4cf31491c10d1bd9105f34b11a133d
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/111290
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: Anton Markov <amarkov@google.com>
diff --git a/pw_sync/inline_borrowable_test.cc b/pw_sync/inline_borrowable_test.cc
index a626027..4e55ac7 100644
--- a/pw_sync/inline_borrowable_test.cc
+++ b/pw_sync/inline_borrowable_test.cc
@@ -36,6 +36,7 @@
// A custom type that is neither copyable nor movable.
class CustomType {
public:
+ explicit constexpr CustomType(int z) : x_(z), y_(-z) {}
constexpr CustomType(int x, int y) : x_(x), y_(y) {}
CustomType(const CustomType&) = delete;
@@ -79,7 +80,18 @@
EXPECT_TRUE(trivial.acquire()->yes());
}
-TEST(InlineBorrowableTest, TestCustomTypeInPlace) {
+TEST(InlineBorrowableTest, TestCustomTypeInPlace1Arg) {
+ InlineBorrowable<CustomType> custom(std::in_place, 1);
+ EXPECT_EQ(custom.acquire()->data(), std::make_pair(1, -1));
+}
+
+TEST(InlineBorrowableTest, TestCustomTypeInPlace1ArgLValue) {
+ int x = 1;
+ InlineBorrowable<CustomType> custom(std::in_place, x);
+ EXPECT_EQ(custom.acquire()->data(), std::make_pair(x, -x));
+}
+
+TEST(InlineBorrowableTest, TestCustomTypeInPlace2Arg) {
InlineBorrowable<CustomType> custom(std::in_place, 1, 2);
EXPECT_EQ(custom.acquire()->data(), std::make_pair(1, 2));
}
@@ -94,6 +106,16 @@
EXPECT_EQ(custom.acquire()->data(), std::make_pair(1, 2));
}
+TEST(InlineBorrowableTest, TestCustomTypeFromMutableFactory) {
+ int i = 0;
+ auto factory = [&i]() mutable {
+ i++;
+ return CustomType(1, 2);
+ };
+ InlineBorrowable<CustomType> custom(factory);
+ EXPECT_EQ(custom.acquire()->data(), std::make_pair(1, 2));
+}
+
TEST(InlineBorrowableTest, TestTrivialTypeWithInterruptSpinLock) {
InlineBorrowable<TrivialType, VirtualInterruptSpinLock>
trivial_interrupt_safe;
diff --git a/pw_sync/public/pw_sync/inline_borrowable.h b/pw_sync/public/pw_sync/inline_borrowable.h
index e9d0d93..0ee9fb5 100644
--- a/pw_sync/public/pw_sync/inline_borrowable.h
+++ b/pw_sync/public/pw_sync/inline_borrowable.h
@@ -91,27 +91,47 @@
// [&]{ return Foo{foo_arg1, foo_arg2}; }
// [&]{ return MyLock{lock_arg1, lock_arg2}; }
//
- template <typename ObjectConstructor, typename LockConstructor = Lock()>
+ template <typename ObjectConstructor,
+ typename LockConstructor = Lock(),
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<GuardedType&&, ObjectConstructor>>,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<Lock&&, LockConstructor>>>
constexpr explicit InlineBorrowable(
const ObjectConstructor& object_ctor,
const LockConstructor& lock_ctor = internal::DefaultConstruct<Lock>)
: Storage(object_ctor, lock_ctor),
Base(Storage::object_, Storage::lock_) {}
- template <typename ObjectConstructor, typename LockConstructor = Lock()>
+ template <typename ObjectConstructor,
+ typename LockConstructor = Lock(),
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<GuardedType&&, ObjectConstructor>>,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<Lock&&, LockConstructor>>>
constexpr explicit InlineBorrowable(
ObjectConstructor& object_ctor,
const LockConstructor& lock_ctor = internal::DefaultConstruct<Lock>)
: Storage(object_ctor, lock_ctor),
Base(Storage::object_, Storage::lock_) {}
- template <typename ObjectConstructor, typename LockConstructor = Lock()>
+ template <typename ObjectConstructor,
+ typename LockConstructor = Lock(),
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<GuardedType&&, ObjectConstructor>>,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<Lock&&, LockConstructor>>>
constexpr explicit InlineBorrowable(const ObjectConstructor& object_ctor,
LockConstructor& lock_ctor)
: Storage(object_ctor, lock_ctor),
Base(Storage::object_, Storage::lock_) {}
- template <typename ObjectConstructor, typename LockConstructor = Lock()>
+ template <typename ObjectConstructor,
+ typename LockConstructor = Lock(),
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<GuardedType&&, ObjectConstructor>>,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<Lock&&, LockConstructor>>>
constexpr explicit InlineBorrowable(ObjectConstructor& object_ctor,
LockConstructor& lock_ctor)
: Storage(object_ctor, lock_ctor),
diff --git a/pw_sync/public/pw_sync/internal/borrowable_storage.h b/pw_sync/public/pw_sync/internal/borrowable_storage.h
index d05965e..d13c94f 100644
--- a/pw_sync/public/pw_sync/internal/borrowable_storage.h
+++ b/pw_sync/public/pw_sync/internal/borrowable_storage.h
@@ -24,7 +24,7 @@
protected:
// Construct the object in-place using a list of arguments.
template <typename... Args>
- constexpr BorrowableStorage(std::in_place_t, Args&&... args)
+ constexpr explicit BorrowableStorage(std::in_place_t, Args&&... args)
: object_{std::forward<Args>(args)...}, lock_ {}
{}
@@ -38,22 +38,42 @@
std::forward<std::tuple<LockArgs...>>(lock_args))} {}
// Construct the object and lock in-place using the provided factories.
- template <typename ObjectConstructor, typename LockConstructor>
+ template <typename ObjectConstructor,
+ typename LockConstructor,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<ObjectType&&, ObjectConstructor>>,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<Lock&&, LockConstructor>>>
constexpr BorrowableStorage(const ObjectConstructor& object_ctor,
const LockConstructor& lock_ctor)
: object_{object_ctor()}, lock_{lock_ctor()} {}
- template <typename ObjectConstructor, typename LockConstructor>
+ template <typename ObjectConstructor,
+ typename LockConstructor,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<ObjectType&&, ObjectConstructor>>,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<Lock&&, LockConstructor>>>
constexpr BorrowableStorage(ObjectConstructor& object_ctor,
const LockConstructor& lock_ctor)
: object_{object_ctor()}, lock_{lock_ctor()} {}
- template <typename ObjectConstructor, typename LockConstructor>
+ template <typename ObjectConstructor,
+ typename LockConstructor,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<ObjectType&&, ObjectConstructor>>,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<Lock&&, LockConstructor>>>
constexpr BorrowableStorage(const ObjectConstructor& object_ctor,
LockConstructor& lock_ctor)
: object_{object_ctor()}, lock_{lock_ctor()} {}
- template <typename ObjectConstructor, typename LockConstructor>
+ template <typename ObjectConstructor,
+ typename LockConstructor,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<ObjectType&&, ObjectConstructor>>,
+ typename = std::enable_if_t<
+ std::is_invocable_r_v<Lock&&, LockConstructor>>>
constexpr BorrowableStorage(ObjectConstructor& object_ctor,
LockConstructor& lock_ctor)
: object_{object_ctor()}, lock_{lock_ctor()} {}
