From acea0d24e2bcf9a11973aac15df4dc12913f9b02 Mon Sep 17 00:00:00 2001 From: Dario Lipicar Date: Sat, 22 Aug 2026 18:17:44 -0300 Subject: [PATCH] feat(codegen): a lossless Qt type mapping, and LIDL types in getMethods (#149) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(codegen): a lossless Qt type mapping — typed containers and optionals `lidlTypeToQt` answered four different LIDL types with one Qt name. `[uint]`, `[bstr]`, `[[uint]]` and `[any]` were all QVariantList; `{tstr: uint}` and `{tstr: any}` were both QVariantMap; every `?T` was a bare QVariant. A Qt consumer therefore lost, on the SAME contract, types that the std consumer next door kept — it could not tell `?tstr` from `?uint`, and got no compile-time check on any element. The table is now recursive: [T] QList ([tstr] stays QStringList) {tstr: V} QMap ?T std::optional (through optionalValueType, so ??T stays two-state) any QVariant — KEPT, deliberately `any` is the one row that must not widen: QVariant is the only Qt type that holds bytes AND an exact uint64 AND arbitrary nesting at once, so every narrower spelling would lose what it was chosen to carry. The rule is applied at the LEAF, so anything whose element type bottoms out at `any` keeps the QVariant-family spelling at every depth — `[any]` is QVariantList, `[[any]]` still is, `{tstr: [any]}` is QVariantMap, `?any` is QVariant. THE TRAP, and why this is not just a rename. A widened name must never reach QVariant::fromValue / qvariant_cast / logos::qt::toWire as a WHOLE value. logos-protocol's qvariantToNlohmann matches a CLOSED userType() set: QList is in none of it, so it serialises to JSON null. The decode fails just as quietly — qvariant_cast> of a QVariantList yields an EMPTY list. Neither direction warns. So every widened slot is encoded and decoded by a generator-emitted ELEMENT LOOP, the shape the record cases already used, and `lidlQtNeedsElementLoop` is the single predicate that decides which slots need one. The emitted loops take their source as a lambda PARAMETER, not a body-local binding. They nest (`[[uint]]`), every level wants the same short names, and a local — or a range-for over a name the loop itself declares — is then self-referential: it compiles and reads uninitialised memory. Measured: three round-trip tests died on SIGTRAP before the argument form. THE STRING-KEYED EMITTER IS FROZEN, ON PURPOSE. generator_lib is keyed on flat type NAMES (lidl_to_json flattens the contract before it gets there, because that emitter also serves the metaobject-introspection path), so it cannot derive the levels an element loop needs without parsing C++ type names back into a tree. Every widened spelling is folded back to the name it produced before (legacyQtBase), which keeps BOTH surfaces it feeds byte-for-byte unchanged: the legacy Qt consumer, and the Qt-free lp one whose table is DERIVED from it through mapParamTypeStd. Verified by generating a 28-method contract through both before and after: the diff is empty. The widened types are spent in the TypeExpr-driven emitters instead (lidl_gen_client.cpp here, lidl_gen_qt_consumer.cpp in logos-qt-sdk). Also here, because both are consequences of the table becoming recursive: * lidlTypeToQt gained a record-name HOOK. A wrapper nests its record structs in the wrapper class, so a type written outside that scope must qualify them — and the emitters used to do that by matching the three shapes that could mention a record on the finished string. `?Point` and `QList>` are now spellable, so the qualification happens during the walk, at the one place that knows a name is a record. * lidlTypeToLidlText — the LIDL contract spelling of a type. Unused here; the commit that follows puts getMethods() on it. Co-Authored-By: Claude Opus 5 * feat(cdylib): getMethods publishes the LIDL contract vocabulary, not Qt names A module's published metadata — `returnType`, `parameters[].type`, `signature` — answered in Qt type names. Two things wrong with that, and the second is the one that matters: * a cdylib module is Qt-FREE. It described itself in the types of a language it does not use, to readers (`lm`, logoscore's method listing, basecamp's module inspector) that are showing a human what the module offers. * it was LOSSY. `[uint]`, `[bstr]` and `[any]` are three different LIDL types and all three published as the single word QVariantList, so the listing could not be read back as a contract. That is now `[uint]`, `[bstr]`, `[any]`; `{tstr: uint}`; `? tstr`; and a record publishes its declared name. WHY THIS IS SAFE — checked, not assumed. The historical objection is recorded in the function this replaces: these strings are read as METATYPES, and emitting a record's struct name here once made the host SIGSEGV. Nothing in the current runtime does that. logos-plugin-qt's QtProviderObject dispatches on `method.returnMetaType()` / `parameterMetaType(i)` — the QMetaObject, never this JSON — and every remaining reader treats these fields as opaque text: logos-module's `lm` prints them, logoscore's output.cpp prints them, basecamp's CoreModuleManager forwards the JSON to QML, and the plain wire's json_mapping only round-trips them. Nothing anywhere builds a QMetaObject from this metadata. The spelling comes from lidlTypeToLidlText, which mirrors logos-lidl's serializeTypeExpr. It is a COPY, because that function is file-local to logos-lidl's serializer.cpp and the public headers expose no type printer — so instead of hoping, the pairing is ASSERTED: the test round-trips each shape through `lidl::serialize` and reads the type text back out of the emitted `.lidl`. When logos-lidl exports a printer, delete the copy and call it. Not fixed by this, and not attempted: the Rust SDK's provider generator has its own `qt_type_name` writing the same JSON, so the two languages now disagree about how a module describes itself. That is a cross-repo change. Co-Authored-By: Claude Opus 5 * test(doctests): the generator round-trip pins the lossless Qt spellings `cpp-sdk-generator-roundtrip.test.yaml` is a CI gate (.github/workflows/doctests.yml), and two of its `expect_contains` were pinned to the type names the Qt consumer produced BEFORE the lossless mapping: QStringList labels(const QVariantList& ids QVariant nearest(const Point& p, QVariant limit The generator now emits `QList` and `std::optional` / `std::optional` for those slots, so both assertions failed. The `nearest` step's `run` grep was pinned the same way (`QVariant nearest`), so the line it was supposed to assert on was not even in the output being searched. Verified by running the spec's own steps against the generator built from this commit: 10 run-steps, 0 failures. The `[uint]` -> QList and `?T` -> std::optional lines were read out of the real `consumer/sensor_module_api.h` and `geometry/geometry_module_api.h`, not written from the mapping table. Prose too, in three places that described the old table: the Flow-3 type mapping ("other arrays -> QVariantList"), the composite-types intro ("optionals ... stay QVariantMap / QVariant"), and the composite-signature step. They now say what the mapping actually is — one LIDL type, one C++ spelling, with `any` the single deliberate exception — and `nearest` is called out as the one signature carrying both halves of the optional mapping. `doctests/outputs/cpp-sdk-generator-roundtrip.md` carries the same prose corrections. That tree is hand-pinned and CI never diffs it, which is exactly why it must be corrected by hand. Co-Authored-By: Claude Opus 5 * fix(generator): the consumer wrapper comes from the contract, not from getMethods `logos-cpp-generator --module-only` — the invocation logos-plugin-qt's generate-module-headers.sh makes for every module's lp wrapper — built that wrapper's whole type surface out of the plugin's PUBLISHED `getMethods()` metadata. It now builds it out of the module's `.lidl` contract, the file the same invocation already passes as `--events-from`. WHY THIS IS A DEFECT AND NOT A PREFERENCE. generator_lib is keyed on flat type NAMES, and mapParamType / mapReturnType fall back to QVariant for a name they do not recognise (generator_lib.cpp:142 and :153). So the wrapper's types depend on the VOCABULARY a module happens to publish its metadata in, and a vocabulary this emitter has no row for degrades to QVariant — LogosMap on the lp surface — with no diagnostic at any layer. It is a machine reader of a listing that every other consumer treats as human-facing text, and it fails silently. It was measured, not theorised. 621772a made the cdylib backend publish the LIDL contract vocabulary (`tstr`, `[uint]`, `result`, `? tstr`) in place of Qt type names, because that listing is what `lm`, logoscore and basecamp show a human and Qt names are the wrong answer for a Qt-free module. Every `interface: "universal"` module's lp wrapper collapsed: logos-test-modules' `checks.unit-tests-new-api` went PASS -> FAIL, and the compiler said exactly why — error: no viable conversion from 'LogosMap' to 'StdLogosResult' StdLogosResult r = modules().test_basic_module.resultWithMap(); `result` is not a name mapReturnType knows, so it became QVariant, so it became LogosMap. Bisected to exactly 621772a (5ffd90b passes, dd52d9d fails). THE FIX IS TO STOP READING THAT VOCABULARY, not to learn a second one. `int` means a 32-bit Qt int in one table and a 64-bit LIDL integer in the other, and the reader cannot tell from the string which table it is holding — a merged table would silently mistype every integer on every module. The contract has no such ambiguity: it is a TypeExpr tree, and lidl_to_json is the single place it is flattened. Taking methods from it makes this path emit the same wrapper as `--general-only --dep =.lidl`, which is what buildHeaders.nix already runs under cross-compilation and for the entire Qt surface. Contract-first, on every platform, for every surface. WHAT CHANGED, exactly: * loadEventsFromLidl -> loadContractFromLidl. It already parsed the whole contract and threw the methods away; it now returns them, after the same lidlCheckRecords + lidlInjectIdentity + noteOptionalPositionalSlots that main.cpp's --dep path applies. Identity is injected rather than read, matching the provider side (main.cpp's --backend cdylib), so the two cannot disagree about name() / version(). * A sidecar that is NAMED BUT MISSING is now refused (exit 2), and an unreadable or malformed one is fatal (exit 4). Both used to be shrugged off — which shipped a wrapper with no typed events, and would now ship one with no typed methods, in the silently-empty shape generate-module-headers.sh exists to refuse. * The plugin is STILL LOADED. That load is the dlopen check this path performs (exit 3 on an SDK/ABI skew) and it is unchanged; what the plugin says about itself is now compared against the contract instead of believed, and a divergence — a stale sidecar — is reported by name on stderr. Only `isInvokable` entries are compared: a cdylib publishes its events into the same array, tagged `"type": "event"`, and both emitters already skip those. * A module with NO contract keeps introspection — a handcrafted Qt plugin's QMetaObject is still the only description of its API that exists, and Qt type names are the right vocabulary for it — but a listing spelled in the LIDL vocabulary with no contract to go with it is now REFUSED (exit 7) instead of silently producing the untyped wrapper. That combination is only reachable by hand: buildHeaders.nix always passes the flag when the sidecar exists, and it is the shape the developer guide used to suggest. The two vocabularies are not distinguishable in general, which is the whole reason this emitter must read only one — but they do not have to be: the words they share (`int`, `bool`) are all in the known table and never reach the fallback, so the check keys on the LIDL half Qt has no word for at all (`tstr`, `bstr`, `uint`, `float64`, `result`, `any`, and anything starting `[`, `{` or `?`). No Qt type is spelled that way, so it cannot false-fire; a false negative is just the old behaviour. THE ENUMERATION, because two previous ones missed this reader. Searching for who greps `returnType` is what missed it; the question is what the data FLOWS INTO. Every consumer of a published getMethods array in the workspace: MACHINE (one, and it is this one) logos-cpp-sdk cpp-generator/plugin_introspect.cpp, reached only through logos-plugin-qt's generate-module-headers.sh / buildHeaders.nix. HUMAN-READABLE OR OPAQUE PASSTHROUGH (all of them) logos-module's `lm` (prints; --json re-emits verbatim), logoscore-cli's client/output.cpp (prints) and core_service_dispatch.cpp (forwards), logos-logoscore-tui (formats one line per method), logos-module-viewer (reads the QMetaObject directly, not this JSON), basecamp's CoreModuleManager / MainUIBackend (hands the JSON string to QML), logos-protocol's json_mapping.cpp and qvariant_rpc_value.cpp (round-trip the strings unread). PRODUCERS, for completeness: lidl_gen_cdylib.cpp (LIDL vocabulary), logos-plugin-qt's QtProviderObject (Qt names, from the QMetaObject) and lidl_gen_cdylib_glue.cpp (forwards the cdylib's), logos-rust-sdk's rustgen_provider.rs (still Qt names — the two languages disagree, as 621772a noted), and logos-protocol's ModuleProxy, which appends derived name()/version() entries spelled `QString`. None of that reaches a type decision any more, which is the point of the change. Build-system paths checked and clear: ` --module-only` is invoked from exactly one place in the workspace (generate-module-headers.sh:60); LogosModule.cmake, buildPlugin.nix and mkLogosModuleTests.nix all use `--general-only`, which is contract-driven already; the doctests' `--lidl --module-only` is a different mode entirely. VERIFIED. `nix build path:./repos/logos-test-modules#checks.aarch64-darwin.unit-tests-new-api` with this SDK overridden in (plus the logos-lidl overrides the branch needs at the qt-sdk and plugin-qt nodes) — 32 passed, 0 failed. The same command against this branch's HEAD fails to compile, as above. The build log shows the path taken, per module: Detected new-API plugin (LogosProviderPlugin), using getMethods() — 43 methods Using the module's LIDL contract for the method surface — 41 methods (the plugin's published listing is a description, not a type source) The refusal, measured by hand against a real LIDL-publishing plugin (test_basic_module, built from this branch) because no check exercises a hand-run invocation: no --events-from -> exit 7, nothing written, the message above naming 8 offending slots with --events-from -> exit 0, 41 typed methods, 69 `std::string` in the emitted lp header a pre-621772a build of the SAME module (Qt-name listing), no --events-from -> exit 0, still generates, still typed — the refusal does not fire on the vocabulary this emitter can read nix/tests-generator-cli.nix gains the two CLI-surface cases this adds: a `--events-from` naming a file that does not exist is refused with that sentence, and — the control that makes it mean something — the same command with a READABLE contract gets past the flag and fails on the plugin instead. No plugin is needed for either: the contract is loaded before the plugin is opened. logos-cpp-sdk's own checks (tests, generator-cli, module-impl-abi): 334 of 334. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- README.md | 16 + cpp-generator/docs/project.md | 16 +- cpp-generator/docs/spec.md | 2 +- .../experimental/lidl_emit_common.cpp | 146 ++++++++-- cpp-generator/experimental/lidl_emit_common.h | 58 ++++ .../experimental/lidl_gen_cdylib.cpp | 59 ++-- .../experimental/lidl_gen_client.cpp | 211 +++++++++++--- cpp-generator/generator_lib.cpp | 42 ++- cpp-generator/lidl_to_json.cpp | 28 +- cpp-generator/plugin_introspect.cpp | 274 ++++++++++++++++-- .../cpp-sdk-generator-roundtrip.test.yaml | 39 ++- .../outputs/cpp-sdk-generator-roundtrip.md | 34 ++- nix/tests-generator-cli.nix | 37 +++ tests/experimental/test_lidl_gen_cdylib.cpp | 83 ++++++ tests/experimental/test_lidl_gen_client.cpp | 21 +- tests/experimental/test_lidl_type_mapping.cpp | 250 +++++++++++++++- tests/generator/test_map_param_type.cpp | 58 ++++ 17 files changed, 1187 insertions(+), 187 deletions(-) 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"); +}