diff --git a/cpp-generator/experimental/impl_header_parser.cpp b/cpp-generator/experimental/impl_header_parser.cpp index d5210e3..0f98e13 100644 --- a/cpp-generator/experimental/impl_header_parser.cpp +++ b/cpp-generator/experimental/impl_header_parser.cpp @@ -51,13 +51,151 @@ static QSet g_recordNames; // (same reason g_recordNames is file-static); parseImplHeader drains it. static QStringList g_unmappableSpellings; -static TypeExpr cppTypeToLidl(const QString& raw) +// --------------------------------------------------------------------------- +// Spellings with NO LIDL type at all. +// +// These used to reach the `any` fallback at the bottom of cppTypeToLidl, and +// `any` is ADMITTED by every backend gate — so the declaration was accepted, the +// published contract said `any`, and the generated dispatch handed the raw +// nlohmann::json straight to the author's parameter. That either worked by luck +// through nlohmann's implicit conversions, threw at call time, or emitted a +// non-canonical wire value. Nothing said a word. +// +// Now every one of them is recorded here and parseImplHeader turns the list into +// a hard parse error naming the offending C++ type and the fix. +// +// cppTypeToLidl still RETURNS the historical `any` for these: the diagnostic and +// the mapping are separate, so a declaration whose diagnostic is later discarded +// (a helper struct that never reaches the contract, a reserved lifecycle hook) +// produces byte-identical output to before. +struct UnsupportedSpelling { + QString context; // "method 'foo': parameter 'bar'" + QString record; // non-empty when this came from a record field + QString declared; // the full spelling as written on the declaration + QString offending; // the spelling that has no LIDL type (may be nested) + QString hint; // what to write instead +}; +static QList g_unsupported; + +// Drop the qualifiers that are about how a value is PASSED rather than what it +// is: cppTypeToLidl normalizes with this, and the diagnostics compare against it +// so `const nlohmann::json&` and `nlohmann::json` are recognised as the same +// spelling instead of reading as a type nested inside itself. +static QString normalizeCppSpelling(const QString& raw) { - // Normalize: strip const, &, leading/trailing whitespace QString t = raw.trimmed(); t.remove(QRegularExpression("^const\\s+")); t.remove(QRegularExpression("\\s*&$")); - t = t.trimmed(); + return t.trimmed(); +} + +// Collapse whitespace so `unsigned long int` and `unsigned long int` are one +// key, and drop the redundant `int` from the multi-word integer spellings. +static QString normalizeNumericSpelling(QString t) +{ + t = t.simplified(); + static const QRegularExpression trailingInt("\\s+int$"); + if (t != "int" && t.contains(' ')) + t.remove(trailingInt); + return t; +} + +// What to write instead. Every hint names a spelling that IS in the contract, +// because "unsupported" without a replacement just moves the guesswork. +static QString unsupportedHint(const QString& t) +{ + static const QString kWidenNote = + "Widening is source-compatible for every caller; a narrow type on the " + "wire is not, which is why LIDL has none."; + + // uint8_t has exactly ONE meaning in this contract and it is not a number. + if (t == "uint8_t" || t == "std::uint8_t") + return "uint8_t means BYTES here, and only as `std::vector` " + "(LIDL `bstr`). For a small number declare `uint64_t` (LIDL " + "`uint`); for binary data declare `std::vector`."; + + const QString n = normalizeNumericSpelling(t); + static const QSet kUnsigned = { + "unsigned", "unsigned char", "unsigned short", "unsigned long", + "unsigned long long", "uint16_t", "uint32_t", "size_t", + "uintptr_t", "uintmax_t", + "std::uint16_t", "std::uint32_t", "std::size_t", "std::uintptr_t" + }; + static const QSet kSigned = { + "char", "signed char", "signed", "short", "int", "long", "long long", + "int8_t", "int16_t", "int32_t", "ssize_t", "ptrdiff_t", "intptr_t", + "intmax_t", "std::int8_t", "std::int16_t", "std::int32_t", + "std::ptrdiff_t", "std::intptr_t" + }; + static const QSet kFloating = { "float", "long double" }; + + if (kUnsigned.contains(n)) + return "LIDL numbers are 64-bit only. Declare it `uint64_t` (LIDL " + "`uint`). " + kWidenNote; + if (kSigned.contains(n)) + return "LIDL numbers are 64-bit only. Declare it `int64_t` (LIDL " + "`int`). " + kWidenNote; + if (kFloating.contains(n)) + return "LIDL has one floating type, `float64`. Declare it `double`."; + + if (t.startsWith("std::set<") || t.startsWith("std::unordered_set<") + || t.startsWith("std::multiset<")) + return "LIDL has no set type. Declare it `std::vector` (LIDL `[T]`); " + "uniqueness is not carried on the wire, so the module has to " + "enforce it either way."; + if (t.startsWith("std::pair<") || t.startsWith("std::tuple<")) + return "LIDL has no pair or tuple. Declare a `struct` in this header — " + "it becomes a contract `type` with named fields — or, for " + "key/value data, `std::map` (LIDL `{tstr: V}`). " + "A struct is usually the right answer: positional pairs have no " + "field names for a consumer in another language to bind to."; + if (t.startsWith("std::map<") || t.startsWith("std::unordered_map<") + || t.startsWith("std::multimap<")) + return "LIDL map keys are always `tstr`. Declare it " + "`std::map` / `std::unordered_map`, or a `[T]` of a struct carrying the key as a field."; + if (t.startsWith("std::list<") || t.startsWith("std::deque<") + || t.startsWith("std::array<") || t.startsWith("std::forward_list<")) + return "LIDL's sequence type is `[T]`, spelled `std::vector`. " + "Declare it that way."; + if (t.startsWith("Q")) + return "Qt types cannot appear in a universal impl header — the " + "module's own translation units are Qt-free, and Qt is confined " + "to the generated glue. Use the std spelling (`std::string`, " + "`std::vector`, `std::map`) or the untyped " + "`LogosMap` / `LogosList`."; + if (t.endsWith("*") || t.endsWith("&&")) + return "A pointer or rvalue reference has no wire form. Pass the value " + "(by value or `const T&`), or a `struct` declared in this " + "header."; + + return "The recognised spellings are: `bool`, `int64_t`, `uint64_t`, " + "`double`, `std::string`, `std::vector` (bytes), " + "`std::optional`, `std::vector`, `std::map`, " + "`std::unordered_map`, `LogosMap` / `LogosList` / " + "`nlohmann::json` (untyped JSON), `StdLogosResult` and `void` as " + "returns, plus any `struct` declared in this header. Rewrite the " + "declaration with one of them, or declare a struct for it."; +} + +// `context` names the declaration being typed ("method 'foo': parameter 'bar'") +// and `declared` the full spelling on it, so a nested offender reports both the +// element that has no LIDL type and the declaration that carries it. `record` is +// set only while typing a struct's fields, so a diagnostic can be withdrawn when +// the struct turns out never to reach the contract. +// +// `nameEmitted` marks the slots whose C++ spelling the generator WRITES OUT into +// code the author's own declaration has to match — a record field's codec, an +// event's generated body. In those the derived spelling is a constraint on the +// author; everywhere else the generated code only has to consume or produce a +// value, and can adapt to whatever the author declared. +static TypeExpr cppTypeToLidl(const QString& raw, const QString& context = QString(), + const QString& declared = QString(), + const QString& record = QString(), + bool nameEmitted = false) +{ + // Normalize: strip const, &, leading/trailing whitespace + QString t = normalizeCppSpelling(raw); // Primitives if (t == "bool") return { TypeExpr::Primitive, "bool", {} }; @@ -113,7 +251,7 @@ static TypeExpr cppTypeToLidl(const QString& raw) // [{tstr: int}]. Without it the element list above was exhaustive and // every other vector fell all the way through to the opaque `any`, // which then encoded a record as a LogosMap. - return { TypeExpr::Array, "", { cppTypeToLidl(inner) } }; + return { TypeExpr::Array, "", { cppTypeToLidl(inner, context, declared, record, nameEmitted) } }; } // Qt collection types — pass through directly (non-std-convertible) @@ -131,17 +269,60 @@ static TypeExpr cppTypeToLidl(const QString& raw) if (t == "LogosList") return { TypeExpr::Array, "", { {TypeExpr::Primitive, "any", {}} } }; + // The alias spelled out. LogosMap / LogosList ARE nlohmann::json, and real + // modules write the underlying name — test_fullapi_cpp's `echoAny` / + // `fireAnyEvent` / `anyEvent`, and both full_api interface headers, all + // declare `nlohmann::json`. It reached `any` ONLY through the fallback at + // the bottom of this function, so naming it here is a PREREQUISITE for + // turning that fallback into an error: without this branch the whole + // cross-language conformance chain stops building. + // + // Mapped to the bare `any` primitive rather than LogosMap's `{tstr: any}` / + // LogosList's `[any]`: `nlohmann::json` is an untyped value of ANY kind, not + // specifically an object or an array. That is the type the fallback already + // produced for it, so nothing about the published contract moves. + if (t == "nlohmann::json" || t == "json") + return { TypeExpr::Primitive, "any", {} }; + // StdLogosResult — pure C++ result type for universal impls. The generator // emits a StdLogosResult→Qt LogosResult conversion in the glue layer. if (t == "StdLogosResult") return { TypeExpr::Primitive, "result", {} }; - // std::map -> {tstr: T}. Absent before, so a typed map was - // unspellable header-first and fell through to `any`. - static QRegularExpression mapRe("^std::map\\s*<\\s*std::string\\s*,\\s*(.+)\\s*>$"); + // std::map / std::unordered_map -> {tstr: T}. Absent before, + // so a typed map was unspellable header-first and fell through to `any`. + // + // Both containers, because logos_codec.h specializes Codec for both and they + // are the same wire shape — a JSON object. Only the KEY is constrained: a + // non-`std::string` key falls through to the unsupported report below, since + // `{tstr: T}` is the only map LIDL has. + static QRegularExpression mapRe( + "^std::(?:unordered_)?map\\s*<\\s*std::string\\s*,\\s*(.+)\\s*>$"); QRegularExpressionMatch mm = mapRe.match(t); if (mm.hasMatch()) { - TypeExpr val = cppTypeToLidl(mm.captured(1).trimmed()); + // ...with one boundary. In a `nameEmitted` slot the generator WRITES the + // spelling out — a record field's codec says `Codec>` and + // an event's generated body repeats the parameter list the author + // declared. It has to pick one of the two container names there, and + // picking the wrong one is a compile error in code the author never + // wrote. Method parameters and returns have no such constraint: they go + // through logos::JsonArg / deduced logos::toJson, which instantiate with + // whatever the author declared. + if (t.startsWith("std::unordered_map") && nameEmitted && !context.isEmpty()) { + UnsupportedSpelling u; + u.context = context; + u.record = record; + u.declared = declared.isEmpty() ? t : declared; + u.offending = t; + u.hint = "`{tstr: T}` has two C++ spellings and this slot's spelling " + "is written into generated code your own declaration has to " + "match, so it can only be one of them: declare it " + "`std::map`. (A method parameter or return " + "may use either container — those are decoded and encoded " + "through your declared type, not a named one.)"; + g_unsupported.append(u); + } + TypeExpr val = cppTypeToLidl(mm.captured(1).trimmed(), context, declared, record, nameEmitted); return { TypeExpr::Map, "", { {TypeExpr::Primitive, "tstr", {}}, val } }; } @@ -157,7 +338,7 @@ static TypeExpr cppTypeToLidl(const QString& raw) static QRegularExpression optRe("^std::optional\\s*<\\s*(.+)\\s*>$"); QRegularExpressionMatch om = optRe.match(t); if (om.hasMatch()) { - TypeExpr inner = cppTypeToLidl(om.captured(1).trimmed()); + TypeExpr inner = cppTypeToLidl(om.captured(1).trimmed(), context, declared, record, nameEmitted); // std::optional> has NO LIDL type. // // `?T` is two-state, and optionality is idempotent under that rule — so @@ -183,7 +364,34 @@ static TypeExpr cppTypeToLidl(const QString& raw) if (g_recordNames.contains(t)) return { TypeExpr::Named, t.toStdString(), {} }; - // Fallback: treat as opaque + // NOTHING above matched: this spelling has no LIDL type. + // + // It used to return the opaque `any` right here, silently. `any` is admitted + // by every backend gate, so the declaration was accepted and the generated + // dispatch handed the raw nlohmann::json to the author's parameter with no + // `logos::fromJson<>` and no check — the one hole left open after #113-#122 + // closed it for every TYPED slot. A `std::vector` parameter + // published `[any]` and worked by accident; a + // `std::vector>` published `[any]` and + // shipped raw binary through a UTF-8 string. + // + // The return value is UNCHANGED (`any`) on purpose: mapping and diagnosis + // are separate concerns. A diagnostic that is later withdrawn — a helper + // struct that never reaches the contract, a reserved lifecycle hook — must + // leave the emitted output byte-identical to what it was. + // + // An empty spelling is not a C++ type at all, it is this line-based parser + // failing to find one (a macro, a member initialiser). Reporting "'' has no + // LIDL type" would be noise, so it keeps the old behaviour. + if (!t.isEmpty() && !context.isEmpty()) { + UnsupportedSpelling u; + u.context = context; + u.record = record; + u.declared = declared.isEmpty() ? t : declared; + u.offending = t; + u.hint = unsupportedHint(t); + g_unsupported.append(u); + } return { TypeExpr::Primitive, "any", {} }; } @@ -219,6 +427,9 @@ static std::vector scanForRecords(const QStringList& lines) TypeDecl td; td.name = om.captured(1).toStdString(); + // Withdraw this struct's diagnostics if it turns out to declare no + // fields at all — nothing is published, so nothing is misreported. + const int diagMark = g_unsupported.size(); for (int j = i + 1; j < lines.size(); ++j) { const QString body = lines.at(j).trimmed(); if (body.startsWith("};")) break; @@ -235,10 +446,15 @@ static std::vector scanForRecords(const QStringList& lines) if (!fm.hasMatch()) continue; FieldDecl fd; fd.name = fm.captured(2).toStdString(); - fd.type = cppTypeToLidl(fm.captured(1).trimmed()); + const QString spelling = fm.captured(1).trimmed(); + fd.type = cppTypeToLidl( + spelling, + QString("type '%1': field '%2'").arg(om.captured(1), fm.captured(2)), + spelling, om.captured(1), /*nameEmitted=*/true); td.fields.push_back(fd); } if (!td.fields.empty()) out.push_back(td); + else while (g_unsupported.size() > diagMark) g_unsupported.removeLast(); } return out; } @@ -296,7 +512,11 @@ static void keepOnlyReferencedRecords(ModuleDecl& module) // Parse a single method declaration line // --------------------------------------------------------------------------- -static bool parseMethodLine(const QString& line, MethodDecl& out) +// `kind` is "method" or "event" — it only labels the diagnostics an unsupported +// C++ spelling produces, so the report matches the section the declaration was +// written in rather than the function that happens to parse both. +static bool parseMethodLine(const QString& line, MethodDecl& out, + const QString& kind = "method") { // Find the parameter list: everything between the last '(' and ')' int parenOpen = -1; @@ -347,7 +567,9 @@ static bool parseMethodLine(const QString& line, MethodDecl& out) return false; out.name = methodName.toStdString(); QString retTypeStr = stripDeclarationSpecifiers(prefix.left(nameStart).trimmed()); - out.returnType = cppTypeToLidl(retTypeStr); + out.returnType = cppTypeToLidl( + retTypeStr, QString("%1 '%2': return type").arg(kind, methodName), retTypeStr, + QString(), /*nameEmitted=*/kind == "event"); // Flag methods whose impl returns LogosMap / LogosList so the generator // can emit nlohmann→Qt conversion code in the glue layer. out.jsonReturn = (retTypeStr == "LogosMap" || retTypeStr == "LogosList"); @@ -385,8 +607,13 @@ static bool parseMethodLine(const QString& line, MethodDecl& out) if (pNameStart >= pNameEnd) continue; ParamDecl pd; - pd.name = p.mid(pNameStart, pNameEnd - pNameStart).toStdString(); - pd.type = cppTypeToLidl(p.left(pNameStart)); + const QString pName = p.mid(pNameStart, pNameEnd - pNameStart); + const QString pSpelling = p.left(pNameStart).trimmed(); + pd.name = pName.toStdString(); + pd.type = cppTypeToLidl( + p.left(pNameStart), + QString("%1 '%2': parameter '%3'").arg(kind, methodName, pName), + pSpelling, QString(), /*nameEmitted=*/kind == "event"); out.params.push_back(pd); } } @@ -413,10 +640,15 @@ ImplParseResult parseImplHeader(const QString& headerPath, { ImplParseResult result; - // Both file-statics are per-parse state: one process generates for more than - // one module. Cleared here rather than next to their first use because the - // metadata's event params are typed before the header is even read. + // All three file-statics are per-parse state: one process generates for more + // than one module. g_recordNames is cleared HERE as well as beside + // scanForRecords, because a name left over from the previous module's header + // would otherwise be visible while this one's metadata events are typed. g_unmappableSpellings.clear(); + g_unsupported.clear(); + g_recordNames.clear(); + + QJsonArray metadataEvents; // --- Read metadata.json --- { @@ -440,24 +672,12 @@ ImplParseResult parseImplHeader(const QString& headerPath, for (const QString& depName : dependencyNames(deps)) result.module.depends.push_back(depName.toStdString()); - // Read events declared in metadata.json - QJsonArray events = obj.value("events").toArray(); - for (const QJsonValue& ev : events) { - QJsonObject evObj = ev.toObject(); - EventDecl ed; - ed.name = evObj.value("name").toString().toStdString(); - ed.description = evObj.value("description").toString().toStdString(); - QJsonArray params = evObj.value("params").toArray(); - for (const QJsonValue& pv : params) { - QJsonObject po = pv.toObject(); - ParamDecl pd; - pd.name = po.value("name").toString().toStdString(); - pd.type = cppTypeToLidl(po.value("type").toString()); - ed.params.push_back(pd); - } - if (!ed.name.empty()) - result.module.events.push_back(ed); - } + // Events declared in metadata.json. Only READ here — their parameter + // types are C++ spellings like any other, and typing them requires the + // record set, which does not exist until the header has been scanned. + // They used to be typed right here, against whatever g_recordNames the + // PREVIOUS module's parse left behind. + metadataEvents = obj.value("events").toArray(); } // --- Read and parse header --- @@ -529,6 +749,31 @@ ImplParseResult parseImplHeader(const QString& headerPath, g_recordNames.clear(); result.module.types = scanForRecords(lines); + // Now the record set exists, the metadata-declared events can be typed. They + // stay AHEAD of the header's `logos_events:` events, as they always were. + for (const QJsonValue& ev : metadataEvents) { + QJsonObject evObj = ev.toObject(); + EventDecl ed; + ed.name = evObj.value("name").toString().toStdString(); + ed.description = evObj.value("description").toString().toStdString(); + const QJsonArray params = evObj.value("params").toArray(); + for (const QJsonValue& pv : params) { + QJsonObject po = pv.toObject(); + ParamDecl pd; + const QString pName = po.value("name").toString(); + const QString pType = po.value("type").toString(); + pd.name = pName.toStdString(); + pd.type = cppTypeToLidl( + pType, + QString("event '%1': parameter '%2' (declared in metadata.json)") + .arg(evObj.value("name").toString(), pName), + pType, QString(), /*nameEmitted=*/true); + ed.params.push_back(pd); + } + if (!ed.name.empty()) + result.module.events.push_back(ed); + } + // State machine: find "class ", then collect declarations. // `InLogosEvents` is entered by the literal `logos_events:` token // (mirrors Qt's `signals:`) — methods declared there are parsed as @@ -688,7 +933,7 @@ ImplParseResult parseImplHeader(const QString& headerPath, if (line.endsWith(';')) { QString decl = line.left(line.size() - 1).trimmed(); MethodDecl md; - if (parseMethodLine(decl, md)) { + if (parseMethodLine(decl, md, "event")) { EventDecl ed; ed.name = md.name; ed.params = md.params; @@ -715,6 +960,10 @@ ImplParseResult parseImplHeader(const QString& headerPath, if (line.endsWith(';')) { QString decl = line.left(line.size() - 1).trimmed(); MethodDecl md; + // Withdraw the declaration's diagnostics if it turns out to be a + // reserved lifecycle hook: it is not part of the contract, so an + // unsupported spelling in it is not a contract defect. + const int diagMark = g_unsupported.size(); if (parseMethodLine(decl, md)) { // LogosModuleContext lifecycle hooks / context accessors are // framework plumbing, not part of the module's API contract. @@ -731,6 +980,9 @@ ImplParseResult parseImplHeader(const QString& headerPath, if (!reserved.contains(qs(md.name))) { md.description = joinDocLines(pendingDoc).toStdString(); result.module.methods.push_back(md); + } else { + while (g_unsupported.size() > diagMark) + g_unsupported.removeLast(); } } } @@ -745,6 +997,49 @@ done: // contract types. keepOnlyReferencedRecords(result.module); + // A C++ spelling with no LIDL type is a BUILD ERROR, not a silent `any`. + // + // Reported after keepOnlyReferencedRecords so a helper struct that never + // reaches the contract cannot fail the build: publishing is what makes a + // declaration's type a promise, and an internal struct promises nothing. + { + std::set published; + for (const TypeDecl& td : result.module.types) published.insert(td.name); + + QStringList reports; + QSet seen; + for (const UnsupportedSpelling& u : g_unsupported) { + if (!u.record.isEmpty() && !published.count(u.record.toStdString())) + continue; // struct dropped: not part of the contract + QString line = " " + u.context; + // "declared X, whose element Y" only when Y really is nested inside + // X — not when the two differ by a `const` and an `&`. + if (normalizeCppSpelling(u.declared) != u.offending) + line += QString(" is declared `%1`, whose element `%2` has no " + "LIDL type.\n ").arg(u.declared, u.offending); + else + line += QString(" is `%1`, which has no LIDL type.\n ") + .arg(u.offending); + line += u.hint; + if (seen.contains(line)) continue; + seen.insert(line); + reports << line; + } + if (!reports.isEmpty()) { + result.error = + headerPath + ": " + QString::number(reports.size()) + + (reports.size() == 1 ? " declaration uses" : " declarations use") + + " a C++ type that has no LIDL type.\n\n" + + reports.join("\n\n") + + "\n\nEach of these used to be published as the opaque `any`, with no " + "diagnostic. `any` is admitted by every backend gate, so the generated " + "dispatch handed the raw JSON straight to the parameter with no decode " + "and no check — the value either converted by luck, threw at call time, " + "or went onto the wire in a form no other language decodes.\n"; + return result; + } + } + if (!g_unmappableSpellings.isEmpty()) { g_unmappableSpellings.removeDuplicates(); err << "Warning: " << headerPath << ": " << g_unmappableSpellings.join(", ") diff --git a/cpp-generator/experimental/lidl_gen_cdylib.cpp b/cpp-generator/experimental/lidl_gen_cdylib.cpp index ae20ca3..11bf34c 100644 --- a/cpp-generator/experimental/lidl_gen_cdylib.cpp +++ b/cpp-generator/experimental/lidl_gen_cdylib.cpp @@ -142,6 +142,23 @@ QString jsonArgToStd(const TypeExpr& te, const QString& expr, const QString& pat const QString cpp = lidlTypeToStdCdylib(te, recs); if (cpp == "LogosMap" || cpp == "LogosList") return expr; // untyped JSON passes through, as it always has + // A TYPED map does not NAME its C++ type — it hands the compiler a proxy and + // lets the author's own declaration pick it. + // + // `{tstr: T}` has two C++ spellings, std::map and std::unordered_map, and + // logos_codec.h specializes Codec for both. Naming one of them here would + // silently make the other a compile error in generated code the author never + // wrote: `logos::fromJson>` returns a std::map, and a std::map + // does not convert to an unordered_map parameter. logos::JsonArg instantiates + // the conversion with the EXACT parameter type instead, so both spellings + // decode — through the same Codec, with the same path in the same error. + // + // Only maps: every other LIDL type has exactly one C++ spelling here, and + // JsonArg documents one type it cannot serve (std::optional, whose own + // converting constructor out-ranks the proxy's conversion operator) — the + // Optional branch above returns before reaching this line. + if (te.kind == TypeExpr::Map) + return "logos::JsonArg(" + expr + ", \"" + path + "\")"; return "logos::fromJson<" + cpp + ">(" + expr + ", \"" + path + "\")"; } @@ -164,12 +181,17 @@ QString stdReturnToJson(const MethodDecl& md, const QString& var, return var; // LogosMap / LogosList are nlohmann::json already } if (te.kind == TypeExpr::Primitive) { - if (te.name == "bstr") return "lidlBytesToJson(" + var + ")"; + if (te.name == "bstr") return "logos::bytesToJson(" + var + ")"; if (te.name == "any") return var; return "nlohmann::json(" + var + ")"; } if (cppRet == "LogosMap" || cppRet == "LogosList") return var; + // Same reason the map ARGUMENT does not name its type: `{tstr: T}` is both + // std::map and std::unordered_map, so let the return variable's own type be + // deduced rather than asserting one of them. + if (te.kind == TypeExpr::Map) + return "logos::toJson(" + var + ")"; // `nlohmann::json(v)` would serialize a vector as a plain number // array and a record not at all; the codec keeps bytes tagged at depth. return "logos::toJson<" + cppRet + ">(" + var + ")"; @@ -372,15 +394,6 @@ void emitRecordCodecs(QTextStream& s, const ModuleDecl& module, s << "}} // namespace logos::detail\n\n"; } -bool hasBytesEventParam(const ModuleDecl& module) -{ - for (const EventDecl& ed : module.events) - for (const ParamDecl& pd : ed.params) - if (pd.type.kind == TypeExpr::Primitive && pd.type.name == "bstr") - return true; - return false; -} - // The Qt spelling of what actually crosses the Qt boundary. // // NOT lidlTypeToQt: that answers the CONSUMER's question ("what type does the @@ -420,35 +433,23 @@ bool hasJsonEventParam(const ModuleDecl& module) return false; } -// The SCALAR tagged-bytes helpers. A `[bstr]` (and bytes at any deeper -// nesting) rides logos::Codec instead: its full specialization for -// std::vector beats the generic vector rule, so one mechanism covers -// [bstr], [[bstr]] and {tstr: [bstr]} alike. #111 emitted a dedicated depth-1 -// list codec here; the generic one subsumes it, and keeping both left an -// unused static in every module that mentioned [bstr]. -void emitBytesEncodeHelpers(QTextStream& s) -{ - s << "// Canonical tagged bytes form {\"_bytes\": base64url} (see logos_protocol.h)\n"; - s << "std::string lidlB64UrlEncode(const std::vector& bytes)\n{\n"; - s << " static const char* alpha = \"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_\";\n"; - s << " std::string out;\n"; - s << " size_t i = 0;\n"; - s << " while (i + 3 <= bytes.size()) {\n"; - s << " uint32_t n = (uint32_t(bytes[i]) << 16) | (uint32_t(bytes[i+1]) << 8) | uint32_t(bytes[i+2]);\n"; - s << " out += alpha[(n >> 18) & 0x3f]; out += alpha[(n >> 12) & 0x3f];\n"; - s << " out += alpha[(n >> 6) & 0x3f]; out += alpha[n & 0x3f];\n"; - s << " i += 3;\n }\n"; - s << " if (i < bytes.size()) {\n"; - s << " uint32_t n = uint32_t(bytes[i]) << 16;\n"; - s << " if (i + 1 < bytes.size()) n |= uint32_t(bytes[i+1]) << 8;\n"; - s << " out += alpha[(n >> 18) & 0x3f]; out += alpha[(n >> 12) & 0x3f];\n"; - s << " if (i + 1 < bytes.size()) out += alpha[(n >> 6) & 0x3f];\n"; - s << " }\n return out;\n}\n\n"; - - s << "nlohmann::json lidlBytesToJson(const std::vector& bytes)\n{\n"; - s << " return nlohmann::json{{\"_bytes\", lidlB64UrlEncode(bytes)}};\n}\n\n"; - -} +// The generated base64 codec is GONE — all of it. +// +// #117 replaced the emitted generic codec with logos-protocol's logos_codec.h, +// but left behind the base64 pair it had grown around: an encoder +// (lidlB64UrlEncode / lidlBytesToJson) and a decoder (lidlB64Idx / +// lidlBytesFromJson), ~89 emitted lines in every module's export TU. The decoder +// had no call site at all — every byte parameter had already moved to +// logos::bytesFromJsonLenient — and the encoder was a byte-for-byte reimplementation +// of logos::bytesToJson, which is included via in the very same +// translation unit. +// +// A second copy of an encoder is not free: this is the arrangement that let the +// emitted and canonical halves drift over padded base64 once already, and it is +// exactly the duplication #117's own comment set out to end. Scalar `bstr` slots +// now call logos::bytesToJson directly, which is what every composite slot +// (`[bstr]`, `{tstr: bstr}`, records) has been doing through logos::Codec since +// #117. void emitInterfaceJson(QTextStream& s, const ModuleDecl& module) { @@ -650,64 +651,6 @@ QString lidlMakeModuleImplExports(const ModuleDecl& module, s << " if (out) std::memcpy(out, str.data(), str.size() + 1);\n"; s << " return out;\n}\n\n"; - emitBytesEncodeHelpers(s); - - s << "int lidlB64Idx(char ch)\n{\n"; - s << " if (ch >= 'A' && ch <= 'Z') return ch - 'A';\n"; - s << " if (ch >= 'a' && ch <= 'z') return ch - 'a' + 26;\n"; - s << " if (ch >= '0' && ch <= '9') return ch - '0' + 52;\n"; - s << " if (ch == '-') return 62;\n if (ch == '_') return 63;\n return -1;\n}\n\n"; - - s << "std::vector lidlBytesFromJson(const nlohmann::json& j)\n{\n"; - s << " std::vector out;\n"; - s << " // Lenient bytes decode (matches the std path, where a QString or\n"; - s << " // QByteArray arg both became bytes): a caller may send the tagged\n"; - s << " // {\"_bytes\": base64url} form, a plain string (raw UTF-8 bytes), or\n"; - s << " // an array of byte values. Only the tagged form needs base64.\n"; - s << " if (j.is_string()) {\n"; - s << " const std::string s = j.get();\n"; - s << " out.assign(s.begin(), s.end());\n"; - s << " return out;\n"; - s << " }\n"; - s << " if (j.is_number()) {\n"; - s << " // A number arg becomes its decimal text as bytes — matches\n"; - s << " // Qt's QVariant(int)->QByteArray, so a caller (or the\n"; - s << " // logoscore CLI's type auto-detection) passing a bare number\n"; - s << " // to a bytes param behaves the same as the Qt path.\n"; - s << " const std::string s = j.dump();\n"; - s << " out.assign(s.begin(), s.end());\n"; - s << " return out;\n"; - s << " }\n"; - s << " if (j.is_array()) {\n"; - s << " for (const auto& e : j)\n"; - s << " if (e.is_number_integer() || e.is_number_unsigned())\n"; - s << " out.push_back(static_cast(e.get() & 0xff));\n"; - s << " return out;\n"; - s << " }\n"; - s << " if (!j.is_object() || j.size() != 1 || !j.contains(\"_bytes\") || !j[\"_bytes\"].is_string())\n"; - s << " return out;\n"; - s << " const std::string s64 = j[\"_bytes\"].get();\n"; - s << " size_t i = 0;\n"; - s << " while (i + 4 <= s64.size()) {\n"; - s << " int a = lidlB64Idx(s64[i]), b = lidlB64Idx(s64[i+1]), c2 = lidlB64Idx(s64[i+2]), d = lidlB64Idx(s64[i+3]);\n"; - s << " if (a < 0 || b < 0 || c2 < 0 || d < 0) return {};\n"; - s << " uint32_t n = (uint32_t(a) << 18) | (uint32_t(b) << 12) | (uint32_t(c2) << 6) | uint32_t(d);\n"; - s << " out.push_back((n >> 16) & 0xff); out.push_back((n >> 8) & 0xff); out.push_back(n & 0xff);\n"; - s << " i += 4;\n }\n"; - s << " size_t rem = s64.size() - i;\n"; - s << " if (rem == 2 || rem == 3) {\n"; - s << " int a = lidlB64Idx(s64[i]), b = lidlB64Idx(s64[i+1]);\n"; - s << " if (a < 0 || b < 0) return {};\n"; - s << " uint32_t n = (uint32_t(a) << 18) | (uint32_t(b) << 12);\n"; - s << " out.push_back((n >> 16) & 0xff);\n"; - s << " if (rem == 3) {\n"; - s << " int c2 = lidlB64Idx(s64[i+2]);\n"; - s << " if (c2 < 0) return {};\n"; - s << " n |= uint32_t(c2) << 6;\n"; - s << " out.push_back((n >> 8) & 0xff);\n"; - s << " }\n }\n return out;\n}\n\n"; - - s << "nlohmann::json lidlResultToJson(const StdLogosResult& r)\n{\n"; s << " nlohmann::json obj;\n"; s << " obj[\"success\"] = r.success;\n"; @@ -807,7 +750,32 @@ QString lidlMakeModuleImplExports(const ModuleDecl& module, for (size_t i = 0; i < md.params.size(); ++i) if (!paramIsOptional(md.params[i])) minArgs = i + 1; s << " if (m == \"" << md.name << "\") {\n"; - s << " if (args.size() < " << minArgs << ") return nullptr;\n"; + // A wrong argument COUNT is reported, not swallowed. + // + // This used to be `return nullptr`, and the Qt glue turns a NULL reply + // into an empty QVariant — indistinguishable from a method that + // legitimately returned nothing. "You passed 2 of 4 arguments" looked + // like a successful empty answer. + // + // The shape is the one logos-rust-sdk's args::invalid_args() already + // emits (src/args.rs), so a C++ and a Rust provider answer a malformed + // call identically — which is what that module's + // invalid_args_shape_matches_cpp test claims, and what was not true + // until now. Same three keys, same message text, same `origin`. + // + // Emitted only when the method has at least one REQUIRED parameter: + // `args.size() < 0` is unsigned-compared and always false, so a zero-arg + // method carried a dead branch (the Rust generator skips it for the same + // reason). + if (minArgs > 0) { + s << " if (args.size() < " << minArgs << ") {\n"; + s << " nlohmann::json err{{\"code\", \"invalid_args\"},\n"; + s << " {\"message\", \"expected " << minArgs + << " arguments, got \" + std::to_string(args.size())},\n"; + s << " {\"origin\", \"" << module.name << "\"}};\n"; + s << " return lidlStrdup(err.dump());\n"; + s << " }\n"; + } QString call = "lidlImpl()." + qs(md.name) + "("; for (size_t i = 0; i < md.params.size(); ++i) { const QString expr = (i < minArgs) @@ -913,13 +881,11 @@ QString lidlMakeEventsSourceCdylib(const ModuleDecl& module, s << "#include \n"; s << "\n"; - // Only the modules that actually emit binary event payloads need the bytes - // encoder; emitting it everywhere would leave it unused (and warned about). - if (hasBytesEventParam(module)) { - s << "namespace {\n\n"; - emitBytesEncodeHelpers(s); - s << "} // namespace\n\n"; - } + // No local bytes encoder any more, and so no hasBytesEventParam() gate for + // it either: a `bstr` event parameter calls logos::bytesToJson, which the + // pulled in by "_types.h" above already provides. + // The gate existed only to keep the emitted copy from sitting unused in + // modules whose events carry no binary data. for (const EventDecl& ed : module.events) { s << "void " << implClass << "::" << ed.name << "("; @@ -964,7 +930,7 @@ QString lidlMakeEventsSourceCdylib(const ModuleDecl& module, continue; } if (pd.type.kind == TypeExpr::Primitive && pd.type.name == "bstr") - s << " args.push_back(lidlBytesToJson(" << pd.name << "));\n"; + s << " args.push_back(logos::bytesToJson(" << pd.name << "));\n"; else s << " args.push_back(" << pd.name << ");\n"; } diff --git a/doctests/cpp-sdk-generator-roundtrip.test.yaml b/doctests/cpp-sdk-generator-roundtrip.test.yaml index 9b3168e..8666bbe 100644 --- a/doctests/cpp-sdk-generator-roundtrip.test.yaml +++ b/doctests/cpp-sdk-generator-roundtrip.test.yaml @@ -262,7 +262,7 @@ sections: - "void SensorModuleImpl::reading(uint64_t id, double value)" - "args.push_back(value);" - "void SensorModuleImpl::capture(uint64_t id, const std::vector& frame)" - - "args.push_back(lidlBytesToJson(frame));" + - "args.push_back(logos::bytesToJson(frame));" - title: "Flow 3 — LIDL → consumer header" step: true diff --git a/tests/experimental/test_impl_header_parser.cpp b/tests/experimental/test_impl_header_parser.cpp index f5588da..63857f2 100644 --- a/tests/experimental/test_impl_header_parser.cpp +++ b/tests/experimental/test_impl_header_parser.cpp @@ -689,3 +689,244 @@ TEST_F(ImplHeaderParserTest, NestedOptionalCollapsesAndIsReported) << errOutput.toStdString(); EXPECT_TRUE(errOutput.contains("no LIDL type")) << errOutput.toStdString(); } + +// A one-class header written to a temp dir and parsed, for the cases that are +// about a single declaration rather than a whole fixture module. +namespace { + +QString probeHeader(QTemporaryDir& dir, const QString& body) +{ + const QString hp = dir.filePath("probe_impl.h"); + QFile f(hp); + if (!f.open(QIODevice::WriteOnly | QIODevice::Text)) + return QString(); + f.write(("#pragma once\n" + "#include \n" + "#include \n" + "#include \n" + "#include \n" + "#include \n" + "#include \n" + "#include \n" + "#include \n" + "#include \n" + + body).toUtf8()); + return hp; +} + +} // namespace + +// `nlohmann::json` reaches `any` only through the fallback, and `any` is the +// RIGHT answer for it — test_fullapi_cpp's echoAny / fireAnyEvent / anyEvent +// are the conformance matrix's `any` cells. Naming it is what lets the fallback +// become an error without taking them out. +TEST_F(ImplHeaderParserTest, NlohmannJsonIsAnyByName) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "class ProbeImpl {\n" + "public:\n" + " nlohmann::json echoAny(const nlohmann::json& v);\n" + " bool fireAnyEvent(const nlohmann::json& v);\n" + "logos_events:\n" + " void anyEvent(const nlohmann::json& v);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + + ASSERT_EQ(r.module.methods.size(), 2u); + EXPECT_EQ(r.module.methods[0].returnType.name, "any"); + EXPECT_EQ(r.module.methods[0].params[0].type.name, "any"); + EXPECT_EQ(r.module.methods[1].params[0].type.name, "any"); + ASSERT_EQ(r.module.events.size(), 1u); + EXPECT_EQ(r.module.events[0].params[0].type.name, "any"); +} + +// --------------------------------------------------------------------------- +// A C++ spelling with no LIDL type is a BUILD ERROR, not a silent `any`. +// +// cppTypeToLidl used to end with `// Fallback: treat as opaque` -> `any`, and +// `any` is admitted by every backend gate. So an unrecognised spelling was +// accepted in silence, published as `any`, and dispatched as a raw +// `lidlImpl().f(args.at(0))` with no decode and no check. +// --------------------------------------------------------------------------- + +TEST_F(ImplHeaderParserTest, NarrowNumericIsRejectedWithTheWidening) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "class ProbeImpl {\n" + "public:\n" + " int64_t f(uint32_t depth);\n" + "};\n"); + ASSERT_FALSE(hp.isEmpty()); + + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_TRUE(r.hasError()) << "uint32_t was admitted as `any`"; + // Names the declaration, the offending type, and what to write instead. + EXPECT_TRUE(r.error.contains("method 'f': parameter 'depth'")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("`uint32_t`")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("`uint64_t`")) << r.error.toStdString(); +} + +// The offender may be nested. Report BOTH: the element with no LIDL type, and +// the declaration that carries it. +TEST_F(ImplHeaderParserTest, NestedOffenderNamesTheDeclarationToo) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "class ProbeImpl {\n" + "public:\n" + " int64_t f(const std::vector& instruction);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_TRUE(r.hasError()); + EXPECT_TRUE(r.error.contains("const std::vector&")) << r.error.toStdString(); + EXPECT_TRUE(r.error.contains("`uint32_t`")) << r.error.toStdString(); +} + +// Each family gets a hint that names a replacement. A diagnostic without one +// just moves the guesswork. +TEST_F(ImplHeaderParserTest, EachUnsupportedFamilyNamesItsReplacement) +{ + struct Case { const char* decl; const char* mentions; }; + const Case cases[] = { + {"int64_t f(float v);", "`double`"}, + {"int64_t f(uint8_t v);", "std::vector"}, + {"int64_t f(size_t v);", "`uint64_t`"}, + {"int64_t f(const std::set& v);", "std::vector"}, + {"int64_t f(const std::pair& v);", "struct"}, + {"int64_t f(const std::map& v);", "`tstr`"}, + }; + for (const Case& c : cases) { + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + QString("class ProbeImpl {\npublic:\n %1\n};\n").arg(c.decl)); + QString e; + QTextStream es(&e); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", es); + ASSERT_TRUE(r.hasError()) << c.decl; + EXPECT_TRUE(r.error.contains(c.mentions)) + << c.decl << "\n" << r.error.toStdString(); + } +} + +// std::unordered_map is the second C++ spelling of `{tstr: T}`. +// logos_codec.h has always specialized Codec for it; the parser had not, so it +// published `any`. +TEST_F(ImplHeaderParserTest, UnorderedMapIsAStringKeyedMap) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "class ProbeImpl {\n" + "public:\n" + " int64_t f(const std::unordered_map& m);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.methods.size(), 1u); + const TypeExpr& t = r.module.methods[0].params[0].type; + EXPECT_EQ(t.kind, TypeExpr::Map); + ASSERT_EQ(t.elements.size(), 2u); + EXPECT_EQ(t.elements[0].name, "tstr"); + EXPECT_EQ(t.elements[1].name, "tstr"); +} + +// ...except in the two slots whose C++ spelling the generator WRITES OUT — a +// record field's codec and an event's generated body. There it has to pick one +// container name, and picking the wrong one is a compile error in code the +// author never wrote. Say so at the declaration instead. +TEST_F(ImplHeaderParserTest, UnorderedMapIsRejectedWhereTheSpellingIsEmitted) +{ + for (const char* body : { + "struct Rec {\n" + " std::unordered_map m;\n" + "};\n" + "class ProbeImpl {\npublic:\n Rec echo(const Rec& v);\n};\n", + + "class ProbeImpl {\n" + "public:\n" + " bool fire();\n" + "logos_events:\n" + " void changed(const std::unordered_map& m);\n" + "};\n"}) { + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, body); + QString e; + QTextStream es(&e); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", es); + ASSERT_TRUE(r.hasError()) << body; + EXPECT_TRUE(r.error.contains("std::map")) << r.error.toStdString(); + } +} + +// A helper struct the API never mentions is dropped from the contract, so an +// unsupported spelling INSIDE it promises nothing and must not fail the build. +// Publishing is what makes a declaration's type a promise. +TEST_F(ImplHeaderParserTest, UnreferencedHelperStructDoesNotFailTheBuild) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "struct PendingAction {\n" + " uint32_t attempts;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " int64_t f(int64_t n);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + EXPECT_TRUE(r.module.types.empty()); + + // ...and the same struct DOES fail once a method publishes it. + QTemporaryDir dir2; + ASSERT_TRUE(dir2.isValid()); + const QString hp2 = probeHeader(dir2, + "struct PendingAction {\n" + " uint32_t attempts;\n" + "};\n" + "class ProbeImpl {\n" + "public:\n" + " PendingAction f(int64_t n);\n" + "};\n"); + QString e2; + QTextStream es2(&e2); + auto r2 = parseImplHeader(hp2, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", es2); + ASSERT_TRUE(r2.hasError()); + EXPECT_TRUE(r2.error.contains("type 'PendingAction': field 'attempts'")) + << r2.error.toStdString(); +} + +// The reserved LogosModuleContext hooks are framework plumbing, not contract. +// They are dropped after parsing, so their spellings are not a contract defect. +TEST_F(ImplHeaderParserTest, ReservedHookSpellingsAreNotContractDefects) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = probeHeader(dir, + "class ProbeImpl {\n" + "public:\n" + " void onContextReady(uint32_t generation);\n" + " int64_t f(int64_t n);\n" + "};\n"); + auto r = parseImplHeader(hp, "ProbeImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + ASSERT_EQ(r.module.methods.size(), 1u); + EXPECT_EQ(r.module.methods[0].name, "f"); +} diff --git a/tests/experimental/test_lidl_gen_cdylib.cpp b/tests/experimental/test_lidl_gen_cdylib.cpp index 35f36dd..db6c8ce 100644 --- a/tests/experimental/test_lidl_gen_cdylib.cpp +++ b/tests/experimental/test_lidl_gen_cdylib.cpp @@ -82,10 +82,12 @@ TEST(LidlGenCdylib, BinaryEventPayloadUsesCanonicalBytesEncoding) const QString source = eventsSourceFor(m); - // The real argument is serialized, through the canonical encoder... - EXPECT_TRUE(source.contains("args.push_back(lidlBytesToJson(payload));")); - EXPECT_TRUE(source.contains("std::string lidlB64UrlEncode")); - EXPECT_TRUE(source.contains("nlohmann::json lidlBytesToJson")); + // The real argument is serialized, through THE canonical encoder — the one + // in logos-protocol's logos_codec.h, reached via "_types.h". The + // sidecar used to emit its own base64 encoder beside this call. + EXPECT_TRUE(source.contains("args.push_back(logos::bytesToJson(payload));")); + EXPECT_FALSE(source.contains("lidlB64UrlEncode")); + EXPECT_FALSE(source.contains("lidlBytesToJson")); // ...and the empty tagged value is gone. EXPECT_FALSE(source.contains("nlohmann::json{{\"_bytes\", \"\"}}")); @@ -98,22 +100,33 @@ TEST(LidlGenCdylib, BinaryEventPayloadUsesCanonicalBytesEncoding) EXPECT_TRUE(source.contains("const std::vector& payload")); } -// The encoder is only needed by modules that actually emit binary payloads. -// Emitted unconditionally it is an unused static function in every other -// module's sidecar (-Wunused-function). -TEST(LidlGenCdylib, BytesEncoderOmittedWhenNoEventCarriesBytes) +// No module carries a local base64 codec any more — not the ones with binary +// events and not the ones without. The gate that used to decide which got one +// is gone with it. +TEST(LidlGenCdylib, NoModuleEmitsItsOwnBase64Codec) { - const ModuleDecl m = moduleWithEvent("fault", { + const ModuleDecl bytes = moduleWithEvent("messageReceived", { + param("payload", prim("bstr")), + }); + const ModuleDecl plain = moduleWithEvent("fault", { param("code", prim("int")), param("message", prim("tstr")), param("fatal", prim("bool")), }); - const QString source = eventsSourceFor(m); - - EXPECT_FALSE(source.contains("lidlB64UrlEncode")); - EXPECT_FALSE(source.contains("lidlBytesToJson")); - EXPECT_TRUE(source.contains("args.push_back(code);")); + for (const QString& source : {eventsSourceFor(bytes), eventsSourceFor(plain), + implSourceFor(bytes), implSourceFor(plain)}) { + EXPECT_FALSE(source.contains("lidlB64UrlEncode")) << source.toStdString(); + EXPECT_FALSE(source.contains("lidlB64Idx")) << source.toStdString(); + EXPECT_FALSE(source.contains("lidlBytesToJson")) << source.toStdString(); + // The decoder had no call site at all after #117 — emitted into every + // module and never once called. + EXPECT_FALSE(source.contains("lidlBytesFromJson")) << source.toStdString(); + EXPECT_FALSE(source.contains( + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_")) + << source.toStdString(); + } + EXPECT_TRUE(eventsSourceFor(plain).contains("args.push_back(code);")); } // The sidecar is compiled into the module's Qt-free cdylib, so a JSON payload @@ -350,6 +363,16 @@ TypeExpr opt(const TypeExpr& inner) return {TypeExpr::Optional, "", {inner}}; } +TypeExpr arr(const TypeExpr& elem) +{ + return {TypeExpr::Array, "", {elem}}; +} + +TypeExpr map(const TypeExpr& key, const TypeExpr& value) +{ + return {TypeExpr::Map, "", {key, value}}; +} + FieldDecl field(const char* name, const TypeExpr& type) { FieldDecl f; @@ -473,7 +496,10 @@ TEST(LidlGenCdylib, OptionalArgumentMayBeAbsentOrNull) param("maybe", opt(prim("tstr")))})); const QString src = lidlMakeModuleImplExports(m, "OImpl", "o_impl.h"); - EXPECT_TRUE(src.contains("if (args.size() < 1) return nullptr;")) << src.toStdString(); + // The gate counts REQUIRED parameters — the same rule the Rust generator + // applies, so the two report the same `expected` for the same contract. + EXPECT_TRUE(src.contains("if (args.size() < 1) {")) << src.toStdString(); + EXPECT_TRUE(src.contains("\"expected 1 arguments, got \"")) << src.toStdString(); EXPECT_TRUE(src.contains("(args.size() > 1 ? args.at(1) : nlohmann::json())")) << src.toStdString(); EXPECT_TRUE(src.contains("logos::fromJson>")) @@ -483,8 +509,12 @@ TEST(LidlGenCdylib, OptionalArgumentMayBeAbsentOrNull) << src.toStdString(); } -// ...and a method with no optional parameter emits the gate it always did. -TEST(LidlGenCdylib, RequiredOnlyArityGateIsUnchanged) +// A wrong argument COUNT is reported, in the shape logos-rust-sdk's +// args::invalid_args() emits — same three keys, same message text, same origin. +// It used to `return nullptr`, which the Qt glue turns into an empty QVariant: +// "you passed 1 of 2 arguments" was indistinguishable from a successful empty +// answer. +TEST(LidlGenCdylib, WrongArgumentCountReportsInvalidArgs) { ModuleDecl m; m.name = "o_module"; @@ -492,10 +522,32 @@ TEST(LidlGenCdylib, RequiredOnlyArityGateIsUnchanged) {param("a", prim("tstr")), param("b", prim("tstr"))})); const QString src = lidlMakeModuleImplExports(m, "OImpl", "o_impl.h"); - EXPECT_TRUE(src.contains("if (args.size() < 2) return nullptr;")) << src.toStdString(); + EXPECT_TRUE(src.contains("if (args.size() < 2) {")) << src.toStdString(); + EXPECT_TRUE(src.contains("{\"code\", \"invalid_args\"}")) << src.toStdString(); + EXPECT_TRUE(src.contains( + "{\"message\", \"expected 2 arguments, got \" + std::to_string(args.size())}")) + << src.toStdString(); + EXPECT_TRUE(src.contains("{\"origin\", \"o_module\"}")) << src.toStdString(); + EXPECT_TRUE(src.contains("return lidlStrdup(err.dump());")) << src.toStdString(); + // The silent reply is gone from the arity path. + EXPECT_FALSE(src.contains("if (args.size() < 2) return nullptr;")) << src.toStdString(); EXPECT_FALSE(src.contains("args.size() > ")) << src.toStdString(); } +// `args.size()` is unsigned, so `< 0` never fires: a zero-argument method +// carried a dead branch. The Rust generator has always skipped it; now both do. +TEST(LidlGenCdylib, ZeroArgumentMethodEmitsNoArityGate) +{ + ModuleDecl m; + m.name = "o_module"; + m.methods.push_back(method("ping", prim("tstr"), {})); + + const QString src = lidlMakeModuleImplExports(m, "OImpl", "o_impl.h"); + EXPECT_FALSE(src.contains("args.size() < 0")) << src.toStdString(); + EXPECT_FALSE(src.contains("invalid_args")) << src.toStdString(); + EXPECT_TRUE(src.contains("lidlImpl().ping()")) << src.toStdString(); +} + // R4. Optional widens the accepted domain by exactly ONE inhabitant (empty); a // present value is still decoded as T. For `bstr` that has to be the LENIENT // decode a bare `bstr` argument gets, or the identical value would be accepted @@ -554,3 +606,43 @@ TEST(LidlGenCdylib, OptionalEventParamIsConstRefAndNullWhenEmpty) << src.toStdString(); EXPECT_TRUE(src.contains("#include ")) << src.toStdString(); } + +// `{tstr: T}` is the one LIDL type with two C++ spellings (std::map and +// std::unordered_map), and logos_codec.h specializes Codec for both. Naming one +// of them in the generated dispatch made the other a compile error in code the +// author never wrote, so the map slots hand the compiler a proxy / deduce +// instead and let the author's declaration pick. +TEST(LidlGenCdylib, TypedMapBindsTheAuthorsOwnContainer) +{ + ModuleDecl m; + m.name = "o_module"; + m.methods.push_back(method("echoIntMap", map(prim("tstr"), prim("int")), + {param("v", map(prim("tstr"), prim("int")))})); + + const QString src = lidlMakeModuleImplExports(m, "OImpl", "o_impl.h"); + EXPECT_TRUE(src.contains("logos::JsonArg(args.at(0), \"arg0\")")) << src.toStdString(); + EXPECT_TRUE(src.contains("logos::toJson(result)")) << src.toStdString(); + EXPECT_FALSE(src.contains("logos::fromJson>")) + << src.toStdString(); + EXPECT_FALSE(src.contains("logos::toJson>")) + << src.toStdString(); +} + +// ...and only maps. Every other type has one C++ spelling here, and JsonArg +// documents one target it cannot serve — std::optional, whose converting +// constructor out-ranks the proxy's conversion operator, so an empty optional +// would decode as a wrong-typed X and throw. +TEST(LidlGenCdylib, NonMapSlotsStillNameTheirType) +{ + ModuleDecl m; + m.name = "o_module"; + m.methods.push_back(method("f", prim("bool"), + {param("a", arr(prim("int"))), + param("b", opt(map(prim("tstr"), prim("tstr"))))})); + + const QString src = lidlMakeModuleImplExports(m, "OImpl", "o_impl.h"); + EXPECT_TRUE(src.contains("logos::fromJson>")) << src.toStdString(); + EXPECT_TRUE(src.contains("logos::fromJson>>")) + << src.toStdString(); + EXPECT_FALSE(src.contains("logos::JsonArg")) << src.toStdString(); +}