From 90f8b37a3bb6e7b954c9e2846aea48ee82b3a06f Mon Sep 17 00:00:00 2001
From: Ryan Ofsky <ryan@ofsky.org>
Date: Mon, 22 Jul 2024 21:47:25 -0400
Subject: [PATCH] Fix $Proxy.wrap mapping for empty structs with no fields

Trying to pass an empty $Proxy.wrap struct with no fields fails because
BuildOne overload for the last field number is incorrectly declared to accept
an lvalue Builder reference instead of a univeral reference, and the
CustomBuildField overload which calls BuildOne passes an rvalue Builder.

The problem did not happen as long as the wrapped struct had at least one field
because CustomBuildField would call the other BuildOne overload for the first
field number which accepts an rvalue Builder, and that BuildOne overload would
call the next BuildOne overload for the next field number, passing the builder
as an lvalue.

A test is included which triggers the bug. Compiler output before the bugfix looked like:

include/mp/proxy-types.h:1066:5: error: no matching function for call to 'BuildOne'
 1066 |     BuildOne<0>(local_type, invoke_context, output.init(), value);

include/mp/proxy-types.h:1050:6: note: candidate function [with index = 0, LocalType = mp::test::FooEmpty, Value = mp::test::FooEmpty, Output = mp::test::messages::FooEmpty::Builder] not viable: expects an lvalue for 3rd argument
 1050 | void BuildOne(TypeList<LocalType> param,
---
 include/mp/proxy-types.h | 4 ++--
 test/mp/test/foo.capnp   | 4 ++++
 test/mp/test/foo.h       | 5 +++++
 test/mp/test/test.cpp    | 2 ++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/mp/proxy-types.h b/include/mp/proxy-types.h
index 7f3f7eb2..f81cfa03 100644
--- a/include/mp/proxy-types.h
+++ b/include/mp/proxy-types.h
@@ -1049,8 +1049,8 @@ void BuildOne(TypeList<LocalType> param,
 template <size_t index, typename LocalType, typename Value, typename Output>
 void BuildOne(TypeList<LocalType> param,
     InvokeContext& invoke_context,
-    Output& output,
-    Value& value,
+    Output&& output,
+    Value&& value,
     typename std::enable_if<index == ProxyType<LocalType>::fields>::type* enable = nullptr)
 {
 }
diff --git a/test/mp/test/foo.capnp b/test/mp/test/foo.capnp
index 01987a45..9080ee3c 100644
--- a/test/mp/test/foo.capnp
+++ b/test/mp/test/foo.capnp
@@ -24,6 +24,7 @@ interface FooInterface $Proxy.wrap("mp::test::FooImplementation") {
     callbackSaved @9 (context :Proxy.Context, arg: Int32) -> (result :Int32);
     callbackExtended @10 (context :Proxy.Context, callback :ExtendedCallback, arg: Int32) -> (result :Int32);
     passCustom @11 (arg :FooCustom) -> (result :FooCustom);
+    passEmpty @12 (arg :FooEmpty) -> (result :FooEmpty);
 }
 
 interface FooCallback $Proxy.wrap("mp::test::FooCallback") {
@@ -46,6 +47,9 @@ struct FooCustom $Proxy.wrap("mp::test::FooCustom") {
     v2 @1 :Int32;
 }
 
+struct FooEmpty $Proxy.wrap("mp::test::FooEmpty") {
+}
+
 struct Pair(T1, T2) {
     first @0 :T1;
     second @1 :T2;
diff --git a/test/mp/test/foo.h b/test/mp/test/foo.h
index bc6a22e5..43f4205b 100644
--- a/test/mp/test/foo.h
+++ b/test/mp/test/foo.h
@@ -27,6 +27,10 @@ struct FooCustom
     int v2;
 };
 
+struct FooEmpty
+{
+};
+
 class FooCallback
 {
 public:
@@ -55,6 +59,7 @@ class FooImplementation
     int callbackSaved(int arg) { return m_callback->call(arg); }
     int callbackExtended(ExtendedCallback& callback, int arg) { return callback.callExtended(arg); }
     FooCustom passCustom(FooCustom foo) { return foo; }
+    FooEmpty passEmpty(FooEmpty foo) { return foo; }
     std::shared_ptr<FooCallback> m_callback;
 };
 
diff --git a/test/mp/test/test.cpp b/test/mp/test/test.cpp
index 457d2c65..ed845cd6 100644
--- a/test/mp/test/test.cpp
+++ b/test/mp/test/test.cpp
@@ -106,6 +106,8 @@ KJ_TEST("Call FooInterface methods")
     KJ_EXPECT(custom_in.v1 == custom_out.v1);
     KJ_EXPECT(custom_in.v2 == custom_out.v2);
 
+    foo->passEmpty(FooEmpty{});
+
     disconnect_client();
     thread.join();