From 16144d4efaa86ee2d5bfaeb6a158cb7290065ff8 Mon Sep 17 00:00:00 2001 From: Dario Gabriel Lipicar Date: Fri, 31 Jul 2026 10:16:30 -0300 Subject: [PATCH] feat(parser): an unknown C++ spelling is a build error, not a silent `any` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cppTypeToLidl ended with `// Fallback: treat as opaque` -> `any`, and `any` is ADMITTED by every backend gate. So a spelling nobody had written a branch for was accepted in silence, published as `any`, and dispatched as a bare `lidlImpl().f(args.at(0))` — no logos::fromJson<>, no check. That is the one hole #113-#122 closed for every typed slot and left open for anything that reached `any`. An 11-method hostile probe was admitted wholesale: uint32_t, size_t, float and uint8_t all typed as `any`; std::set, std::vector> and a non-string-keyed map as `any`; std::vector as `[any]`. Now every spelling with no LIDL type is collected with the declaration that carries it, and parseImplHeader turns the list into a parse error naming the offending type and the fix: method 'send_generic_public_transaction': parameter 'instruction' declared `const std::vector&`, whose element `uint32_t` has no LIDL type. LIDL numbers are 64-bit only. Declare it `uint64_t` (LIDL `uint`). Widening is source-compatible for every caller; a narrow type on the wire is not, which is why LIDL has none. Numbers get that tailored hint (uint8_t its own — it means bytes here, and only as std::vector); sets, pairs/tuples, non-string map keys, list/deque/array, Qt types and pointers each get theirs; anything else gets the full table of recognised spellings. A hint that does not name a replacement just moves the guesswork, so all of them do. std::unordered_map joins std::map as a spelling of `{tstr: T}` — the codec has always handled both, and the previous commit made the generated dispatch bind whichever the author declared. Two slots are the exception and say so: a record FIELD and an event PARAMETER, where the generator writes the spelling out into code the author's own declaration has to match and can only pick one name. Three properties keep this from breaking things it should not: * The MAPPING is unchanged. cppTypeToLidl still returns `any` for an unsupported spelling; only a diagnostic is recorded. A diagnostic that is later withdrawn therefore leaves output byte-identical. * Diagnostics are withdrawn for declarations that never reach the contract — a helper struct dropped by keepOnlyReferencedRecords, a reserved lifecycle hook (onContextReady and friends), a struct with no parsed fields. Publishing is what makes a type a promise. * An empty spelling is not a C++ type, it is this line-based parser failing to find one. It keeps the old behaviour rather than reporting `''`. Also fixes a latent ordering bug found while threading the context through: metadata.json's event parameters were typed BEFORE the header was read, i.e. against whatever g_recordNames the previous module's parse had left behind. They are now read there and typed after scanForRecords, in the same position in module.events as before. Verified by generating over every impl header and every .lidl in the workspace: 31 derived contracts byte-identical, 368 consumer-umbrella files byte-identical under both --api-style qt and --api-style lp, 46 generated types headers byte-identical. Exactly two modules now fail, at exactly the four slots a prior scan identified as silent admissions. Co-Authored-By: Claude Opus 5 --- .../experimental/impl_header_parser.cpp | 362 ++++++++++++++++-- .../experimental/test_impl_header_parser.cpp | 187 +++++++++ 2 files changed, 507 insertions(+), 42 deletions(-) diff --git a/cpp-generator/experimental/impl_header_parser.cpp b/cpp-generator/experimental/impl_header_parser.cpp index c4de127..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) @@ -134,17 +272,15 @@ static TypeExpr cppTypeToLidl(const QString& raw) // 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 reaches `any` today ONLY through the fallback - // at the bottom of this function, so naming it here is a PREREQUISITE for + // 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 - // produces for it, so nothing about any published contract moves — this - // commit is deliberately output-neutral, and the next parser commit relies - // on that to tell a legitimate `any` from a silent admission. + // produced for it, so nothing about the published contract moves. if (t == "nlohmann::json" || t == "json") return { TypeExpr::Primitive, "any", {} }; @@ -153,12 +289,40 @@ static TypeExpr cppTypeToLidl(const QString& raw) 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 } }; } @@ -174,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 @@ -200,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", {} }; } @@ -236,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; @@ -252,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; } @@ -313,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; @@ -364,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"); @@ -402,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); } } @@ -430,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 --- { @@ -457,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 --- @@ -546,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 @@ -705,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; @@ -732,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. @@ -748,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(); } } } @@ -762,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/tests/experimental/test_impl_header_parser.cpp b/tests/experimental/test_impl_header_parser.cpp index 1fc3bb2..63857f2 100644 --- a/tests/experimental/test_impl_header_parser.cpp +++ b/tests/experimental/test_impl_header_parser.cpp @@ -743,3 +743,190 @@ TEST_F(ImplHeaderParserTest, NlohmannJsonIsAnyByName) 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"); +}