From 7b9889edb8d4f98c6ee6d5f698f58db7c38706a4 Mon Sep 17 00:00:00 2001
From: w0xlt <94266259+w0xlt@users.noreply.github.com>
Date: Thu, 28 May 2026 17:51:30 -0700
Subject: [PATCH] mpgen: emit return value after output parameters

The generated proxy call chain is matched positionally with
FunctionTraits::Fields, which represents C++ methods as parameters followed by
the return type. Previously mpgen emitted fields in Cap'n Proto schema order,
so a schema that kept an existing result field first and appended new output
fields would map the C++ return value before those output parameters.

This made wire-compatible schema extensions like -> (result, reason, debug)
fail for C++ methods returning a value and filling output references. Emit
non-return fields first and the retval field last in proxy glue, while leaving
the Cap'n Proto schema field order unchanged on the wire.

Add a regression test for a method whose schema declares result before a Text
output parameter.
---
 src/mp/gen.cpp         | 14 ++++++++++++--
 test/mp/test/foo.capnp |  1 +
 test/mp/test/foo.h     |  1 +
 test/mp/test/test.cpp  |  3 +++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/mp/gen.cpp b/src/mp/gen.cpp
index 603f9ccb..556260aa 100644
--- a/src/mp/gen.cpp
+++ b/src/mp/gen.cpp
@@ -524,8 +524,8 @@ static void Generate(kj::StringPtr src_prefix,
                 std::ostringstream server_invoke_start;
                 std::ostringstream server_invoke_end;
                 int argc = 0;
-                for (const auto& field : fields) {
-                    if (field.skip) continue;
+                auto build_field = [&](const Field& field) {
+                    if (field.skip) return;
 
                     const auto& f = field.param_is_set ? field.param : field.result;
                     auto field_name = f.getProto().getName();
@@ -589,6 +589,16 @@ static void Generate(kj::StringPtr src_prefix,
                     server_invoke_start << ", Accessor<" << base_name << "_fields::" << Cap(field_name) << ", "
                                         << field_flags.str() << ">>(";
                     server_invoke_end << ")";
+                };
+
+                for (const auto& field : fields) {
+                    if (!field.retval) build_field(field);
+                }
+                // C++ return values appear after all method parameters in FunctionTraits.
+                // Keep this order even when the Cap'n Proto result field is declared
+                // earlier for wire compatibility.
+                for (const auto& field : fields) {
+                    if (field.retval) build_field(field);
                 }
 
                 const std::string static_str{is_construct || is_destroy ? "static " : ""};
diff --git a/test/mp/test/foo.capnp b/test/mp/test/foo.capnp
index 5bdee257..57c18b1c 100644
--- a/test/mp/test/foo.capnp
+++ b/test/mp/test/foo.capnp
@@ -15,6 +15,7 @@ interface FooInterface $Proxy.wrap("mp::test::FooImplementation") {
     add @0 (a :Int32, b :Int32) -> (result :Int32);
     addOut @19 (a :Int32, b :Int32) -> (ret :Int32);
     addInOut @20 (x :Int32, sum :Int32) -> (sum :Int32);
+    addResultOut @23 (value :Int32) -> (result :Int32, text :Text);
     mapSize @1 (map :List(Pair(Text, Text))) -> (result :Int32);
     pass @2 (arg :FooStruct) -> (result :FooStruct);
     raise @3 (arg :FooStruct) -> (error :FooStruct $Proxy.exception("mp::test::FooStruct"));
diff --git a/test/mp/test/foo.h b/test/mp/test/foo.h
index 317af025..51d4a2e7 100644
--- a/test/mp/test/foo.h
+++ b/test/mp/test/foo.h
@@ -67,6 +67,7 @@ class FooImplementation
     int add(int a, int b) { return a + b; }
     void addOut(int a, int b, int& out) { out = a + b; }
     void addInOut(int x, int& sum) { sum += x; }
+    int addResultOut(int value, std::string& text) { text = std::to_string(value); return value + 1; }
     int mapSize(const std::map<std::string, std::string>& map) { return map.size(); }
     FooStruct pass(FooStruct foo) { return foo; }
     void raise(FooStruct foo) { throw foo; }
diff --git a/test/mp/test/test.cpp b/test/mp/test/test.cpp
index d91edb40..f6b2f1ba 100644
--- a/test/mp/test/test.cpp
+++ b/test/mp/test/test.cpp
@@ -138,6 +138,9 @@ KJ_TEST("Call FooInterface methods")
     KJ_EXPECT(ret == 7);
     foo->addInOut(3, ret);
     KJ_EXPECT(ret == 10);
+    std::string text;
+    KJ_EXPECT(foo->addResultOut(41, text) == 42);
+    KJ_EXPECT(text == "41");
 
     FooStruct in;
     in.name = "name";