diff --git a/README.md b/README.md index e3a7e6e..ba1300d 100644 --- a/README.md +++ b/README.md @@ -227,6 +227,22 @@ so the two run the same single implementation. **Plugin path (`logos-cpp-generator /path/to/plugin.dylib`), with or without `--module-only`:** - `_api.h` and `_api.cpp` — the wrapper for that one plugin, and nothing else +- **`--events-from .lidl>`** names the module's CONTRACT (the + sidecar `buildPlugin.nix` installs at `$out/share/logos/.lidl`). The + flag keeps its historical name, but the wrapper's typed methods, record + structs and typed `on(callback)` accessors all come out of that + one file. The plugin is still loaded — that is the dlopen check — but its + published `getMethods()` is a human-facing DESCRIPTION, not a type source: + this emitter is keyed on flat type names with a `QVariant` fallback, so a + metadata vocabulary it does not recognise silently produced an untyped + wrapper. A named-but-missing sidecar is refused rather than fallen back from +- Without `--events-from`, the wrapper comes from the plugin's `QMetaObject`. + That is the handcrafted-Qt-module path, where no contract exists — but if the + plugin's listing is spelled in the **LIDL** vocabulary (`tstr`, `[uint]`, + `? tstr`, a record's declared name), the generator **refuses** (exit 7) and + names the contract to pass, rather than emitting a wrapper of `QVariant` / + `LogosMap`. Nix builds pass the flag for you; a hand-run invocation has to + say it **With `--umbrella` / `--general-only`:** - `logos_sdk.h` and `logos_sdk.cpp` — the umbrella that aggregates the wrappers diff --git a/cpp-generator/docs/project.md b/cpp-generator/docs/project.md index 3325b2c..caf04bb 100644 --- a/cpp-generator/docs/project.md +++ b/cpp-generator/docs/project.md @@ -169,7 +169,7 @@ Read the array through `dependencyNames()` (`metadata_dependencies.h`) rather th - `enum class ApiStyle { Qt, Lp }` — passed to every wrapper-emitting function. - File-local `mapParamTypeStd` / `mapReturnTypeStd` — the std-side type-mapping table the `lp` surface exposes. Hidden from `generator_lib.h` (not part of the public surface). -- `makeHeader(moduleName, className, methods, apiStyle, events)` / `makeSource(moduleName, className, headerBaseName, methods, apiStyle, events)` — single entry points that branch on `apiStyle` internally to emit the right include block, signature shape, and conversion bridges. `events` is loaded from a `.lidl` sidecar via `--events-from`; when non-empty, the wrapper also gets one typed `on(callback)` adapter per declared event (callback arg types follow `apiStyle`). +- `makeHeader(moduleName, className, methods, apiStyle, events)` / `makeSource(moduleName, className, headerBaseName, methods, apiStyle, events)` — single entry points that branch on `apiStyle` internally to emit the right include block, signature shape, and conversion bridges. `methods`, `events` and `records` all come from the same `.lidl` contract when the module ships one (loaded via `--events-from`); only a module with no contract is described by its plugin's `QMetaObject`. A non-empty `events` also gives the wrapper one typed `on(callback)` adapter per declared event (callback arg types follow `apiStyle`). - `makeUmbrellaHeaderFromDeps(deps, interfaceNames, apiStyle, originName, binding)` / `makeUmbrellaSourceFromDeps(deps, interfaceNames)` — the `logos_sdk.{h,cpp}` aggregate above. `binding` is the `UmbrellaBinding` from `--binding api|origin`: `FromApi` emits the `LogosModules(LogosAPI*)` constructor, `ExplicitOrigin` emits a default-constructible umbrella that names `originName` as the call origin and mentions no `LogosAPI` at all. They return the text; `main.cpp`'s `runUmbrellaMode` writes it. That split is what lets the aggregate be asserted on directly, without a filesystem. Flag plumbing: @@ -255,9 +255,9 @@ module build. `--general-only` is an exact alias for `--umbrella` (it is what `LogosModule.cmake`, `buildPlugin.nix` and `buildHeaders.nix` pass today), and both route to the one implementation in `main.cpp`. -### Consumer wrapper with typed event accessors +### Consumer wrapper from the module's contract -The `--events-from ` flag points the `.dylib` plugin-introspection codegen at a LIDL sidecar shipped alongside the dep's pre-built headers. When set, the generated `_api.{h,cpp}` gains one typed `on(callback)` accessor per declared event (callback arg types match `--api-style`): +The `--events-from ` flag points the `.dylib` plugin-introspection codegen at the LIDL sidecar shipped alongside the dep's pre-built headers. The flag keeps its historical name, but the file it names is the module's whole **contract**, and everything the wrapper is generated from comes out of it: the typed methods, the typed `on(callback)` accessors, and the record structs. Callback and signature types match `--api-style`. ```bash logos-cpp-generator /path/to/plugin.dylib \ @@ -266,6 +266,16 @@ logos-cpp-generator /path/to/plugin.dylib \ --output-dir ./generated ``` +**Contract-first, exactly like the Qt surface.** A module that ships a contract is described by it; only a module that ships none (a handcrafted Qt plugin) is described by its compiled plugin's `QMetaObject`. Both paths end in the same `makeHeader` / `makeSource`, and with a contract this path emits the same wrapper as `--general-only --dep =.lidl` — the path `buildHeaders.nix` already takes under cross-compilation. + +The methods used to come from the plugin's published `getMethods()`, and that was a defect rather than a simplification. `generator_lib` is keyed on flat type NAMES with a QVariant fallback (`mapParamType` / `mapReturnType`), so a module whose metadata is spelled in a vocabulary this emitter does not recognise silently produced a wrapper of `QVariant` / `LogosMap` with no diagnostic anywhere. It was measured: when the cdylib backend began publishing the LIDL contract vocabulary (`tstr`, `[uint]`, `? tstr`) instead of Qt type names, every `interface: "universal"` module's lp wrapper collapsed to `LogosMap`. Teaching the reader a second vocabulary is not a fix — `int` is a 32-bit Qt int in one table and a 64-bit LIDL integer in the other, so a merged table mistypes every integer and the reader cannot tell from the string which one it is holding. + +Two consequences worth knowing: + +- **A named-but-missing sidecar is refused** (exit 2), as is an unreadable or malformed one (exit 4). Falling back to introspection would emit a wrapper that compiles and is wrong in a way nothing downstream can see. +- **A LIDL-spelled listing with no contract is refused** (exit 7). Only a hand-run invocation can reach that combination — `buildHeaders.nix` always passes the flag when the sidecar exists — and it is the shape this section used to suggest. The check keys on the LIDL primitives Qt has no word for (`tstr`, `bstr`, `uint`, `float64`, `result`, `any`) plus anything starting `[`, `{` or `?`, so it cannot false-fire on a Qt name; the words the two vocabularies share (`int`, `bool`) are in the known table and never reach the fallback. +- **The plugin is still loaded**, so the dlopen check (exit 3 on an SDK/ABI skew) is unchanged, and the two method NAME sets are compared. A divergence — a stale sidecar — is reported on stderr as a `Note:`; the wrapper follows the contract. Only `isInvokable` entries are compared, because a cdylib publishes its events into the same array. + In Nix builds this is wired automatically: `buildHeaders.nix` looks for `/share/logos/.lidl` (which `buildPlugin.nix`'s installPhase placed there) and threads it through. ## Building diff --git a/cpp-generator/docs/spec.md b/cpp-generator/docs/spec.md index 465824c..8e3f8ce 100644 --- a/cpp-generator/docs/spec.md +++ b/cpp-generator/docs/spec.md @@ -288,7 +288,7 @@ logos_events: // expands to `public:`; recog } ``` - `buildPlugin.nix` ships this at `$out/share/logos/.lidl`. `buildHeaders.nix` passes it to the consumer-side codegen via `--events-from`, which adds typed `on(callback)` accessors to the generated `` wrapper (one per declared event, callback-arg types respect `--api-style`). + `buildPlugin.nix` ships this at `$out/share/logos/.lidl`. `buildHeaders.nix` passes it to the consumer-side codegen via `--events-from`, and it is the whole CONTRACT, not only the events: the generated `` wrapper takes its typed methods, its record structs and its typed `on(callback)` accessors from this one file (callback-arg and signature types respect `--api-style`). Only a module that ships no contract is described instead by its compiled plugin's `QMetaObject`. Module metadata (name, version, description, dependencies) still comes from `metadata.json`, not from the header. diff --git a/cpp-generator/experimental/lidl_emit_common.cpp b/cpp-generator/experimental/lidl_emit_common.cpp index 3cc2385..1969b62 100644 --- a/cpp-generator/experimental/lidl_emit_common.cpp +++ b/cpp-generator/experimental/lidl_emit_common.cpp @@ -13,7 +13,84 @@ QString lidlToPascalCase(const QString& name) return out; } +// A type "bottoms out at `any`" when its scalar LEAF is `any` — or an +// unrecognised primitive, which this table has always spelled QVariant too. +// Optionality and container nesting are transparent to the question: +// `[[any]]`, `{tstr: [any]}` and `?any` all bottom out at `any`. +bool lidlQtBottomsOutAtAny(const TypeExpr& te) +{ + switch (te.kind) { + case TypeExpr::Primitive: + return !(te.name == "void" || te.name == "tstr" || te.name == "bstr" + || te.name == "int" || te.name == "uint" || te.name == "float64" + || te.name == "bool" || te.name == "result"); + case TypeExpr::Named: + // A record declared by the contract: a real struct, never a blob. + return false; + case TypeExpr::Array: + // A degenerate Array carrying no element (unreachable from the parser, + // constructible by hand or over the JSON bridge) keeps the opaque + // spelling rather than being described as typed. + return te.elements.size() != 1 || lidlQtBottomsOutAtAny(te.elements[0]); + case TypeExpr::Map: + return te.elements.size() != 2 || lidlQtBottomsOutAtAny(te.elements[1]); + case TypeExpr::Optional: + // Through optionalValueType(), so `??T` answers for T — optionality is + // idempotent under the two-state rule. + return te.elements.empty() || lidlQtBottomsOutAtAny(optionalValueType(te)); + } + return true; +} + +bool lidlQtNeedsElementLoop(const TypeExpr& te) +{ + if (lidlQtBottomsOutAtAny(te)) return false; // QVariant / List / Map + switch (te.kind) { + case TypeExpr::Array: + // `[tstr]` is QStringList, which crosses whole (QMetaType::QStringList + // is in qvariantToNlohmann's closed set). Every other typed array is + // QList, which is not. + return !(te.elements[0].kind == TypeExpr::Primitive + && te.elements[0].name == "tstr"); + case TypeExpr::Map: + case TypeExpr::Optional: + return true; + case TypeExpr::Primitive: + case TypeExpr::Named: + return false; + } + return false; +} + +// The LIDL contract spelling. Mirrors logos-lidl's serializeTypeExpr; see the +// header for why it is a copy and what pins it. +QString lidlTypeToLidlText(const TypeExpr& te) +{ + switch (te.kind) { + case TypeExpr::Primitive: + case TypeExpr::Named: + return QString::fromStdString(te.name); + case TypeExpr::Array: + if (te.elements.size() != 1) return QStringLiteral("any"); + return "[" + lidlTypeToLidlText(te.elements[0]) + "]"; + case TypeExpr::Map: + if (te.elements.size() != 2) return QStringLiteral("any"); + return "{" + lidlTypeToLidlText(te.elements[0]) + ": " + + lidlTypeToLidlText(te.elements[1]) + "}"; + case TypeExpr::Optional: + if (te.elements.empty()) return QStringLiteral("any"); + return "? " + lidlTypeToLidlText(te.elements[0]); + } + return QStringLiteral("any"); +} + QString lidlTypeToQt(const TypeExpr& te) +{ + return lidlTypeToQt(te, [](const QString& n) { return n; }); +} + +QString lidlTypeToQt(const TypeExpr& te, + const std::function& recordName) { switch (te.kind) { case TypeExpr::Primitive: @@ -31,46 +108,61 @@ QString lidlTypeToQt(const TypeExpr& te) if (te.name == "float64") return "double"; if (te.name == "bool") return "bool"; if (te.name == "result") return "LogosResult"; + // `any` — KEPT untyped, and it is the only row here that is. QVariant is + // the sole Qt type that carries bytes AND an exact uint64 AND arbitrary + // nesting, so narrowing it would lose what it was chosen to hold. if (te.name == "any") return "QVariant"; return "QVariant"; case TypeExpr::Named: // A record declared by the contract: its generated struct. One LIDL // type, one type per language — a record is not a QVariant blob. - return QString::fromStdString(te.name); + return recordName(QString::fromStdString(te.name)); case TypeExpr::Array: - if (te.elements.size() == 1 - && te.elements[0].kind == TypeExpr::Primitive + // `[any]` (and anything else whose leaf is `any`) keeps QVariantList: + // there is no narrower Qt list that can hold those elements. + if (lidlQtBottomsOutAtAny(te)) return "QVariantList"; + // `[tstr]` is QStringList — the one typed array Qt has a native + // spelling for, and the one this table already produced. + if (te.elements[0].kind == TypeExpr::Primitive && te.elements[0].name == "tstr") { return "QStringList"; } - // A list of records is a typed list: QVariantList could not hold a - // record without Q_DECLARE_METATYPE, and the point of a record is that - // the consumer gets the struct. - if (te.elements.size() == 1 && te.elements[0].kind == TypeExpr::Named) - return "QList<" + QString::fromStdString(te.elements[0].name) + ">"; - return "QVariantList"; + // Every other `[T]` — including a list of records, which could not ride + // a QVariantList without Q_DECLARE_METATYPE — is the typed list. The + // element spelling is this same table applied recursively, so + // `[[uint]]` is QList> and `[?tstr]` is + // QList>. + return "QList<" + lidlTypeToQt(te.elements[0], recordName) + ">"; case TypeExpr::Map: - if (te.elements.size() == 2 && te.elements[1].kind == TypeExpr::Named) - return "QMap"; - return "QVariantMap"; + if (lidlQtBottomsOutAtAny(te)) return "QVariantMap"; + // The key is spelled QString unconditionally, as it always has been: a + // JSON object key IS a string, so a contract that writes a non-tstr key + // does not change what crosses the wire. + return "QMap"; case TypeExpr::Optional: - // `?T` on the QT surface, deliberately, and this is the one mapping in - // this table that LOSES the value type. + // `?T` -> std::optional. This row used to be a bare QVariant and was + // the ONE mapping in this table that lost the value type: a Qt consumer + // could not tell `?tstr` from `?uint`, while the std surface next door + // kept both through std::optional. // - // Qt has no optional template, and the type this name is read for is a - // metatype: the legacy consumer path and the cdylib's getMethods() - // introspection both hand it to the host, which marshals a QVariant - // across the plugin boundary. There is no metatype called - // `std::optional` — emitting one would fail exactly the way - // emitting a record's struct name here once made the host SIGSEGV. + // The objection that kept it QVariant was that the name is read as a + // METATYPE — the legacy consumer path and getMethods() introspection + // both handed it to the host to marshal, and there is no metatype called + // `std::optional`. Both halves of that are now false: + // getMethods() publishes the LIDL spelling (lidlTypeToQtWire), and the + // string-keyed legacy emitter folds every widened spelling back to the + // name it used before (legacyQtBase in generator_lib.cpp). What is left + // reading this row is the TypeExpr-driven Qt emitters, which emit + // element loops rather than a metatype lookup. // - // QVariant is at least the RIGHT SHAPE: an invalid QVariant is Qt's - // single empty inhabitant, and the wire's `null` becomes precisely - // that. So `?T` is two-state here — it is just untyped, in the same way - // `any` is, which means a Qt consumer gets no compile-time check on the - // value and cannot tell `?tstr` from `?uint`. That is a real gap, not a - // finished mapping; see cpp-generator/docs/project.md ("Optionality"). - return "QVariant"; + // Recursed through optionalValueType() rather than elements[0], because + // optionality is idempotent under the two-state rule: `??T` denotes the + // same two states as `?T` and must not become + // std::optional>. A degenerate Optional carrying no + // element keeps the opaque fallback instead of recursing forever + // (lidlQtBottomsOutAtAny answers true for it). + if (lidlQtBottomsOutAtAny(te)) return "QVariant"; + return "std::optional<" + lidlTypeToQt(optionalValueType(te), recordName) + ">"; } return "QVariant"; } diff --git a/cpp-generator/experimental/lidl_emit_common.h b/cpp-generator/experimental/lidl_emit_common.h index cfa3aaa..dc04028 100644 --- a/cpp-generator/experimental/lidl_emit_common.h +++ b/cpp-generator/experimental/lidl_emit_common.h @@ -5,9 +5,67 @@ #pragma once #include +#include + #include "lidl_compat.h" QString lidlToPascalCase(const QString& name); QString lidlTypeToQt(const TypeExpr& te); + +// The same table, with a hook on how a `Named` (record) type is spelled. +// +// A generated wrapper nests its record structs in the wrapper class +// (`InfoModule::Point`, so two dependencies may both declare a `Point`), so a +// type written OUTSIDE that class scope — a return type before the `Class::` of +// a definition — has to qualify them. Before the table produced composites, the +// only shapes that could mention a record were `Point`, `QList` and +// `QMap`, and each emitter matched those three by hand on the +// finished string. That stops working the moment `?Point`, +// `QList>` or `QMap` are spellable. +// +// So the qualification happens DURING the walk, at the one place that knows a +// name is a record, and there is still one table: lidlTypeToQt(te) is exactly +// this with an identity hook. +QString lidlTypeToQt(const TypeExpr& te, + const std::function& recordName); QString lidlTypeToStd(const TypeExpr& te); bool lidlIsStdConvertible(const TypeExpr& te); + +// The LIDL CONTRACT spelling of a type — `tstr`, `[Point]`, `{tstr: uint}`, +// `? tstr`. Mirrors logos-lidl's `serializeTypeExpr` (src/serializer.cpp) byte +// for byte, so a type name published in getMethods() is the same text the +// module's own `.lidl` artifact carries for that slot. +// +// It is a COPY, and that is a defect this cannot fix from here: +// `serializeTypeExpr` is file-local to logos-lidl's serializer.cpp and the +// public headers expose no type-expression printer, so there is nothing to +// delegate to. The pairing is asserted instead — tests/experimental +// round-trips each shape through `lidl::serialize` and compares — so the two +// cannot drift silently. When logos-lidl exports a `typeToString`, delete this +// and call it. +QString lidlTypeToLidlText(const TypeExpr& te); + +// True when the Qt spelling of `te` is one of the QVariant-family names — +// QVariant / QVariantList / QVariantMap — i.e. when the type's scalar leaf is +// `any` (or an unrecognised primitive). Those slots keep the untyped spelling +// because QVariant is the only Qt type that holds bytes AND an exact uint64 AND +// arbitrary nesting all at once, so widening them would LOSE information. +bool lidlQtBottomsOutAtAny(const TypeExpr& te); + +// True when the Qt spelling of `te` is a TYPED container or a std::optional — +// `QList`, `QMap`, `std::optional` — and therefore must be +// encoded and decoded by a generator-emitted ELEMENT LOOP. +// +// THIS IS THE ONE PREDICATE THAT KEEPS THE WIDENED TABLE FROM DESTROYING DATA. +// logos-protocol's qvariantToNlohmann matches a CLOSED userType() set +// (QByteArray, LogosResult, the integer types, QStringList, QVariantList, +// QVariantMap, the QJson types); `QList` matches NONE of them and +// serialises to JSON null. The other direction fails just as quietly: +// `qvariant_cast>` of a QVariantList yields an EMPTY list. +// Neither direction warns. So a name this returns true for must never be +// handed to logos::qt::toWire / fromWire (or to QVariant::fromValue / +// qvariant_cast) as a whole value — only its ELEMENTS may cross, one at a time. +// +// QStringList is deliberately NOT one of these: it is in the closed set, so it +// crosses whole and always did. +bool lidlQtNeedsElementLoop(const TypeExpr& te); diff --git a/cpp-generator/experimental/lidl_gen_cdylib.cpp b/cpp-generator/experimental/lidl_gen_cdylib.cpp index 4e86be2..d75d039 100644 --- a/cpp-generator/experimental/lidl_gen_cdylib.cpp +++ b/cpp-generator/experimental/lidl_gen_cdylib.cpp @@ -409,29 +409,37 @@ void emitRecordCodecs(QTextStream& s, const ModuleDecl& module, s << "}} // namespace logos::detail\n\n"; } -// The Qt spelling of what actually crosses the Qt boundary. +// The type name a method's PUBLISHED metadata carries — getMethods()'s +// `returnType`, `parameters[].type` and `signature`. // -// NOT lidlTypeToQt: that answers the CONSUMER's question ("what type does the -// caller hold?") and since records became real structs it answers `Blob` / -// `QList`. Those names are correct in a generated consumer wrapper, where -// the struct exists — but this JSON is the module's getMethods(), read by the -// host to marshal a QVariant across the plugin boundary, and there is no -// metatype called `Blob`. Emitting it made the host SIGSEGV on the first call -// to any record method. +// It is the LIDL CONTRACT spelling: `tstr`, `uint`, `[Point]`, `{tstr: uint}`, +// `? tstr`. That is the only vocabulary in which this question has one right +// answer. A module's published surface is its contract, and every consumer of +// this JSON — `lm`, `logoscore`'s method listing, basecamp's module inspector — +// is showing a human what the module offers. Answering in Qt names made a +// Qt-free cdylib module describe itself in the types of a language it does not +// use, and answered three different LIDL types (`[uint]`, `[bstr]`, `[any]`) +// with one word, QVariantList, so the listing could not be read back. // -// A record IS a variant map at that boundary; the struct only exists inside the -// cdylib. -QString lidlTypeToQtWire(const TypeExpr& te, const std::set& recs) +// NOT lidlTypeToQt, and no longer a near-copy of it. That function answers the +// CONSUMER's question — "what C++ type does the caller hold?" — and its answers +// are now typed C++ spellings (QList, std::optional) that +// are meaningless outside a generated wrapper. +// +// WHY THIS IS SAFE, checked rather than assumed. The historical objection was +// that these strings are read as METATYPES: emitting a record's struct name +// here once made the host SIGSEGV. Nothing in the current runtime does that. +// The dispatch paths key on QMetaObject types instead — logos-plugin-qt's +// QtProviderObject reads `method.returnMetaType()` / `parameterMetaType(i)` and +// never touches this JSON — and every reader of these fields that remains +// (logos-module's `lm`, logoscore's `output.cpp`, basecamp's CoreModuleManager, +// the plain wire's json_mapping round-trip) treats them as opaque text. +// +// The `recs` parameter is gone with the Qt spelling: a record publishes its +// declared NAME, which is what the contract calls it. +QString lidlTypeToPublishedName(const TypeExpr& te) { - if (isRecord(te, recs)) - return "QVariantMap"; - if (te.kind == TypeExpr::Array && te.elements.size() == 1 - && isRecord(te.elements[0], recs)) - return "QVariantList"; - if (te.kind == TypeExpr::Map && te.elements.size() == 2 - && isRecord(te.elements[1], recs)) - return "QVariantMap"; - return lidlTypeToQt(te); + return lidlTypeToLidlText(te); } // True when any event parameter is spelled LogosMap / LogosList, so the sidecar @@ -468,7 +476,6 @@ bool hasJsonEventParam(const ModuleDecl& module) void emitInterfaceJson(QTextStream& s, const ModuleDecl& module) { - const std::set recs = recordNames(module); s << "static nlohmann::json lidlInterfaceJson()\n{\n"; s << " nlohmann::json methods = nlohmann::json::array();\n"; for (const MethodDecl& md : module.methods) { @@ -481,17 +488,17 @@ void emitInterfaceJson(QTextStream& s, const ModuleDecl& module) } QString sig = qs(md.name) + "("; for (int i = 0; i < md.params.size(); ++i) { - sig += lidlTypeToQtWire(md.params[i].type, recs); + sig += lidlTypeToPublishedName(md.params[i].type); if (i + 1 < md.params.size()) sig += ","; } sig += ")"; s << " obj[\"signature\"] = \"" << sig << "\";\n"; - s << " obj[\"returnType\"] = \"" << lidlTypeToQtWire(md.returnType, recs) << "\";\n"; + s << " obj[\"returnType\"] = \"" << lidlTypeToPublishedName(md.returnType) << "\";\n"; s << " obj[\"isInvokable\"] = true;\n"; if (!md.params.empty()) { s << " nlohmann::json params = nlohmann::json::array();\n"; for (const ParamDecl& pd : md.params) { - s << " params.push_back({{\"type\", \"" << lidlTypeToQtWire(pd.type, recs) + s << " params.push_back({{\"type\", \"" << lidlTypeToPublishedName(pd.type) << "\"}, {\"name\", \"" << pd.name << "\"}});\n"; } s << " obj[\"parameters\"] = params;\n"; @@ -509,7 +516,7 @@ void emitInterfaceJson(QTextStream& s, const ModuleDecl& module) } QString sig = qs(ed.name) + "("; for (int i = 0; i < ed.params.size(); ++i) { - sig += lidlTypeToQtWire(ed.params[i].type, recs); + sig += lidlTypeToPublishedName(ed.params[i].type); if (i + 1 < ed.params.size()) sig += ","; } sig += ")"; @@ -517,7 +524,7 @@ void emitInterfaceJson(QTextStream& s, const ModuleDecl& module) if (!ed.params.empty()) { s << " nlohmann::json params = nlohmann::json::array();\n"; for (const ParamDecl& pd : ed.params) { - s << " params.push_back({{\"type\", \"" << lidlTypeToQtWire(pd.type, recs) + s << " params.push_back({{\"type\", \"" << lidlTypeToPublishedName(pd.type) << "\"}, {\"name\", \"" << pd.name << "\"}});\n"; } s << " obj[\"parameters\"] = params;\n"; diff --git a/cpp-generator/experimental/lidl_gen_client.cpp b/cpp-generator/experimental/lidl_gen_client.cpp index 76a3767..4111b9f 100644 --- a/cpp-generator/experimental/lidl_gen_client.cpp +++ b/cpp-generator/experimental/lidl_gen_client.cpp @@ -40,6 +40,76 @@ static QString qtToVariantExpr(const TypeExpr& te, const QString& expr); static QString qtFromVariantExpr(const TypeExpr& te, const QString& expr); static QString returnConversionFor(const TypeExpr& te, const QString& qt); +// A field's `?T` as a TypeExpr, whichever of the two spellings the author used +// (`? name: T` sets the flag and leaves the type T; `name: ?T` makes the type +// an Optional). Building one shape here is what makes the two emit identical +// code — the rule fieldIsOptional()/fieldValueType() exist to enforce. +static TypeExpr fieldOptionalType(const FieldDecl& f) +{ + if (f.type.kind == TypeExpr::Optional) return f.type; + TypeExpr o; + o.kind = TypeExpr::Optional; + o.elements.push_back(f.type); + return o; +} + +// Does this slot need a generator-emitted ELEMENT LOOP rather than a whole-value +// QVariant hop? +// +// Two reasons a slot can need one, and they are now the same question: +// * it mentions a RECORD — a struct with no Q_DECLARE_METATYPE, so +// QVariant::fromValue of it is a blob nothing can read back; +// * its Qt spelling is a TYPED container or a std::optional — +// QList, QMap, std::optional — +// which QVariant handles even worse than a record: qvariantToNlohmann +// matches a CLOSED userType() set and answers null for it, and +// qvariant_cast back yields an EMPTY container. Silently, both ways. +// +// QStringList, QVariantList and QVariantMap are NOT in this set: they are in +// that closed set and cross whole, exactly as they always did. +static bool holdsRecordType(const TypeExpr& te) +{ + return lidlIsRecord(te) + || (te.kind == TypeExpr::Array && te.elements.size() == 1 && lidlIsRecord(te.elements[0])) + || (te.kind == TypeExpr::Map && te.elements.size() == 2 && lidlIsRecord(te.elements[1])); +} + +static bool needsElementLoop(const TypeExpr& te) +{ + return holdsRecordType(te) || lidlQtNeedsElementLoop(te); +} + +// Does any slot in the contract materialise a std::optional on this surface? +// Gates the generated `#include `, so a contract with no optional (or +// one whose only optionals are `?any`) keeps its header byte-for-byte. +static bool typeUsesStdOptional(const TypeExpr& te) +{ + if (te.kind == TypeExpr::Optional) + return lidlQtNeedsElementLoop(te) + || (!te.elements.empty() && typeUsesStdOptional(optionalValueType(te))); + for (const TypeExpr& e : te.elements) + if (typeUsesStdOptional(e)) return true; + return false; +} + +static bool moduleUsesStdOptional(const ModuleDecl& m) +{ + for (const TypeDecl& t : m.types) + for (const FieldDecl& f : t.fields) { + // The field's EFFECTIVE type: the optional wrapper when either + // spelling makes it optional, the written type otherwise. + const TypeExpr eff = fieldIsOptional(f) ? fieldOptionalType(f) : f.type; + if (typeUsesStdOptional(eff)) return true; + } + for (const MethodDecl& md : m.methods) { + if (typeUsesStdOptional(md.returnType)) return true; + for (const ParamDecl& p : md.params) if (typeUsesStdOptional(p.type)) return true; + } + for (const EventDecl& ed : m.events) + for (const ParamDecl& p : ed.params) if (typeUsesStdOptional(p.type)) return true; + return false; +} + static QString returnConversion(const QString& qt) { if (qt == "bool") return "return _result.toBool();"; @@ -58,15 +128,14 @@ static QString returnConversion(const QString& qt) return "return _result;"; } -// Records (and containers holding them) decode through the generated -// conversions; everything else keeps the historical QVariant accessor. +// Records, containers holding them, and every TYPED container / optional decode +// through a generated element loop; everything else keeps the historical +// QVariant accessor. `[tstr]` and `[any]` stay on the accessor: QStringList and +// QVariantList are QVariant-native, so `.toStringList()` / `.toList()` is both +// correct and what shipped. static QString returnConversionFor(const TypeExpr& te, const QString& qt) { - const bool holdsRecord = - lidlIsRecord(te) - || (te.kind == TypeExpr::Array && te.elements.size() == 1 && lidlIsRecord(te.elements[0])) - || (te.kind == TypeExpr::Map && te.elements.size() == 2 && lidlIsRecord(te.elements[1])); - if (holdsRecord) + if (needsElementLoop(te)) return "return " + qtFromVariantExpr(te, "_result") + ";"; return returnConversion(qt); } @@ -82,11 +151,7 @@ static QString returnConversionFor(const TypeExpr& te, const QString& qt) // the caller reached for. static QString asyncReturnConversionFor(const TypeExpr& te, const QString& qt) { - const bool holdsRecord = - lidlIsRecord(te) - || (te.kind == TypeExpr::Array && te.elements.size() == 1 && lidlIsRecord(te.elements[0])) - || (te.kind == TypeExpr::Map && te.elements.size() == 2 && lidlIsRecord(te.elements[1])); - if (holdsRecord) + if (needsElementLoop(te)) return qtFromVariantExpr(te, "v"); return "qvariant_cast<" + qt + ">(v)"; } @@ -123,20 +188,43 @@ static bool lidlIsRecord(const TypeExpr& te) } // value expression of the Qt type -> QVariant +// +// The loop is emitted whenever the surface type is not QVariant-native — for a +// record (a struct with no metatype) and now equally for every TYPED container +// and optional. `QVariant::fromValue(QList)` is not a compile +// error and not a runtime warning; it produces a QVariant that +// qvariantToNlohmann answers `null` for, because that function matches a CLOSED +// userType() set. So the whole value must never cross — only its elements, one +// at a time, each of which IS in that set. static QString qtToVariantExpr(const TypeExpr& te, const QString& expr) { if (lidlIsRecord(te)) return qs(te.name) + "ToVariant(" + expr + ")"; - if (te.kind == TypeExpr::Array && te.elements.size() == 1 - && (lidlIsRecord(te.elements[0]) || te.elements[0].kind != TypeExpr::Primitive)) { - return "[&]{ QVariantList __l; for (const auto& __e : " + expr + ") __l.append(" - + qtToVariantExpr(te.elements[0], "__e") + "); return QVariant(__l); }()"; + // THE SOURCE IS A LAMBDA PARAMETER, never a local bound inside the body. + // These loops nest — `[[uint]]` puts one inside another — and every level + // wants the same short names, so a body-local (or a range-for over a name + // the loop itself declares) would be self-referential: it compiles, and it + // reads uninitialised memory. An ARGUMENT is evaluated in the ENCLOSING + // scope, before the inner names exist. + if (te.kind == TypeExpr::Array && te.elements.size() == 1 && needsElementLoop(te)) { + return "[&](const auto& __c){ QVariantList __l; for (const auto& __e : __c) __l.append(" + + qtToVariantExpr(te.elements[0], "__e") + "); return QVariant(__l); }(" + + expr + ")"; } - if (te.kind == TypeExpr::Map && te.elements.size() == 2 - && (lidlIsRecord(te.elements[1]) || te.elements[1].kind != TypeExpr::Primitive)) { - return "[&]{ QVariantMap __m; for (auto __it = " + expr + ".begin(); __it != " + expr - + ".end(); ++__it) __m.insert(__it.key(), " - + qtToVariantExpr(te.elements[1], "__it.value()") + "); return QVariant(__m); }()"; + if (te.kind == TypeExpr::Map && te.elements.size() == 2 && needsElementLoop(te)) { + return "[&](const auto& __c){ QVariantMap __m; for (auto __it = __c.begin(); " + "__it != __c.end(); ++__it) __m.insert(__it.key(), " + + qtToVariantExpr(te.elements[1], "__it.value()") + "); return QVariant(__m); }(" + + expr + ")"; + } + // `?T` -> std::optional: EMPTY is the invalid QVariant, which is Qt's + // single empty inhabitant and what the wire's `null` becomes. `?any` never + // reaches here (it is still spelled QVariant, so needsElementLoop is false) + // and rides the fromValue below unchanged. + if (te.kind == TypeExpr::Optional && needsElementLoop(te)) { + const TypeExpr& v = optionalValueType(te); + return "[&](const auto& __c){ return __c.has_value() ? " + qtToVariantExpr(v, "*__c") + + " : QVariant(); }(" + expr + ")"; } return "QVariant::fromValue(" + expr + ")"; } @@ -155,16 +243,29 @@ static QString qtFromVariantExpr(const TypeExpr& te, const QString& expr) if (n == "float64") return expr + ".toDouble()"; if (n == "bool") return expr + ".toBool()"; } + // Source as a lambda PARAMETER, for the reason given on the encode side. if (te.kind == TypeExpr::Array && te.elements.size() == 1) { const TypeExpr& e = te.elements[0]; - return "[&]{ " + lidlTypeToQt(te) + " __acc; for (const QVariant& __e : " + expr - + ".toList()) __acc.append(" + qtFromVariantExpr(e, "__e") + "); return __acc; }()"; + return "[&](const QVariant& __s){ " + lidlTypeToQt(te) + + " __acc; for (const QVariant& __e : __s.toList()) __acc.append(" + + qtFromVariantExpr(e, "__e") + "); return __acc; }(" + expr + ")"; } if (te.kind == TypeExpr::Map && te.elements.size() == 2) { const TypeExpr& v = te.elements[1]; - return "[&]{ " + lidlTypeToQt(te) + " __acc; const QVariantMap __mm = " + expr - + ".toMap(); for (auto __it = __mm.begin(); __it != __mm.end(); ++__it) __acc.insert(" - + "__it.key(), " + qtFromVariantExpr(v, "__it.value()") + "); return __acc; }()"; + return "[&](const QVariant& __s){ " + lidlTypeToQt(te) + + " __acc; const QVariantMap __mm = __s.toMap(); " + "for (auto __it = __mm.begin(); __it != __mm.end(); ++__it) __acc.insert(" + + "__it.key(), " + qtFromVariantExpr(v, "__it.value()") + "); return __acc; }(" + + expr + ")"; + } + // `?T`: an invalid (or null) QVariant is the empty state — absent and + // explicit-null are the SAME state, as the two-state rule requires — and + // anything else is a present T decoded by this same table. + if (te.kind == TypeExpr::Optional && needsElementLoop(te)) { + const TypeExpr& v = optionalValueType(te); + const QString opt = lidlTypeToQt(te); + return "[&](const QVariant& __s){ if (!__s.isValid() || __s.isNull()) return " + opt + + "(); return " + opt + "(" + qtFromVariantExpr(v, "__s") + "); }(" + expr + ")"; } return expr; } @@ -173,23 +274,27 @@ static QString qtFromVariantExpr(const TypeExpr& te, const QString& expr) // else goes through unchanged (packVariantList wraps with QVariant::fromValue). static QString qtArgExpr(const TypeExpr& te, const QString& name) { - const bool holdsRecord = - lidlIsRecord(te) - || (te.kind == TypeExpr::Array && te.elements.size() == 1 && lidlIsRecord(te.elements[0])) - || (te.kind == TypeExpr::Map && te.elements.size() == 2 && lidlIsRecord(te.elements[1])); - return holdsRecord ? qtToVariantExpr(te, name) : name; + return needsElementLoop(te) ? qtToVariantExpr(te, name) : name; } // A record field's Qt type, honouring BOTH optionality spellings. // -// `?T` is QVariant on the Qt surface — Qt has no optional template, and an -// invalid QVariant is its single empty inhabitant. The point of routing through -// fieldIsOptional() is that `? name: T` and `name: ?T` are the same declaration: -// reading `f.type` alone made the flag spelling emit a bare `T` (which cannot be -// empty at all) while the type spelling emitted QVariant, from one contract. +// `?T` is std::optional, the same answer every other slot gets — a Qt +// consumer's `Profile.nickname` is now a std::optional rather than a +// QVariant it has to guess the payload type of, which is what the std surface +// next door has always given (Codec>). `?any` stays QVariant: +// `any` is the one row the widened table keeps untyped, and QVariant already +// has exactly one empty inhabitant, so wrapping it would spell EMPTY twice and +// make a two-state slot three-state. +// +// Routing through fieldIsOptional()/fieldOptionalType() is what makes the two +// spellings identical: reading `f.type` alone made the flag spelling emit a +// bare `T` (which cannot be empty at all) while the type spelling emitted an +// optional, from one contract. static QString lidlFieldTypeQt(const FieldDecl& f) { - return fieldIsOptional(f) ? QString("QVariant") : lidlTypeToQt(f.type); + return fieldIsOptional(f) ? lidlTypeToQt(fieldOptionalType(f)) + : lidlTypeToQt(f.type); } static void emitRecords(QTextStream& s, const ModuleDecl& module) @@ -211,10 +316,23 @@ static void emitRecords(QTextStream& s, const ModuleDecl& module) for (const FieldDecl& f : t.fields) { if (fieldIsOptional(f)) { // A record field is a NAMED slot: empty is spelled by OMITTING - // the key, not by inserting an invalid QVariant. Same rule the + // the key, not by inserting an empty value. Same rule the // cdylib record codec follows, on the other surface. - s << " if (v." << qs(f.name) << ".isValid())\n"; - s << " __m.insert(\"" << qs(f.name) << "\", v." << qs(f.name) << ");\n"; + // + // The emptiness TEST follows the field's own spelling — + // `.has_value()` for a std::optional, `.isValid()` for the + // `?any` slot that stays a QVariant — because those are the two + // types this surface can produce for an optional field. + const QString fv = "v." + qs(f.name); + const TypeExpr ot = fieldOptionalType(f); + if (lidlQtNeedsElementLoop(ot)) { + s << " if (" << fv << ".has_value())\n"; + s << " __m.insert(\"" << qs(f.name) << "\", " + << qtToVariantExpr(fieldValueType(f), "*" + fv) << ");\n"; + } else { + s << " if (" << fv << ".isValid())\n"; + s << " __m.insert(\"" << qs(f.name) << "\", " << fv << ");\n"; + } continue; } s << " __m.insert(\"" << qs(f.name) << "\", " @@ -228,10 +346,14 @@ static void emitRecords(QTextStream& s, const ModuleDecl& module) for (const FieldDecl& f : t.fields) { if (fieldIsOptional(f)) { // Absent and null both arrive as an invalid QVariant — the same - // state, as the contract requires. Converting (`.toString()` on - // a flag-optional `tstr`) would have turned "empty" into "", - // which is a VALUE. - s << " __out." << qs(f.name) << " = __m.value(\"" << qs(f.name) << "\");\n"; + // state, as the contract requires — and the optional decode + // below turns exactly that into the empty optional. A bare + // conversion (`.toString()` on a flag-optional `tstr`) would + // have turned "empty" into "", which is a VALUE. + s << " __out." << qs(f.name) << " = " + << qtFromVariantExpr(fieldOptionalType(f), + "__m.value(\"" + qs(f.name) + "\")") + << ";\n"; continue; } s << " __out." << qs(f.name) << " = " @@ -260,6 +382,7 @@ QString lidlMakeHeader(const ModuleDecl& module, BindMode bindMode) s << "#include \n"; s << "#include \n"; s << "#include \n"; + if (moduleUsesStdOptional(module)) s << "#include \n"; s << "#include \"logos_types.h\"\n"; s << "#include \"logos_api.h\"\n"; s << "#include \"logos_api_client.h\"\n"; diff --git a/cpp-generator/generator_lib.cpp b/cpp-generator/generator_lib.cpp index 8aa146a..aea4f44 100644 --- a/cpp-generator/generator_lib.cpp +++ b/cpp-generator/generator_lib.cpp @@ -101,9 +101,47 @@ QString normalizeType(QString t) return t; } +// ─── The widened-Qt-spelling fold ──────────────────────────────────────── +// +// `lidlTypeToQt` now answers `[uint]` with QList, `{tstr: uint}` +// with QMap and `?tstr` with std::optional — the +// lossless spellings a Qt CONSUMER wants. This emitter cannot use them, and the +// reason is structural rather than a matter of taste: +// +// * It is keyed on a FLAT TYPE NAME, not a TypeExpr. `lidl_to_json` flattens +// the contract to strings before it gets here, because the same emitter +// also serves the metaobject-introspection path, which has only names to +// offer. Encoding `QList>` correctly needs an +// element loop per level, and deriving those levels here means parsing C++ +// type names back into a tree — a second, worse frontend. +// * Its Qt flavour marshals whole QVariants through LogosAPIClient, and its +// lp flavour is derived from the same table by mapParamTypeStd below. A +// widened name reaching either without a loop is silent data loss: +// qvariantToNlohmann matches a CLOSED userType() set, so a +// QList serialises to null, and qvariant_cast back yields an +// EMPTY list. Neither direction warns. +// +// So every widened spelling is folded back to the name this emitter already +// produced for that contract, and BOTH surfaces it feeds — the legacy Qt +// consumer and the Qt-free lp one — stay byte-for-byte what they were. This is +// deliberately a freeze, not a fix: the TypeExpr-driven Qt emitters +// (lidl_gen_client.cpp here, lidl_gen_qt_consumer.cpp in logos-qt-sdk) are +// where the widened types are actually spent. +// +// Record-bearing names never reach this: paramTypeFor / returnTypeFor consult +// recordCppType FIRST, and `QList` / `QMap` are matched +// there. What arrives here is only what recordShape declined. +static QString legacyQtBase(const QString& t) +{ + if (t.startsWith("QList<")) return QStringLiteral("QVariantList"); + if (t.startsWith("QMap known = { "void","bool","int","qlonglong","qulonglong","double","float","QString","QStringList","QByteArray","QJsonArray","QVariantList","QVariantMap","QVariant" }; @@ -114,7 +152,7 @@ QString mapParamType(const QString& qtType) QString mapReturnType(const QString& qtType) { - const QString base = normalizeType(qtType); + const QString base = legacyQtBase(normalizeType(qtType)); if (base.isEmpty() || base == "void") return QString("void"); static const QSet known = { "bool","int","qlonglong","qulonglong","double","float","QString","QStringList","QByteArray","QJsonArray","QVariantList","QVariantMap","QVariant","LogosResult" diff --git a/cpp-generator/lidl_to_json.cpp b/cpp-generator/lidl_to_json.cpp index 80af6fe..4ddcc4d 100644 --- a/cpp-generator/lidl_to_json.cpp +++ b/cpp-generator/lidl_to_json.cpp @@ -31,15 +31,15 @@ QString lidlTypeExprToQtTypeName(const TypeExpr& te) // Qt surface, std::optional on the Lp one). What is still flattened is every // POSITIONAL slot — a method parameter, a return type, an event parameter. // Those have no name to hang a flag on, so they only ever had the type-kind -// spelling and there is no spelling divergence to fix; what they lose is the -// value TYPE, exactly as `lidlTypeToQt` documents (`?T` -> QVariant, and via -// the derived std table -> LogosMap). Two-stateness survives — an invalid -// QVariant / a JSON null is the empty inhabitant — but the consumer gets no -// compile-time check on the value and cannot tell `?tstr` from `?uint`. +// spelling and there is no spelling divergence to fix. // -// Widening those means changing the generated method SIGNATURES, which is a -// source break for every existing caller and buys nothing for the -// one-declaration-two-spellings rule. So they stay flattened, and say so. +// `lidlTypeToQt` DOES now answer `?T` with std::optional. This path cannot +// keep it: generator_lib is keyed on flat type NAMES and folds every widened +// spelling back (legacyQtBase), because encoding one correctly needs an element +// loop it has no tree to derive. So the loss is this emitter's, not the +// mapping's — the TypeExpr-driven Qt consumer emitters keep the value type — +// and the note says which surface is affected rather than claiming the table +// still flattens. void noteOptionalPositionalSlots(const ModuleDecl& mod, const QString& where, QTextStream& err) { @@ -58,11 +58,13 @@ void noteOptionalPositionalSlots(const ModuleDecl& mod, const QString& where, if (optSlots.isEmpty()) return; err << "Note: " << where << ": optional positional slot(s) [" << optSlots.join(", ") - << "] are generated as untyped QVariant (LogosMap on the lp surface). A " - "positional slot has no name to carry an optional flag, so `?T` keeps " - "its two states (an invalid QVariant / a JSON null is the empty one) " - "but loses T. Record fields are unaffected — they carry optionality " - "through.\n"; + << "] are generated as untyped QVariant (LogosMap on the lp surface) by " + "THIS emitter, which is keyed on flat type names and folds " + "std::optional back to QVariant. `?T` keeps its two states (an " + "invalid QVariant / a JSON null is the empty one) but loses T here. " + "The TypeExpr-driven Qt consumer emitters keep it as " + "std::optional; record fields are unaffected on every surface — " + "they carry optionality through.\n"; } // Build a getMethods()-shaped QJsonArray (the surface makeHeader/makeSource diff --git a/cpp-generator/plugin_introspect.cpp b/cpp-generator/plugin_introspect.cpp index 1d4a04f..f5b9406 100644 --- a/cpp-generator/plugin_introspect.cpp +++ b/cpp-generator/plugin_introspect.cpp @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include #include "logos_provider_interface.h" #include "generator_lib.h" @@ -19,19 +21,51 @@ #include "experimental/lidl_compat.h" #include "lidl_to_json.h" // ModuleDecl -> the JSON surface generator_lib consumes -// Load events from a `.lidl` sidecar shipped alongside a module's -// pre-built headers. Returns a JSON array of -// { name, params: [ { name, type } ] } -// using Qt-typed type names — same shape generator_lib's makeHeader / -// makeSource already consume for methods. -static QJsonArray loadEventsFromLidl(const QString& lidlPath, QTextStream& err, - QJsonArray* outRecords = nullptr) +// The `.lidl` sidecar a module ships beside its built plugin +// (`/share/logos/.lidl`) — its CONTRACT, parsed into the three JSON +// arrays generator_lib's makeHeader / makeSource consume. +// +// ─── Why the METHODS come from here and not from the plugin ────────────── +// +// This used to load events (and records) only; the methods came from the +// plugin's published `getMethods()`. That made the wrapper's whole type +// surface depend on the VOCABULARY a module happens to publish its metadata +// in, and generator_lib is keyed on flat type NAMES with a QVariant fallback +// (mapParamType / mapReturnType) — so a module that spells its metadata any +// way this emitter does not recognise gets a wrapper of QVariant / LogosMap +// with no diagnostic at all. It is a machine reader of a human-facing +// listing, and it degrades silently. +// +// It measurably broke: when the cdylib backend started publishing the LIDL +// contract vocabulary (`tstr`, `[uint]`, `? tstr`) instead of Qt type names, +// every `interface: "universal"` module's lp wrapper turned into LogosMap. +// Teaching this reader a second vocabulary is not a fix — `int` means a +// 32-bit Qt int in one and a 64-bit LIDL integer in the other, so a merged +// table silently mistypes every integer, and the reader cannot tell from the +// string which table it is holding. +// +// The contract has no such ambiguity: it is a TypeExpr tree, and +// lidl_to_json is the one place it is flattened. So when a module publishes a +// contract, that is what the wrapper is generated from — which also makes +// this path emit byte-identical output to `--general-only --dep +// =.lidl` (main.cpp's generateInterfaceWrappers), the path +// buildHeaders.nix already takes under cross-compilation and for the whole Qt +// surface. Introspection is what is left over for a module that publishes NO +// contract (a handcrafted Qt plugin), where the QMetaObject's Qt type names +// are the only description of its API that exists. +// +// A sidecar that is present but unreadable or malformed is FATAL. Returning +// empty and carrying on is what let a broken sidecar ship a wrapper with no +// typed event accessors and (now) no typed methods — the same silently-empty +// shape generate-module-headers.sh exists to refuse. +static bool loadContractFromLidl(const QString& lidlPath, QTextStream& err, + QJsonArray* outMethods, QJsonArray* outEvents, + QJsonArray* outRecords) { - QJsonArray result; QFile f(lidlPath); if (!f.open(QIODevice::ReadOnly | QIODevice::Text)) { - err << "Failed to open events sidecar: " << lidlPath << "\n"; - return result; + err << "Failed to open contract sidecar: " << lidlPath << "\n"; + return false; } QString source = QString::fromUtf8(f.readAll()); f.close(); @@ -40,12 +74,84 @@ static QJsonArray loadEventsFromLidl(const QString& lidlPath, QTextStream& err, if (pr.hasError()) { err << lidlPath << ":" << pr.errorLine << ":" << pr.errorColumn << ": " << pr.error << "\n"; - return result; + return false; } - noteOptionalPositionalSlots(pr.module, lidlPath, err); - if (outRecords) *outRecords = moduleRecordsToJson(pr.module); - return moduleEventsToJson(pr.module); + ModuleDecl mod = pr.module; + { + QString recErr; + if (!lidlCheckRecords(mod, &recErr)) { + err << lidlPath << ": " << recErr << "\n"; + return false; + } + } + { + // Consumers see name()/version() on every dependency. Added here, not + // read from the artifact: the published .lidl carries only what the + // author wrote, and the provider adds the same two methods from the + // same function (main.cpp's --backend cdylib path), so the two sides + // cannot disagree about them. + QString idErr; + if (!lidlInjectIdentity(mod, &idErr)) { + err << lidlPath << ": " << idErr << "\n"; + return false; + } + } + + noteOptionalPositionalSlots(mod, lidlPath, err); + if (outMethods) *outMethods = moduleMethodsToJson(mod); + if (outEvents) *outEvents = moduleEventsToJson(mod); + if (outRecords) *outRecords = moduleRecordsToJson(mod); + return true; +} + +// Is this published type name spelled in the LIDL CONTRACT vocabulary rather +// than in Qt type names? +// +// The two vocabularies are not distinguishable in general — `int` and `bool` +// are words in both, at different widths — which is exactly why this emitter +// must not try to read both. But it does not have to: those overlapping words +// are all in mapParamType/mapReturnType's known table, so they never reach the +// fallback. What reaches the fallback and is UNAMBIGUOUS is the LIDL half that +// Qt has no word for at all: the primitive names below, and any container or +// optional, which start with a character no C++ type name starts with. +// +// Deliberately NOT a second type table. The answer is only ever used to REFUSE +// — see below — so a false negative degrades to the old behaviour and a false +// positive is impossible: no Qt type is called `tstr`, and none begins with +// `[`, `{` or `?`. +static bool looksLikeLidlSpelling(const QString& raw) +{ + const QString t = raw.trimmed(); + if (t.isEmpty()) return false; + if (t.startsWith('[') || t.startsWith('{') || t.startsWith('?')) return true; + static const QSet unambiguous = { + QStringLiteral("tstr"), QStringLiteral("bstr"), QStringLiteral("uint"), + QStringLiteral("float64"), QStringLiteral("result"), QStringLiteral("any"), + }; + return unambiguous.contains(t); +} + +// Every LIDL-spelled type name in a published listing, as "method: type" for a +// diagnostic. Empty when the listing is in Qt names, which is the only +// vocabulary this emitter can read. +static QStringList lidlSpelledSlots(const QJsonArray& methods) +{ + QStringList out; + for (const QJsonValue& mv : methods) { + if (!mv.isObject()) continue; + const QJsonObject mo = mv.toObject(); + const QString name = mo.value("name").toString(); + const QString ret = mo.value("returnType").toString(); + if (looksLikeLidlSpelling(ret)) + out << (name + "() -> " + ret); + for (const QJsonValue& pv : mo.value("parameters").toArray()) { + const QString pt = pv.toObject().value("type").toString(); + if (looksLikeLidlSpelling(pt)) + out << (name + "(" + pv.toObject().value("name").toString() + ": " + pt + ")"); + } + } + return out; } // The interface/dependency wrapper machinery (InterfaceSpec, parseSpecFlags, @@ -104,7 +210,10 @@ static QJsonArray enumerateMethods(QObject* moduleInstance) // makeSource -> generator_lib.h/cpp -static int generateFromPlugin(const QString& pluginInputPath, const QString& outputDir, ApiStyle apiStyle, const QJsonArray& events, QTextStream& out, QTextStream& err, const QJsonArray& records = {}) +// `contractMethods` is the module's own contract, when it ships one; empty +// when it does not. Non-empty wins over whatever the plugin publishes — see +// loadContractFromLidl for why the published metadata is not a type source. +static int generateFromPlugin(const QString& pluginInputPath, const QString& outputDir, ApiStyle apiStyle, const QJsonArray& events, QTextStream& out, QTextStream& err, const QJsonArray& records = {}, const QJsonArray& contractMethods = {}) { QFileInfo fi(pluginInputPath); if (!fi.exists()) { @@ -142,20 +251,101 @@ static int generateFromPlugin(const QString& pluginInputPath, const QString& out } } - QJsonArray methods; + // What the PLUGIN says about itself. Still read even when a contract is + // present: loading the plugin is the dlopen check this path exists to + // perform (exit 3 on an SDK/ABI skew), and comparing the two name sets is + // the only place a stale sidecar can be noticed at all. + QJsonArray publishedMethods; LogosProviderPlugin* providerPlugin = qobject_cast(instance); if (providerPlugin) { LogosProviderObject* provider = providerPlugin->createProviderObject(); if (provider) { - methods = provider->getMethods(); + publishedMethods = provider->getMethods(); out << "Detected new-API plugin (LogosProviderPlugin), using getMethods() — " - << methods.size() << " methods\n"; + << publishedMethods.size() << " methods\n"; delete provider; } else { err << "LogosProviderPlugin::createProviderObject() returned null\n"; } } else { - methods = enumerateMethods(instance); + publishedMethods = enumerateMethods(instance); + } + + // Contract-first. The published listing is a HUMAN-facing description + // whose vocabulary is the publisher's choice; the contract is the typed + // one. Only a module that ships no contract is described by its plugin. + QJsonArray methods = publishedMethods; + if (contractMethods.isEmpty()) { + // No contract, so the listing is the only description there is — and + // it has to be one this emitter can READ. It is keyed on Qt type names + // with a QVariant fallback, so a listing in the LIDL contract + // vocabulary produces a wrapper of QVariant / LogosMap that compiles + // and has lost every type. That is not hypothetical: it is what + // happened to every `interface: "universal"` module when the cdylib + // backend switched its published metadata to LIDL names. + // + // REFUSED, not warned. The build systems always pass --events-from for + // a module that ships a contract, so reaching here with LIDL names + // means either a hand-run invocation that omitted the flag (the shape + // the docs used to suggest) or a caller that lost it — and in both + // cases the wrapper would be silently untyped. The message names the + // flag, because the fix is always the same one file. + const QStringList lidlSlots = lidlSpelledSlots(publishedMethods); + if (!lidlSlots.isEmpty()) { + err << "Error: '" << moduleName << "' publishes its metadata in the LIDL\n" + << " contract vocabulary, and no --events-from contract was given.\n" + << " Offending slots (up to 8): [" << lidlSlots.mid(0, 8).join(", ") + << "]\n" + << " This emitter reads Qt type names and falls back to QVariant\n" + << " (LogosMap on the lp surface) for anything else, so generating\n" + << " from this listing would emit a wrapper that compiles and has\n" + << " lost every type — with no diagnostic anywhere downstream.\n" + << " Pass --events-from /share/logos/" << moduleName + << ".lidl, the contract\n" + << " the module installs beside its plugin. buildHeaders.nix does\n" + << " this automatically; a hand-run invocation has to say it.\n"; + loader.unload(); + return 7; + } + } else { + methods = contractMethods; + out << "Using the module's LIDL contract for the method surface — " + << methods.size() << " methods (the plugin's published listing is a " + << "description, not a type source)\n"; + + // A stale sidecar is the one way this can now be wrong, and it is + // otherwise invisible: the wrapper would compile and simply not have + // the method. Reported, not fatal — the two sets legitimately differ + // for a plugin whose QMetaObject carries Qt-only slots. + // INVOKABLE entries only, on both sides. A cdylib publishes its + // events into the same array, tagged `"type": "event"` and with no + // `isInvokable` — makeHeader/makeSourceLp already skip those, and + // counting them here would report a divergence for every module that + // declares an event. + auto namesOf = [](const QJsonArray& a) { + QSet n; + for (const QJsonValue& v : a) { + if (!v.isObject()) continue; + const QJsonObject o = v.toObject(); + if (!o.value("isInvokable").toBool()) continue; + n.insert(o.value("name").toString()); + } + return n; + }; + const QSet fromContract = namesOf(contractMethods); + const QSet fromPlugin = namesOf(publishedMethods); + const QStringList onlyContract = QStringList(QList((fromContract - fromPlugin).begin(), (fromContract - fromPlugin).end())); + const QStringList onlyPlugin = QStringList(QList((fromPlugin - fromContract).begin(), (fromPlugin - fromContract).end())); + if (!onlyContract.isEmpty() || !onlyPlugin.isEmpty()) { + err << "Note: the contract and the built plugin list different methods for '" + << moduleName << "'."; + if (!onlyContract.isEmpty()) + err << " Contract only: [" << onlyContract.join(", ") << "]."; + if (!onlyPlugin.isEmpty()) + err << " Plugin only: [" << onlyPlugin.join(", ") << "]."; + err << " The wrapper follows the CONTRACT; a method listed only by the " + "plugin is not reachable through it.\n"; + } } QString className = toPascalCase(moduleName); @@ -170,9 +360,11 @@ static int generateFromPlugin(const QString& pluginInputPath, const QString& out // passed --api-style=lp (typically because it's `interface: // "universal"` or `"cdylib"`). // Both produce the same filename and class name, so the umbrella - // doesn't need to know which style was picked. `events` (loaded - // from a sibling `.lidl` sidecar via --events-from) adds typed - // `on(callback)` accessors next to the existing methods. + // doesn't need to know which style was picked. `methods`, `events` and + // `records` all come from the same sibling `.lidl` sidecar + // (--events-from) when the module ships one — that is one contract in, + // one wrapper out, and it is what makes this path agree with + // `--general-only --dep =.lidl`. QString header = makeHeader(moduleName, className, methods, apiStyle, events, BindMode::Static, records); QString source = makeSource(moduleName, className, headerRel, methods, apiStyle, events, BindMode::Static, records); @@ -335,11 +527,20 @@ int runPluginIntrospectMode(int argc, char* argv[]) return 1; } - // --events-from : load typed event prototypes from a LIDL - // sidecar shipped alongside a dep's pre-built headers. When set, - // the consumer wrapper (_api.{h,cpp}) gains typed - // `on(callback)` accessors next to the existing - // generic `onEvent(name, callback)` channel. + // --events-from : the module's `.lidl` CONTRACT, shipped beside its + // built plugin. The flag keeps its name — generate-module-headers.sh and + // buildHeaders.nix in logos-plugin-qt pass it, and logos-plugin-qt's + // test-header-generator-guard asserts the spelling — but the file it + // names has always been the whole contract, and everything the wrapper is + // generated from now comes out of it: the typed methods, the typed + // `on(callback)` accessors, and the record structs. + // + // Absent (a handcrafted Qt module publishes no contract) means the + // wrapper is generated from the plugin's QMetaObject, exactly as before. + // NAMED BUT MISSING is a refusal rather than a fallback: silently + // introspecting instead would emit a wrapper that compiles and is wrong + // in a way nothing downstream can see. + QJsonArray methodsFromSidecar; QJsonArray eventsFromSidecar; QJsonArray recordsFromSidecar; { @@ -355,11 +556,24 @@ int runPluginIntrospectMode(int argc, char* argv[]) } } } - if (!evPath.isEmpty() && QFileInfo(evPath).exists()) { - eventsFromSidecar = loadEventsFromLidl(evPath, err, &recordsFromSidecar); + if (!evPath.isEmpty()) { + if (!QFileInfo(evPath).exists()) { + err << "Error: --events-from names a contract that does not exist: " + << evPath << "\n" + << " The wrapper's methods, events and records all come from\n" + << " this file. Generating from the plugin's published metadata\n" + << " instead would emit a wrapper of untyped QVariant / LogosMap\n" + << " that compiles and silently loses every type.\n"; + return 2; + } + if (!loadContractFromLidl(evPath, err, &methodsFromSidecar, + &eventsFromSidecar, &recordsFromSidecar)) { + return 4; + } } } QString argPath = args.at(1); - return generateFromPlugin(argPath, outputDir, apiStyle, eventsFromSidecar, out, err, recordsFromSidecar); + return generateFromPlugin(argPath, outputDir, apiStyle, eventsFromSidecar, out, err, + recordsFromSidecar, methodsFromSidecar); } diff --git a/doctests/cpp-sdk-generator-roundtrip.test.yaml b/doctests/cpp-sdk-generator-roundtrip.test.yaml index 7611e5d..9b6c4be 100644 --- a/doctests/cpp-sdk-generator-roundtrip.test.yaml +++ b/doctests/cpp-sdk-generator-roundtrip.test.yaml @@ -274,10 +274,14 @@ sections: variant that hands it a `logos::AsyncResult` (`{value, error}`) so a failed call is distinguishable from a legitimately default-valued one — and each `event` an `on(...)` subscription. The type mapping is the Qt - caller style: + caller style, and it is **lossless** — one LIDL type, one C++ spelling: `float64`→`double`, `tstr`→`QString`, `int`→`qlonglong`, `uint`→ - `qulonglong`, `bstr`→`QByteArray`, `[tstr]`→`QStringList`, other - arrays→`QVariantList`, and `result`→`LogosResult`. + `qulonglong`, `bstr`→`QByteArray`, `result`→`LogosResult`, and each + container carries its element type through — `[tstr]`→`QStringList`, + every other `[T]`→`QList` (so `[uint]` is `QList`). + Only `any` stays `QVariant`: it is the one Qt type that holds bytes *and* + an exact `uint64` *and* arbitrary nesting, so narrowing it would lose what + it was chosen to carry. The integer spellings are 64-bit, and unsigned stays unsigned: LIDL `int`/`uint` are `int64_t`/`uint64_t` in every other binding, so spelling @@ -302,7 +306,7 @@ sections: - "double temperature(" - "qlonglong record(qulonglong id, double value, const QString& note, bool valid" - "QByteArray firmware(const QByteArray& image" - - "QStringList labels(const QVariantList& ids" + - "QStringList labels(const QList& ids" - "LogosResult reset(const QString& id" - title: "The three call surfaces per method" text: | @@ -334,10 +338,14 @@ sections: generates `struct Point` plus the conversions, so a caller writes `Point p = client.translate(q, 1, 2)` instead of digging fields out of a `QVariantMap`. `[Point]` is a `QList` and `{tstr: Point}` a - `QMap`. Maps of `any`, optionals (`?T`) and `any` itself - still cross as untyped JSON and stay `QVariantMap` / `QVariant` — a record - has a declared shape, those do not. Here is a contract that uses all of - them, taken straight to a consumer header. + `QMap`. An **optional** `?T` is a + `std::optional` — two-state in the type system, so an absent value is + not spelled the same way as a present default, and `?uint` is + distinguishable from `?tstr`. Only `any` — and any container whose element + type bottoms out at `any`, such as `{tstr: any}` — stays untyped, as + `QVariant` / `QVariantMap`: a record has a declared shape and an `any` does + not. Here is a contract that uses all of them, taken straight to a consumer + header. steps: - title: "geometry_module.lidl" file: @@ -374,10 +382,15 @@ sections: text: | `Point` is generated as a struct, so a record parameter is taken by const-ref and a record return comes back typed. An array-of-records is a - `QList`. A map of `any`, an optional and a bare `any` stay - `QVariantMap` / `QVariant` — the untyped carriers for JSON whose shape - the contract does not declare. - run: "grep -E 'class GeometryModule|struct Point|Point translate|QList|QVariantMap attributes|QVariant nearest' geometry/geometry_module_api.h" + `QList`. `nearest` shows both halves of the optional mapping in + one signature — `?uint` in, `?Point` out — as + `std::optional` and `std::optional`: the caller can + ask `limit.has_value()` and the return can be *nothing* without + colliding with a legitimate `Point{0, 0}`. Only `attributes` and + `describe` stay `QVariantMap` / `QVariant`: `{tstr: any}` and `any` are + the untyped carriers for JSON whose shape the contract does not + declare. + run: "grep -E 'class GeometryModule|struct Point|Point translate|QList|QVariantMap attributes|std::optional nearest' geometry/geometry_module_api.h" code_block: | grep -E 'class|translate|bounds|attributes|nearest' geometry/geometry_module_api.h expect_contains: @@ -386,4 +399,4 @@ sections: - "Point translate(const Point& p, double dx, double dy" - "Point bounds(const QList& points" - "QVariantMap attributes(const QVariantMap& tags" - - "QVariant nearest(const Point& p, QVariant limit" + - "std::optional nearest(const Point& p, const std::optional& limit" diff --git a/doctests/outputs/cpp-sdk-generator-roundtrip.md b/doctests/outputs/cpp-sdk-generator-roundtrip.md index 937d780..5583947 100644 --- a/doctests/outputs/cpp-sdk-generator-roundtrip.md +++ b/doctests/outputs/cpp-sdk-generator-roundtrip.md @@ -258,10 +258,15 @@ cat provider/sensor_module_events_cdylib.cpp The consumer side. From the same contract, `--module-only` emits the typed wrapper a *consumer* compiles against to call `sensor_module`. Each LIDL `method` becomes a synchronous caller plus an `…Async` variant, and each -`event` an `on(...)` subscription. The type mapping is the Qt caller style: +`event` an `on(...)` subscription. The type mapping is the Qt caller style, +and it is **lossless** — one LIDL type, one C++ spelling: `float64`→`double`, `tstr`→`QString`, `int`→`qlonglong`, `uint`→ -`qulonglong`, `bstr`→`QByteArray`, `[tstr]`→`QStringList`, other -arrays→`QVariantList`, and `result`→`LogosResult`. +`qulonglong`, `bstr`→`QByteArray`, `result`→`LogosResult`, and each +container carries its element type through — `[tstr]`→`QStringList`, +every other `[T]`→`QList` (so `[uint]` is `QList`). +Only `any` stays `QVariant`: it is the one Qt type that holds bytes *and* +an exact `uint64` *and* arbitrary nesting, so narrowing it would lose what +it was chosen to carry. The integer spellings are 64-bit, and unsigned stays unsigned: LIDL `int`/`uint` are `int64_t`/`uint64_t` in every other binding, so spelling @@ -293,10 +298,14 @@ A **record becomes a real C++ struct**: a `type Point { … }` in the contract generates `struct Point` plus the conversions, so a caller writes `Point p = client.translate(q, 1, 2)` instead of digging fields out of a `QVariantMap`. `[Point]` is a `QList` and `{tstr: Point}` a -`QMap`. Maps of `any`, optionals (`?T`) and `any` itself -still cross as untyped JSON and stay `QVariantMap` / `QVariant` — a record -has a declared shape, those do not. Here is a contract that uses all of -them, taken straight to a consumer header. +`QMap`. An **optional** `?T` is a +`std::optional` — two-state in the type system, so an absent value is +not spelled the same way as a present default, and `?uint` is +distinguishable from `?tstr`. Only `any` — and any container whose element +type bottoms out at `any`, such as `{tstr: any}` — stays untyped, as +`QVariant` / `QVariantMap`: a record has a declared shape and an `any` does +not. Here is a contract that uses all of them, taken straight to a consumer +header. ### 6.1 geometry_module.lidl @@ -331,9 +340,14 @@ logos-cpp-generator --lidl geometry_module.lidl \ `Point` is generated as a struct, so a record parameter is taken by const-ref and a record return comes back typed. An array-of-records is a -`QList`. A map of `any`, an optional and a bare `any` stay -`QVariantMap` / `QVariant` — the untyped carriers for JSON whose shape -the contract does not declare. +`QList`. `nearest` shows both halves of the optional mapping in +one signature — `?uint` in, `?Point` out — as +`std::optional` and `std::optional`: the caller can +ask `limit.has_value()` and the return can be *nothing* without +colliding with a legitimate `Point{0, 0}`. Only `attributes` and +`describe` stay `QVariantMap` / `QVariant`: `{tstr: any}` and `any` are +the untyped carriers for JSON whose shape the contract does not +declare. ```bash grep -E 'class|translate|bounds|attributes|nearest' geometry/geometry_module_api.h diff --git a/nix/tests-generator-cli.nix b/nix/tests-generator-cli.nix index d42d846..08f1d18 100644 --- a/nix/tests-generator-cli.nix +++ b/nix/tests-generator-cli.nix @@ -148,6 +148,43 @@ pkgs.runCommand "${common.pname}-generator-cli-tests" grep -q "asserted" anon.err || { cat anon.err >&2; fail "the anonymous-origin refusal does not explain itself"; } echo "OK: --binding origin refuses a module that cannot name itself" + # ── --events-from names the CONTRACT, and a missing one is refused ──── + # + # On the plugin path the wrapper's typed methods, records and event + # accessors all come out of the file this flag names. Shrugging off a + # missing one and introspecting instead would emit a wrapper that compiles + # and has lost every type — the same silently-empty shape + # generate-module-headers.sh exists to refuse, one layer down. + # + # No plugin is needed to assert it: the contract is loaded BEFORE the + # plugin is opened, so a missing sidecar is reported even for a plugin path + # that does not exist. The control below is what makes that meaningful — + # with a readable contract the SAME command gets as far as the plugin and + # fails on the plugin instead. + printf 'module cli_probe_module {\n version "1.0.0"\n method ping() -> tstr\n}\n' > probe.lidl + + set +e + logos-cpp-generator ./nonexistent_plugin.dylib --module-only --api-style lp \ + --events-from ./nonexistent.lidl --output-dir ./gen-nosidecar \ + >nosidecar.out 2>nosidecar.err + status=$? + set -e + [ "$status" -ne 0 ] || fail "--events-from accepted a contract that does not exist" + grep -q -- '--events-from names a contract that does not exist' nosidecar.err \ + || { cat nosidecar.err >&2; fail "a missing contract failed without saying why"; } + echo "OK: --events-from refuses a contract that does not exist" + + set +e + logos-cpp-generator ./nonexistent_plugin.dylib --module-only --api-style lp \ + --events-from ./probe.lidl --output-dir ./gen-sidecar \ + >sidecar.out 2>sidecar.err + status=$? + set -e + [ "$status" -ne 0 ] || fail "control: a nonexistent plugin exited 0" + grep -q 'Plugin file does not exist' sidecar.err \ + || { cat sidecar.err >&2; fail "control: a READABLE contract did not get as far as the plugin"; } + echo "OK: control — a readable contract is accepted and the run reaches the plugin" + mkdir -p "$out" echo "logos-cpp-generator CLI argument-surface tests passed" > "$out/result.txt" '' diff --git a/tests/experimental/test_lidl_gen_cdylib.cpp b/tests/experimental/test_lidl_gen_cdylib.cpp index 39adaa1..df9b085 100644 --- a/tests/experimental/test_lidl_gen_cdylib.cpp +++ b/tests/experimental/test_lidl_gen_cdylib.cpp @@ -956,3 +956,86 @@ TEST(LidlGenCdylib, TheEventsSidecarDoesNotDefineTheCallCallerExport) const QString events = lidlMakeEventsSourceCdylib(m, "DeliveryImpl", "delivery_impl.h"); EXPECT_FALSE(events.contains("logos_module_set_call_caller")) << events.toStdString(); } + +// --------------------------------------------------------------------------- +// getMethods() publishes the CONTRACT vocabulary +// +// `returnType`, `parameters[].type` and `signature` used to be Qt type names, +// which made a Qt-free cdylib module describe itself in the types of a language +// it does not use — and answered three different LIDL types (`[uint]`, +// `[bstr]`, `[any]`) with one word, QVariantList, so the listing could not be +// read back. Every consumer of these fields (`lm`, logoscore's method listing, +// basecamp's inspector) is showing a human what the module offers; the +// dispatch paths key on QMetaObject types and never touch this JSON. +// --------------------------------------------------------------------------- + +namespace { + +TypeExpr arrOf(const TypeExpr& e) { return TypeExpr{ TypeExpr::Array, "", { e } }; } +TypeExpr mapOf(const TypeExpr& v) +{ + return TypeExpr{ TypeExpr::Map, "", { prim("tstr"), v } }; +} +TypeExpr optionalOf(const TypeExpr& e) { return TypeExpr{ TypeExpr::Optional, "", { e } }; } + +} // namespace + +TEST(LidlGenCdylib, PublishedTypesAreTheLidlSpelling) +{ + ModuleDecl m = moduleWithMethod(method("echo_uints", arrOf(prim("uint")), + { param("v", arrOf(prim("uint"))) })); + const QString src = implExportsFor(m); + EXPECT_TRUE(src.contains("obj[\"returnType\"] = \"[uint]\"")) << src.toStdString(); + EXPECT_TRUE(src.contains("obj[\"signature\"] = \"echo_uints([uint])\"")) << src.toStdString(); + EXPECT_TRUE(src.contains("{\"type\", \"[uint]\"}")) << src.toStdString(); + // The Qt vocabulary is GONE from the published surface. + EXPECT_FALSE(src.contains("\"QVariantList\"")) << src.toStdString(); +} + +// Three LIDL types that all used to publish as QVariantList now publish as +// themselves. That distinction is the whole point: the listing is a contract. +TEST(LidlGenCdylib, PublishedTypesDistinguishWhatQtCollapsed) +{ + const QString uints = implExportsFor( + moduleWithMethod(method("m", arrOf(prim("uint")), {}))); + const QString blobs = implExportsFor( + moduleWithMethod(method("m", arrOf(prim("bstr")), {}))); + const QString anys = implExportsFor( + moduleWithMethod(method("m", arrOf(prim("any")), {}))); + EXPECT_TRUE(uints.contains("obj[\"returnType\"] = \"[uint]\"")); + EXPECT_TRUE(blobs.contains("obj[\"returnType\"] = \"[bstr]\"")); + EXPECT_TRUE(anys.contains("obj[\"returnType\"] = \"[any]\"")); +} + +// A record publishes its DECLARED NAME — what the contract calls it — not +// QVariantMap. The historical objection was that these strings were read as +// metatypes; nothing in the runtime does that any more. +TEST(LidlGenCdylib, PublishedRecordTypesUseTheDeclaredName) +{ + ModuleDecl m = moduleWithMethod( + method("bounds", TypeExpr{ TypeExpr::Named, "Blob", {} }, + { param("points", arrOf(TypeExpr{ TypeExpr::Named, "Blob", {} })) })); + TypeDecl t; + t.name = "Blob"; + FieldDecl f; + f.name = "payload"; + f.type = prim("bstr"); + t.fields.push_back(f); + m.types.push_back(t); + + const QString src = implExportsFor(m); + EXPECT_TRUE(src.contains("obj[\"returnType\"] = \"Blob\"")) << src.toStdString(); + EXPECT_TRUE(src.contains("obj[\"signature\"] = \"bounds([Blob])\"")) << src.toStdString(); +} + +TEST(LidlGenCdylib, PublishedTypesSpellMapsAndOptionals) +{ + const QString maps = implExportsFor( + moduleWithMethod(method("m", mapOf(prim("uint")), {}))); + EXPECT_TRUE(maps.contains("obj[\"returnType\"] = \"{tstr: uint}\"")) << maps.toStdString(); + + const QString opts = implExportsFor(moduleWithMethod( + method("m", prim("bool"), { param("id", optionalOf(prim("tstr"))) }))); + EXPECT_TRUE(opts.contains("obj[\"signature\"] = \"m(? tstr)\"")) << opts.toStdString(); + EXPECT_TRUE(opts.contains("{\"type\", \"? tstr\"}")) << opts.toStdString(); +} diff --git a/tests/experimental/test_lidl_gen_client.cpp b/tests/experimental/test_lidl_gen_client.cpp index 92df1d0..57a2ac4 100644 --- a/tests/experimental/test_lidl_gen_client.cpp +++ b/tests/experimental/test_lidl_gen_client.cpp @@ -436,21 +436,26 @@ TEST(LidlGenClient, BothOptionalSpellingsEmitIdenticalCode) EXPECT_EQ(flagged, typed) << flagged.toStdString() << "\n---\n" << typed.toStdString(); } -TEST(LidlGenClient, OptionalRecordFieldIsTwoStateQVariant) +TEST(LidlGenClient, OptionalRecordFieldKeepsItsValueType) { const QString h = lidlMakeHeader(makeOptionalRecordModule(true), BindMode::Bound); - // QVariant, because Qt has no optional and an invalid QVariant is its one - // empty inhabitant. - EXPECT_TRUE(h.contains("QVariant nickname{};")) << h.toStdString(); + // std::optional, not a bare QVariant. The field is still TWO-state + // — std::nullopt is C++'s single empty inhabitant — but the consumer can + // now see that the value is a string, which is what the std surface next + // door has always told it. + EXPECT_TRUE(h.contains("std::optional nickname{};")) << h.toStdString(); EXPECT_TRUE(h.contains("QString required{};")) << h.toStdString(); + EXPECT_TRUE(h.contains("#include ")) << h.toStdString(); // A record field is a NAMED slot: empty omits the key rather than writing an - // invalid QVariant into the map. - EXPECT_TRUE(h.contains("if (v.nickname.isValid())")) << h.toStdString(); - // Absent and null both arrive as an invalid QVariant. Converting (the + // empty value into the map. + EXPECT_TRUE(h.contains("if (v.nickname.has_value())")) << h.toStdString(); + // Absent and null both arrive as an invalid QVariant, and the optional + // decode turns exactly that into nullopt. A bare conversion (the // `.toString()` a required tstr field gets) would have turned "empty" into // "", which is a VALUE. - EXPECT_TRUE(h.contains("__out.nickname = __m.value(\"nickname\");")) << h.toStdString(); + EXPECT_TRUE(h.contains("if (!__s.isValid() || __s.isNull()) return std::optional();")) + << h.toStdString(); EXPECT_FALSE(h.contains("__out.nickname = __m.value(\"nickname\").toString();")) << h.toStdString(); } diff --git a/tests/experimental/test_lidl_type_mapping.cpp b/tests/experimental/test_lidl_type_mapping.cpp index 445076f..bff3726 100644 --- a/tests/experimental/test_lidl_type_mapping.cpp +++ b/tests/experimental/test_lidl_type_mapping.cpp @@ -2,6 +2,10 @@ #include "lidl_gen_client.h" #include "lidl_emit_common.h" +#include + +#include "lidl/serializer.hpp" + // --------------------------------------------------------------------------- // lidlTypeToQt // --------------------------------------------------------------------------- @@ -68,7 +72,7 @@ TEST(LidlTypeToQt, ArrayOfInt) { TypeExpr elem = { TypeExpr::Primitive, "int", {} }; TypeExpr te = { TypeExpr::Array, "", { elem } }; - EXPECT_EQ(lidlTypeToQt(te), "QVariantList"); + EXPECT_EQ(lidlTypeToQt(te), "QList"); } TEST(LidlTypeToQt, MapType) @@ -76,14 +80,14 @@ TEST(LidlTypeToQt, MapType) TypeExpr key = { TypeExpr::Primitive, "tstr", {} }; TypeExpr val = { TypeExpr::Primitive, "int", {} }; TypeExpr te = { TypeExpr::Map, "", { key, val } }; - EXPECT_EQ(lidlTypeToQt(te), "QVariantMap"); + EXPECT_EQ(lidlTypeToQt(te), "QMap"); } TEST(LidlTypeToQt, OptionalType) { TypeExpr inner = { TypeExpr::Primitive, "tstr", {} }; TypeExpr te = { TypeExpr::Optional, "", { inner } }; - EXPECT_EQ(lidlTypeToQt(te), "QVariant"); + EXPECT_EQ(lidlTypeToQt(te), "std::optional"); } // A Named type is a RECORD declared by the contract, and the client generator @@ -268,12 +272,238 @@ TEST(LidlTypeToStd, DegenerateOptionalTerminates) EXPECT_EQ(lidlTypeToStd(TypeExpr{ TypeExpr::Optional, "", {} }), "QVariant"); } -TEST(LidlTypeToQt, OptionalIsUntypedQVariant) +// `?T` KEEPS T on the Qt surface. It used to be a bare QVariant — the one row +// in this table that lost the value type, so a consumer could not tell `?tstr` +// from `?uint` — while the std table next door kept both through +// std::optional. The two surfaces now answer the same question the same way. +TEST(LidlTypeToQt, OptionalKeepsItsValueType) { - // Deliberate, and the one mapping in the Qt table that loses the value - // type: there is no metatype for an optional, and this name is read by the - // host to marshal a QVariant across the plugin boundary. Two-state survives - // (an invalid QVariant is the empty inhabitant); T does not. - EXPECT_EQ(lidlTypeToQt(opt({ TypeExpr::Primitive, "tstr", {} })), "QVariant"); - EXPECT_EQ(lidlTypeToQt(opt({ TypeExpr::Named, "Blob", {} })), "QVariant"); + EXPECT_EQ(lidlTypeToQt(opt({ TypeExpr::Primitive, "tstr", {} })), "std::optional"); + EXPECT_EQ(lidlTypeToQt(opt({ TypeExpr::Primitive, "uint", {} })), "std::optional"); + EXPECT_EQ(lidlTypeToQt(opt({ TypeExpr::Named, "Blob", {} })), "std::optional"); +} + +// --------------------------------------------------------------------------- +// The widened Qt table — every row, and the rows that must NOT widen +// +// The table is the whole of this change's contract with the Qt emitters: they +// spell signatures from it and decide, from lidlQtNeedsElementLoop, whether a +// value may cross whole. A wrong row here is a silently-dropped payload three +// layers down, so each is stated on its own. +// --------------------------------------------------------------------------- + +namespace { + +TypeExpr prim(const char* n) { return TypeExpr{ TypeExpr::Primitive, n, {} }; } +TypeExpr named(const char* n) { return TypeExpr{ TypeExpr::Named, n, {} }; } +TypeExpr arr(const TypeExpr& e) { return TypeExpr{ TypeExpr::Array, "", { e } }; } +TypeExpr map(const TypeExpr& v) { return TypeExpr{ TypeExpr::Map, "", { prim("tstr"), v } }; } +TypeExpr optOf(const TypeExpr& e) { return TypeExpr{ TypeExpr::Optional, "", { e } }; } + +} // namespace + +TEST(LidlTypeToQtWidened, TypedArrays) +{ + EXPECT_EQ(lidlTypeToQt(arr(prim("uint"))), "QList"); + EXPECT_EQ(lidlTypeToQt(arr(prim("int"))), "QList"); + EXPECT_EQ(lidlTypeToQt(arr(prim("bool"))), "QList"); + EXPECT_EQ(lidlTypeToQt(arr(prim("float64"))), "QList"); + EXPECT_EQ(lidlTypeToQt(arr(prim("bstr"))), "QList"); + EXPECT_EQ(lidlTypeToQt(arr(named("Point"))), "QList"); +} + +// `[tstr]` stays QStringList — the one typed array Qt has a native spelling +// for, and the one already in qvariantToNlohmann's closed userType() set. It +// crosses WHOLE, so it must not be given an element loop it does not need. +TEST(LidlTypeToQtWidened, ArrayOfTstrStaysQStringList) +{ + EXPECT_EQ(lidlTypeToQt(arr(prim("tstr"))), "QStringList"); + EXPECT_FALSE(lidlQtNeedsElementLoop(arr(prim("tstr")))); +} + +TEST(LidlTypeToQtWidened, TypedMaps) +{ + EXPECT_EQ(lidlTypeToQt(map(prim("uint"))), "QMap"); + EXPECT_EQ(lidlTypeToQt(map(prim("tstr"))), "QMap"); + EXPECT_EQ(lidlTypeToQt(map(prim("bstr"))), "QMap"); + EXPECT_EQ(lidlTypeToQt(map(named("Point"))), "QMap"); +} + +TEST(LidlTypeToQtWidened, RecursionAtDepth) +{ + EXPECT_EQ(lidlTypeToQt(arr(arr(prim("uint")))), "QList>"); + EXPECT_EQ(lidlTypeToQt(map(arr(prim("uint")))), "QMap>"); + EXPECT_EQ(lidlTypeToQt(arr(map(prim("uint")))), "QList>"); + EXPECT_EQ(lidlTypeToQt(arr(optOf(prim("tstr")))), "QList>"); + EXPECT_EQ(lidlTypeToQt(optOf(arr(prim("uint")))), "std::optional>"); + EXPECT_EQ(lidlTypeToQt(arr(arr(named("Point")))), "QList>"); +} + +// THE `any` RULE. `any` is the only row the widened table keeps untyped, and it +// is not an omission: QVariant is the sole Qt type that carries bytes AND an +// exact uint64 AND arbitrary nesting, so every narrower spelling would LOSE +// something. A type whose scalar leaf is `any` therefore keeps the QVariant- +// family name at every depth. +TEST(LidlTypeToQtWidened, AnyBottomedShapesKeepTheQVariantSpelling) +{ + EXPECT_EQ(lidlTypeToQt(prim("any")), "QVariant"); + EXPECT_EQ(lidlTypeToQt(arr(prim("any"))), "QVariantList"); + EXPECT_EQ(lidlTypeToQt(map(prim("any"))), "QVariantMap"); + EXPECT_EQ(lidlTypeToQt(optOf(prim("any"))), "QVariant"); + EXPECT_EQ(lidlTypeToQt(arr(arr(prim("any")))), "QVariantList"); + EXPECT_EQ(lidlTypeToQt(map(arr(prim("any")))), "QVariantMap"); + EXPECT_EQ(lidlTypeToQt(arr(map(prim("any")))), "QVariantList"); + EXPECT_EQ(lidlTypeToQt(optOf(arr(prim("any")))), "QVariant"); +} + +// `??T` denotes the SAME two states as `?T` — optionality is idempotent under +// the two-state rule — so it must collapse rather than become a three-state +// std::optional>. Recursed through optionalValueType(), which +// is the frontend's own accessor. +TEST(LidlTypeToQtWidened, NestedOptionalCollapses) +{ + EXPECT_EQ(lidlTypeToQt(optOf(optOf(prim("tstr")))), "std::optional"); + EXPECT_EQ(lidlTypeToQt(optOf(optOf(optOf(prim("uint"))))), "std::optional"); +} + +// Degenerate shapes are unreachable from the parser but constructible by hand +// and over the JSON bridge. They must terminate, and they must land on the +// opaque spelling rather than being described as typed. +TEST(LidlTypeToQtWidened, DegenerateShapesTerminate) +{ + EXPECT_EQ(lidlTypeToQt(TypeExpr{ TypeExpr::Optional, "", {} }), "QVariant"); + EXPECT_EQ(lidlTypeToQt(TypeExpr{ TypeExpr::Array, "", {} }), "QVariantList"); + EXPECT_EQ(lidlTypeToQt(TypeExpr{ TypeExpr::Map, "", {} }), "QVariantMap"); +} + +// The record-qualifier hook. A wrapper nests its record structs in the wrapper +// class, so a type written outside that scope has to qualify them — at EVERY +// depth, which is what a string match on the finished name could not do once +// `?Point` and `QList>` became spellable. +TEST(LidlTypeToQtWidened, RecordQualifierReachesEveryDepth) +{ + auto qual = [](const QString& n) { return "Wrapper::" + n; }; + EXPECT_EQ(lidlTypeToQt(named("Point"), qual), "Wrapper::Point"); + EXPECT_EQ(lidlTypeToQt(optOf(named("Point")), qual), "std::optional"); + EXPECT_EQ(lidlTypeToQt(arr(arr(named("Point"))), qual), "QList>"); + EXPECT_EQ(lidlTypeToQt(map(optOf(named("Point"))), qual), + "QMap>"); + // Nothing else is touched by the hook. + EXPECT_EQ(lidlTypeToQt(arr(prim("uint")), qual), "QList"); +} + +// --------------------------------------------------------------------------- +// lidlQtNeedsElementLoop — the predicate that keeps the widened table from +// destroying data. +// +// A name it answers true for must NEVER reach logos::qt::toWire / +// fromWire (or QVariant::fromValue / qvariant_cast) as a whole value: +// qvariantToNlohmann matches a closed userType() set, so QList +// serialises to null, and qvariant_cast back yields an EMPTY list. Silently, +// both directions. So a false negative here is a silently dropped payload. +// --------------------------------------------------------------------------- + +TEST(LidlQtNeedsElementLoop, TrueForEveryTypedContainerAndOptional) +{ + EXPECT_TRUE(lidlQtNeedsElementLoop(arr(prim("uint")))); + EXPECT_TRUE(lidlQtNeedsElementLoop(arr(prim("bstr")))); + EXPECT_TRUE(lidlQtNeedsElementLoop(arr(named("Point")))); + EXPECT_TRUE(lidlQtNeedsElementLoop(map(prim("uint")))); + EXPECT_TRUE(lidlQtNeedsElementLoop(map(named("Point")))); + EXPECT_TRUE(lidlQtNeedsElementLoop(optOf(prim("tstr")))); + EXPECT_TRUE(lidlQtNeedsElementLoop(arr(arr(prim("uint"))))); + EXPECT_TRUE(lidlQtNeedsElementLoop(optOf(arr(prim("uint"))))); +} + +// The QVariant-native spellings cross whole and always did. Emitting a loop for +// them would be a gratuitous change to shipped output. +TEST(LidlQtNeedsElementLoop, FalseForEverythingQVariantNative) +{ + EXPECT_FALSE(lidlQtNeedsElementLoop(prim("tstr"))); + EXPECT_FALSE(lidlQtNeedsElementLoop(prim("uint"))); + EXPECT_FALSE(lidlQtNeedsElementLoop(prim("bstr"))); + EXPECT_FALSE(lidlQtNeedsElementLoop(prim("any"))); + EXPECT_FALSE(lidlQtNeedsElementLoop(prim("result"))); + EXPECT_FALSE(lidlQtNeedsElementLoop(named("Point"))); // a record SCALAR + EXPECT_FALSE(lidlQtNeedsElementLoop(arr(prim("tstr")))); // QStringList + EXPECT_FALSE(lidlQtNeedsElementLoop(arr(prim("any")))); // QVariantList + EXPECT_FALSE(lidlQtNeedsElementLoop(map(prim("any")))); // QVariantMap + EXPECT_FALSE(lidlQtNeedsElementLoop(optOf(prim("any")))); // QVariant +} + +// A record SCALAR is false above because this predicate answers only for the +// TYPE TABLE's spellings; the emitters add the record shapes themselves (a +// struct has no metatype either). Stated so the false above is not read as +// "a record may cross whole". +TEST(LidlQtNeedsElementLoop, RecordScalarIsTheEmittersJobNotThisPredicates) +{ + EXPECT_FALSE(lidlQtNeedsElementLoop(named("Point"))); + EXPECT_EQ(lidlTypeToQt(named("Point")), "Point"); +} + +// --------------------------------------------------------------------------- +// lidlTypeToLidlText — the CONTRACT spelling published in getMethods() +// +// It is a copy of logos-lidl's serializeTypeExpr, which is file-local to that +// repo's serializer.cpp with no public printer to delegate to. So the pairing +// is asserted: each shape is round-tripped through lidl::serialize() and the +// type text is read back out of the emitted `.lidl`. If logos-lidl ever changes +// how it prints a type, these fail rather than the two drifting in silence. +// --------------------------------------------------------------------------- + +namespace { + +// The type text logos-lidl itself writes for `method m(p: )`. +QString serializerSpelling(const TypeExpr& te) +{ + ModuleDecl m; + m.name = "probe"; + MethodDecl md; + md.name = "m"; + ParamDecl p; + p.name = "p"; + p.type = te; + md.params.push_back(p); + md.returnType = TypeExpr{ TypeExpr::Primitive, "bool", {} }; + m.methods.push_back(md); + + const QString text = QString::fromStdString(lidl::serialize(m)); + const int open = text.indexOf("method m(p: "); + if (open < 0) return QStringLiteral(""); + const int start = open + int(strlen("method m(p: ")); + const int close = text.indexOf(')', start); + return text.mid(start, close - start); +} + +} // namespace + +TEST(LidlTypeToLidlText, MatchesTheSerializerForEveryShape) +{ + const TypeExpr shapes[] = { + prim("tstr"), prim("uint"), prim("int"), prim("bstr"), prim("bool"), + prim("float64"), prim("any"), named("Point"), + arr(prim("uint")), arr(prim("tstr")), arr(named("Point")), + map(prim("uint")), map(prim("any")), map(named("Point")), + optOf(prim("tstr")), optOf(named("Point")), + arr(arr(prim("uint"))), map(arr(prim("uint"))), arr(map(prim("uint"))), + arr(optOf(prim("tstr"))), optOf(arr(prim("uint"))), + }; + for (const TypeExpr& te : shapes) { + const QString mine = lidlTypeToLidlText(te); + EXPECT_EQ(mine, serializerSpelling(te)) + << "lidlTypeToLidlText has drifted from lidl::serialize for this shape"; + } +} + +// The spellings themselves, so a reader can see what getMethods() now publishes +// without running the serializer in their head. +TEST(LidlTypeToLidlText, PublishesTheContractVocabulary) +{ + EXPECT_EQ(lidlTypeToLidlText(prim("tstr")), "tstr"); + EXPECT_EQ(lidlTypeToLidlText(prim("uint")), "uint"); + EXPECT_EQ(lidlTypeToLidlText(arr(prim("uint"))), "[uint]"); + EXPECT_EQ(lidlTypeToLidlText(arr(named("Point"))), "[Point]"); + EXPECT_EQ(lidlTypeToLidlText(map(prim("uint"))), "{tstr: uint}"); + EXPECT_EQ(lidlTypeToLidlText(optOf(prim("tstr"))), "? tstr"); + EXPECT_EQ(lidlTypeToLidlText(arr(arr(prim("uint")))), "[[uint]]"); } diff --git a/tests/generator/test_map_param_type.cpp b/tests/generator/test_map_param_type.cpp index b3c7705..b85a4ee 100644 --- a/tests/generator/test_map_param_type.cpp +++ b/tests/generator/test_map_param_type.cpp @@ -46,3 +46,61 @@ TEST(MapParamType, SixtyFourBitIntegersAreKnown) EXPECT_EQ(mapParamType("SomeRecord"), "QVariant"); } + +// --------------------------------------------------------------------------- +// The widened-Qt-spelling FOLD +// +// lidlTypeToQt now answers `[uint]` with QList, `{tstr: uint}` with +// QMap and `?tstr` with std::optional. This +// emitter is keyed on flat type NAMES and cannot encode any of them: doing it +// correctly needs an element loop per level, and it has no tree to derive the +// levels from (lidl_to_json flattens the contract to strings before it gets +// here, because the same emitter also serves the metaobject-introspection path). +// +// So they are folded back to the name this emitter already produced, and BOTH +// surfaces it feeds — the legacy Qt consumer and the Qt-free lp one, whose +// table is DERIVED from this one via mapParamTypeStd — stay byte-for-byte what +// they were. +// +// This is a freeze, and these tests are what makes it one. A future change that +// lets a widened spelling through would otherwise be invisible until a +// QList reached QVariant::fromValue at runtime, where +// qvariantToNlohmann answers null and nothing warns. +// --------------------------------------------------------------------------- + +TEST(WidenedSpellingFold, TypedArraysFoldToQVariantList) +{ + EXPECT_EQ(mapParamType("QList"), "QVariantList"); + EXPECT_EQ(mapParamType("QList"), "QVariantList"); + EXPECT_EQ(mapParamType("QList"), "QVariantList"); + EXPECT_EQ(mapParamType("QList>"), "QVariantList"); + EXPECT_EQ(mapReturnType("QList"), "QVariantList"); + EXPECT_EQ(mapReturnType("const QList&"), "QVariantList"); +} + +TEST(WidenedSpellingFold, TypedMapsFoldToQVariantMap) +{ + EXPECT_EQ(mapParamType("QMap"), "QVariantMap"); + EXPECT_EQ(mapParamType("QMap>"), "QVariantMap"); + EXPECT_EQ(mapReturnType("QMap"), "QVariantMap"); +} + +TEST(WidenedSpellingFold, OptionalsFoldToQVariant) +{ + EXPECT_EQ(mapParamType("std::optional"), "QVariant"); + EXPECT_EQ(mapParamType("std::optional>"), "QVariant"); + EXPECT_EQ(mapReturnType("std::optional"), "QVariant"); +} + +// The fold must not swallow the names it is meant to leave alone: QStringList +// is not a `QList<...>` spelling, and a record name reaches this emitter only +// after recordCppType has declined it. +TEST(WidenedSpellingFold, LeavesTheUnwidenedNamesAlone) +{ + EXPECT_EQ(mapParamType("QStringList"), "QStringList"); + EXPECT_EQ(mapReturnType("QStringList"), "QStringList"); + EXPECT_EQ(mapParamType("QVariantList"), "QVariantList"); + EXPECT_EQ(mapParamType("QVariantMap"), "QVariantMap"); + EXPECT_EQ(mapParamType("qulonglong"), "qulonglong"); + EXPECT_EQ(mapReturnType("LogosResult"), "LogosResult"); +}