From 818a3c6490810bda6b9fbb0899cd252addf9388a Mon Sep 17 00:00:00 2001 From: Dario Gabriel Lipicar Date: Sat, 18 Jul 2026 11:05:52 -0300 Subject: [PATCH 1/2] fix(codegen): pack Qt client args as one element each, not a spread list The generated Qt client wrapper packed a method's arguments with `QVariantList{a, b, ...}` (sync) and `QVariantList{...}` / `QVariantList() << a` (async). For a QVariantList-typed argument -- every `[T]` list type (`[any]`, `[int]`, `[uint]`, `[float64]`, `[bool]`) -- a braced `QVariantList{v}` and `<< v` both CONCATENATE the list's elements into the args list, so `echoList([1,2,3])` went out as three positional args instead of one array arg. The receiver saw an arg-count mismatch and the list round-tripped empty; through a UI->proxy->provider 2-hop it hung the call outright. This is the long-standing "typed arrays empty over the Qt path" bug. Fix both generators that emit the Qt client: - legacy `generator_lib.cpp` (the production `logos-cpp-generator`): wrap each arg in `QVariant::fromValue(...)` in the sync and async call sites. - experimental `lidl_gen_client.cpp`: route both paths through the existing `packVariantList` helper (which already wraps with `QVariant::fromValue`), the same helper the event `trigger` path uses. `QVariant::fromValue` does not double-wrap an already-QVariant (`any`) arg, and scalars/QString/QVariantMap/QByteArray were never affected (they don't concatenate). Empirically: `QVariantList{v}` / `<< v` give size 3 for a 3-element list; the wrapped forms give size 1. Tests: legacy generator_tests gain ListArgWrappedAsOneElement and update the param-packing assertions to the wrapped form; experimental gains ListArgIsPackedAsOneElement. 167/167 green. --- .../experimental/lidl_gen_client.cpp | 28 +++++++------- cpp-generator/legacy/generator_lib.cpp | 12 +++++- tests/experimental/test_lidl_gen_client.cpp | 33 +++++++++++++++- tests/generator/test_make_source.cpp | 38 ++++++++++++++++--- 4 files changed, 87 insertions(+), 24 deletions(-) diff --git a/cpp-generator/experimental/lidl_gen_client.cpp b/cpp-generator/experimental/lidl_gen_client.cpp index 7119f0f..492fdf5 100644 --- a/cpp-generator/experimental/lidl_gen_client.cpp +++ b/cpp-generator/experimental/lidl_gen_client.cpp @@ -233,12 +233,17 @@ QString lidlMakeSource(const ModuleDecl& module, BindMode bindMode) if (ret != "void") s << " QVariant _result = "; else s << " "; - s << "m_client->invokeRemoteMethod(" << targetExpr << ", \"" << md.name << "\", QVariantList{"; + // Pack each argument as ONE element via packVariantList (which wraps + // with QVariant::fromValue). A braced `QVariantList{v}` or `<< v` would + // CONCATENATE a QVariantList-typed arg (any `[T]` list) into the args + // list, sending a 3-element [1,2,3] as three positional args instead of + // one — the historical "typed arrays empty over the Qt path" bug. + s << "m_client->invokeRemoteMethod(" << targetExpr << ", \"" << md.name << "\", packVariantList("; for (int i = 0; i < nParams; ++i) { s << md.params[i].name; if (i + 1 < nParams) s << ", "; } - s << "}, Timeout(), &_err);\n"; + s << "), Timeout(), &_err);\n"; s << " if (err) *err = _err;\n"; s << " else if (!_err.ok()) qWarning() << \"" << className << "::" << md.name << ": remote call failed:\" << QString::fromStdString(_err.message);\n"; @@ -255,19 +260,14 @@ QString lidlMakeSource(const ModuleDecl& module, BindMode bindMode) if (nParams > 0) s << ", "; s << "std::function callback, Timeout timeout) {\n"; s << " if (!callback) return;\n"; - s << " m_client->invokeRemoteMethodAsync(" << targetExpr << ", \"" << md.name << "\", "; - if (nParams == 0) { - s << "QVariantList()"; - } else if (nParams == 1) { - s << "QVariantList() << " << md.params[0].name; - } else { - s << "QVariantList{"; - for (int i = 0; i < nParams; ++i) { - s << md.params[i].name; - if (i + 1 < nParams) s << ", "; - } - s << "}"; + // Same one-element-per-arg packing as the sync path (see above): a + // QVariantList-typed arg must not be spread across the args list. + s << " m_client->invokeRemoteMethodAsync(" << targetExpr << ", \"" << md.name << "\", packVariantList("; + for (int i = 0; i < nParams; ++i) { + s << md.params[i].name; + if (i + 1 < nParams) s << ", "; } + s << ")"; s << ", [callback](QVariant v) {\n"; if (ret == "void") { s << " callback();\n"; diff --git a/cpp-generator/legacy/generator_lib.cpp b/cpp-generator/legacy/generator_lib.cpp index 7b5543c..8225791 100644 --- a/cpp-generator/legacy/generator_lib.cpp +++ b/cpp-generator/legacy/generator_lib.cpp @@ -613,9 +613,15 @@ QString makeSource(const QString& moduleName, const QString& className, const QS if (ret != "void") s << " QVariant _result = "; else s << " "; + // Wrap each argument in QVariant::fromValue so it becomes exactly ONE + // element of the args list. A bare `QVariantList{v}` CONCATENATES a + // QVariantList-typed arg (every `[T]` list) into the args list — sending + // a 3-element [1,2,3] as three positional args — the historical "typed + // arrays empty over the Qt path" bug. fromValue does not double-wrap an + // already-QVariant (`any`) arg. s << "m_client->invokeRemoteMethod(" << targetExpr << ", \"" << name << "\", QVariantList{"; for (int i = 0; i < params.size(); ++i) { - s << wireArg(params.at(i).toObject()); + s << "QVariant::fromValue(" << wireArg(params.at(i).toObject()) << ")"; if (i + 1 < params.size()) s << ", "; } s << "}, Timeout(), &_err);\n"; @@ -672,9 +678,11 @@ QString makeSource(const QString& moduleName, const QString& className, const QS if (params.size() == 0) { s << "QVariantList()"; } else { + // Same one-element-per-arg wrapping as the sync path (see above): a + // QVariantList-typed arg must not be spread across the args list. s << "QVariantList{"; for (int i = 0; i < params.size(); ++i) { - s << wireArg(params.at(i).toObject()); + s << "QVariant::fromValue(" << wireArg(params.at(i).toObject()) << ")"; if (i + 1 < params.size()) s << ", "; } s << "}"; diff --git a/tests/experimental/test_lidl_gen_client.cpp b/tests/experimental/test_lidl_gen_client.cpp index 653d1dd..4acceb0 100644 --- a/tests/experimental/test_lidl_gen_client.cpp +++ b/tests/experimental/test_lidl_gen_client.cpp @@ -198,8 +198,37 @@ TEST(LidlGenClient, MethodWithManyParams) m.methods.push_back(md); QString s = lidlMakeSource(m); - // >5 params should use QVariantList{} syntax - EXPECT_TRUE(s.contains("QVariantList{")); + // Args are packed via packVariantList (one QVariant element per arg), + // regardless of arity — never a braced/`<<` list that would spread a + // QVariantList-typed arg. + EXPECT_TRUE(s.contains("packVariantList(p0, p1, p2, p3, p4, p5, p6)")); +} + +// Regression: a QVariantList-typed ([any]/[int]/...) argument must be packed as +// ONE element, not concatenated into the args list. `QVariantList{v}` and +// `QVariantList() << v` both spread a QVariantList; packVariantList wraps each +// arg with QVariant::fromValue, so a single list arg stays a single arg. +TEST(LidlGenClient, ListArgIsPackedAsOneElement) +{ + ModuleDecl m; + m.name = "arrs"; + MethodDecl md; + md.name = "echoList"; + TypeExpr elem = { TypeExpr::Primitive, "any", {} }; + md.returnType = { TypeExpr::Array, "", { elem } }; + ParamDecl p; + p.name = "v"; + p.type = { TypeExpr::Array, "", { elem } }; + md.params.push_back(p); + m.methods.push_back(md); + + QString s = lidlMakeSource(m); + // Both sync and async pack the single list arg via packVariantList(v). + EXPECT_TRUE(s.contains("invokeRemoteMethod(\"arrs\", \"echoList\", packVariantList(v)")); + EXPECT_TRUE(s.contains("invokeRemoteMethodAsync(\"arrs\", \"echoList\", packVariantList(v)")); + // Guard against the spreading forms regressing back in. + EXPECT_FALSE(s.contains("QVariantList{v}")); + EXPECT_FALSE(s.contains("QVariantList() << v")); } TEST(LidlGenClient, VoidReturnMethod) diff --git a/tests/generator/test_make_source.cpp b/tests/generator/test_make_source.cpp index bea7fd1..d955d1d 100644 --- a/tests/generator/test_make_source.cpp +++ b/tests/generator/test_make_source.cpp @@ -53,7 +53,7 @@ TEST(MakeSourceTest, OneParam) QJsonArray methods; methods.append(makeMethod("fn", "bool", 1)); QString src = makeSource("mod", "Mod", "mod.h", methods); - EXPECT_TRUE(src.contains("m_client->invokeRemoteMethod(\"mod\", \"fn\", QVariantList{p0}, Timeout(), &_err)")); + EXPECT_TRUE(src.contains("m_client->invokeRemoteMethod(\"mod\", \"fn\", QVariantList{QVariant::fromValue(p0)}, Timeout(), &_err)")); EXPECT_TRUE(src.contains("return _result.toBool()")); } @@ -62,7 +62,7 @@ TEST(MakeSourceTest, TwoParams) QJsonArray methods; methods.append(makeMethod("fn", "void", 2)); QString src = makeSource("mod", "Mod", "mod.h", methods); - EXPECT_TRUE(src.contains("m_client->invokeRemoteMethod(\"mod\", \"fn\", QVariantList{p0, p1}, Timeout(), &_err)")); + EXPECT_TRUE(src.contains("m_client->invokeRemoteMethod(\"mod\", \"fn\", QVariantList{QVariant::fromValue(p0), QVariant::fromValue(p1)}, Timeout(), &_err)")); } TEST(MakeSourceTest, ThreeParams) @@ -70,7 +70,7 @@ TEST(MakeSourceTest, ThreeParams) QJsonArray methods; methods.append(makeMethod("fn", "QString", 3)); QString src = makeSource("mod", "Mod", "mod.h", methods); - EXPECT_TRUE(src.contains("p0, p1, p2")); + EXPECT_TRUE(src.contains("QVariant::fromValue(p0), QVariant::fromValue(p1), QVariant::fromValue(p2)")); EXPECT_TRUE(src.contains("return _result.toString()")); } @@ -79,7 +79,7 @@ TEST(MakeSourceTest, FourParams) QJsonArray methods; methods.append(makeMethod("fn", "double", 4)); QString src = makeSource("mod", "Mod", "mod.h", methods); - EXPECT_TRUE(src.contains("p0, p1, p2, p3")); + EXPECT_TRUE(src.contains("QVariant::fromValue(p0), QVariant::fromValue(p1), QVariant::fromValue(p2), QVariant::fromValue(p3)")); EXPECT_TRUE(src.contains("return _result.toDouble()")); } @@ -88,7 +88,7 @@ TEST(MakeSourceTest, FiveParams) QJsonArray methods; methods.append(makeMethod("fn", "float", 5)); QString src = makeSource("mod", "Mod", "mod.h", methods); - EXPECT_TRUE(src.contains("p0, p1, p2, p3, p4")); + EXPECT_TRUE(src.contains("QVariant::fromValue(p0), QVariant::fromValue(p1), QVariant::fromValue(p2), QVariant::fromValue(p3), QVariant::fromValue(p4)")); EXPECT_TRUE(src.contains("return _result.toFloat()")); } @@ -97,10 +97,36 @@ TEST(MakeSourceTest, MoreThanFiveParamsUsesVariantList) QJsonArray methods; methods.append(makeMethod("fn", "QVariant", 6)); QString src = makeSource("mod", "Mod", "mod.h", methods); - EXPECT_TRUE(src.contains("QVariantList{p0, p1, p2, p3, p4, p5}")); + EXPECT_TRUE(src.contains("QVariantList{QVariant::fromValue(p0), QVariant::fromValue(p1), QVariant::fromValue(p2), QVariant::fromValue(p3), QVariant::fromValue(p4), QVariant::fromValue(p5)}")); EXPECT_TRUE(src.contains("return _result")); } +// Regression: a QVariantList-typed ([any]/[int]/...) argument must be wrapped as +// ONE element via QVariant::fromValue. A bare `QVariantList{v}` concatenates the +// list's elements into the args list, sending [1,2,3] as three positional args +// — the "typed arrays empty over the Qt path" bug. +TEST(MakeSourceTest, ListArgWrappedAsOneElement) +{ + QJsonObject m; + m["name"] = "echoList"; + m["returnType"] = "QVariantList"; + m["isInvokable"] = true; + QJsonObject p; + p["type"] = "QVariantList"; + p["name"] = "v"; + QJsonArray params; + params.append(p); + m["parameters"] = params; + QJsonArray methods; + methods.append(m); + + QString src = makeSource("mod", "Mod", "mod.h", methods); + EXPECT_TRUE(src.contains("invokeRemoteMethod(\"mod\", \"echoList\", QVariantList{QVariant::fromValue(v)}, Timeout(), &_err)")); + EXPECT_TRUE(src.contains("invokeRemoteMethodAsync(\"mod\", \"echoList\", QVariantList{QVariant::fromValue(v)}")); + // The bare (spreading) form must not appear. + EXPECT_FALSE(src.contains("QVariantList{v}")); +} + TEST(MakeSourceTest, VoidReturnNoConversion) { QJsonArray methods; From 8966a292837b7c5f6fca1bc1396de7134bee0e79 Mon Sep 17 00:00:00 2001 From: Dario Gabriel Lipicar Date: Sat, 18 Jul 2026 11:43:18 -0300 Subject: [PATCH 2/2] fix(codegen): pass `any` (QVariant) return through raw in the lp wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Qt-free (lp) client generator maps both `any` (QVariant) and the `{tstr:any}` map (QVariantMap) to the same `LogosMap` std type, and decoded a `LogosMap` return as `jv.is_object() ? jv : LogosMap::object()`. For a genuine map that is a no-op, but for `any` it collapsed every NON-object value (a string, a number, an array) to an empty object `{}`. A universal proxy forwarding `echoAny("x")` through this wrapper therefore returned `{}` instead of `"x"` — the concrete cross-version blocker (the UI's runMethods verified echoAny and got FAIL:echoAny through the 2-hop). `lpFromJsonExpr` still receives the original Qt type, so it can tell `any` (mapReturnType == "QVariant") from the map (QVariantMap): pass `any` through unchanged, keep the object coercion only for the map. Test: MakeSourceTest.LpAnyReturnPassesThroughButMapForcesObject. With this + the arg-spread fix, a UI drives the full method surface (incl. echoAny and every array type) through a universal proxy 2-hop end to end. --- cpp-generator/legacy/generator_lib.cpp | 6 +++++ tests/generator/test_make_source.cpp | 34 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/cpp-generator/legacy/generator_lib.cpp b/cpp-generator/legacy/generator_lib.cpp index 8225791..518f197 100644 --- a/cpp-generator/legacy/generator_lib.cpp +++ b/cpp-generator/legacy/generator_lib.cpp @@ -875,6 +875,12 @@ static QString lpFromJsonExpr(const QString& qtType, const QString& jv) if (t == "bool") return "(" + jv + ".is_boolean() ? " + jv + ".get() : false)"; if (t == "std::vector") return "logos::jsonToStringVec(" + jv + ")"; if (t == "std::vector") return "logos::jsonToBytes(" + jv + ")"; + // `any` (QVariant) is a raw json value of ANY shape — pass it through + // unchanged. It shares the LogosMap std type with the `{tstr:any}` map + // (QVariantMap), but only the map is forced to an object below; forcing + // `any` to an object collapsed every non-object value (a string, a number, + // an array) to `{}` (e.g. a proxy forwarding echoAny returned {} for "x"). + if (mapReturnType(qtType) == "QVariant") return jv; if (t == "LogosMap") return "(" + jv + ".is_object() ? " + jv + " : LogosMap::object())"; if (t == "LogosList") return "(" + jv + ".is_array() ? " + jv + " : LogosList::array())"; if (t == "StdLogosResult") return "logos::jsonToStdResult(" + jv + ")"; diff --git a/tests/generator/test_make_source.cpp b/tests/generator/test_make_source.cpp index d955d1d..e1ceafd 100644 --- a/tests/generator/test_make_source.cpp +++ b/tests/generator/test_make_source.cpp @@ -127,6 +127,40 @@ TEST(MakeSourceTest, ListArgWrappedAsOneElement) EXPECT_FALSE(src.contains("QVariantList{v}")); } +// Regression: in the Qt-free (lp) wrapper, an `any` (QVariant) return must pass +// the raw json value through, NOT force it to an object. `any` shares the +// LogosMap std type with the `{tstr:any}` map, and forcing `any` to an object +// collapsed every non-object value to `{}` (e.g. a proxy forwarding echoAny +// returned {} for the string "x"). The map keeps its object coercion. +TEST(MakeSourceTest, LpAnyReturnPassesThroughButMapForcesObject) +{ + QJsonObject any; + any["name"] = "echoAny"; + any["returnType"] = "QVariant"; + any["isInvokable"] = true; + { + QJsonObject p; p["type"] = "QVariant"; p["name"] = "v"; + QJsonArray ps; ps.append(p); any["parameters"] = ps; + } + QJsonObject mp; + mp["name"] = "echoMap"; + mp["returnType"] = "QVariantMap"; + mp["isInvokable"] = true; + { + QJsonObject p; p["type"] = "QVariantMap"; p["name"] = "v"; + QJsonArray ps; ps.append(p); mp["parameters"] = ps; + } + QJsonArray methods; + methods.append(any); + methods.append(mp); + + QString src = makeSourceLp("mod", "Mod", "mod.h", methods); + // `any` return: raw passthrough (return _r;), no is_object coercion. + EXPECT_TRUE(src.contains("return _r;")); + // `{tstr:any}` map return: still forced to an object. + EXPECT_TRUE(src.contains("_r.is_object() ? _r : LogosMap::object()")); +} + TEST(MakeSourceTest, VoidReturnNoConversion) { QJsonArray methods;