From fdfa3f72b86a42be7b2f9a67c773de4eec663fb4 Mon Sep 17 00:00:00 2001 From: Dario Gabriel Lipicar Date: Sat, 22 Aug 2026 03:20:52 -0300 Subject: [PATCH] fix(generator): the consumer wrapper comes from the contract, not from getMethods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- README.md | 16 ++ cpp-generator/docs/project.md | 16 +- cpp-generator/docs/spec.md | 2 +- cpp-generator/plugin_introspect.cpp | 274 +++++++++++++++++++++++++--- nix/tests-generator-cli.nix | 37 ++++ 5 files changed, 311 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 560dd53..30e7bfa 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 555086d..49814d1 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/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/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" ''