[v2] libcamera: base: bound_method: Move return value
diff mbox series

Message ID 20250619093218.1335931-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v2] libcamera: base: bound_method: Move return value
Related show

Commit Message

Barnabás Pőcze June 19, 2025, 9:32 a.m. UTC 
Instead of copying, just move the returned value when the call is made
through an argument pack. This enables, e.g. `Object::invokeMethod()`
to be usable with functions returning types, such as`std::unique_ptr`,
that have no copy ctor/assignment. Since there are no other users of
the argument pack object, this is safe to do. Reference return types
are not supported, so a simple `std::move()` is sufficient.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=273#c1
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
changes in v2:
  * add `[[maybe_unused]]` for gcc 9
  * review comments

v1: https://patchwork.libcamera.org/patch/23598/
---
 include/libcamera/base/bound_method.h | 21 ++++++++-------------
 test/object-invoke.cpp                | 17 +++++++++++++++++
 2 files changed, 25 insertions(+), 13 deletions(-)

--
2.50.0

Patch
diff mbox series 

diff --git a/include/libcamera/base/bound_method.h b/include/libcamera/base/bound_method.h
index 507c320d3..e14614bc4 100644
--- a/include/libcamera/base/bound_method.h
+++ b/include/libcamera/base/bound_method.h
@@ -38,11 +38,6 @@  public:
 	{
 	}

-	R returnValue()
-	{
-		return ret_;
-	}
-
 	std::tuple<typename std::remove_reference_t<Args>...> args_;
 	R ret_;
 };
@@ -56,10 +51,6 @@  public:
 	{
 	}

-	void returnValue()
-	{
-	}
-
 	std::tuple<typename std::remove_reference_t<Args>...> args_;
 };

@@ -140,8 +131,10 @@  public:
 			return func_(args...);

 		auto pack = std::make_shared<PackType>(args...);
-		bool sync = BoundMethodBase::activatePack(pack, deleteMethod);
-		return sync ? pack->returnValue() : R();
+		[[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod);
+
+		if constexpr (!std::is_void_v<R>)
+			return sync ? std::move(pack->ret_) : R();
 	}

 	R invoke(Args... args) override
@@ -175,8 +168,10 @@  public:
 		}

 		auto pack = std::make_shared<PackType>(args...);
-		bool sync = BoundMethodBase::activatePack(pack, deleteMethod);
-		return sync ? pack->returnValue() : R();
+		[[maybe_unused]] bool sync = BoundMethodBase::activatePack(pack, deleteMethod);
+
+		if constexpr (!std::is_void_v<R>)
+			return sync ? std::move(pack->ret_) : R();
 	}

 	R invoke(Args... args) override
diff --git a/test/object-invoke.cpp b/test/object-invoke.cpp
index def1e61e4..ab4a15751 100644
--- a/test/object-invoke.cpp
+++ b/test/object-invoke.cpp
@@ -58,6 +58,19 @@  public:
 		return 42;
 	}

+	struct MoveOnly {
+		MoveOnly() = default;
+		MoveOnly(const MoveOnly &) = delete;
+		MoveOnly &operator=(const MoveOnly &) = delete;
+		MoveOnly(MoveOnly &&) = default;
+		MoveOnly &operator=(MoveOnly &&) = default;
+	};
+
+	MoveOnly methodWithReturnMoveOnly()
+	{
+		return {};
+	}
+
 private:
 	Status status_;
 	int value_;
@@ -186,6 +199,10 @@  protected:
 			return TestFail;
 		}

+		/* Test invoking a method that returns type with no copy ctor/assignment. */
+		object_.invokeMethod(&InvokedObject::methodWithReturnMoveOnly,
+				     ConnectionTypeBlocking);
+
 		return TestPass;
 	}