diff --git a/cpp-generator/docs/project.md b/cpp-generator/docs/project.md index d87d8c3..9299a63 100644 --- a/cpp-generator/docs/project.md +++ b/cpp-generator/docs/project.md @@ -72,6 +72,41 @@ Bridges the existing Qt-flavored backends onto logos-lidl's std AST so they comp - `lidlIsStdConvertible(TypeExpr)` — whether a type has a pure-C++ (Qt-free) representation - `lidlToPascalCase(name)` — converts `snake_case` to `PascalCase` +### Optionality + +`?T` is **two-state**: a value of `T`, or empty. Never three-state — "one LIDL type ↔ one +type per language" leaves nowhere for a third state, because every target has exactly one +empty inhabitant. + +**Two spellings, one meaning.** A record field may be written `? name: T` (the flag) or +`name: ?T` (the type kind); the spec binds them to the same declaration, so they MUST emit +identical code. Backends never answer this themselves — logos-lidl's `fieldIsOptional(f)` / +`fieldValueType(f)` (re-exported by `lidl_compat.h`) are the one place the two are +reconciled. **Reading `f.optional` or `f.type.kind == Optional` on its own is a bug.** + +**Wire rule.** Absent and explicit null are the *same* state on decode and *different* on +encode: + +| | empty is spelled | +|---|---| +| decode, optional slot | absent **or** null → empty | +| decode, required slot | absent and null are both still errors | +| encode, **named** slot (a record field) | the key is **omitted** | +| encode, **positional** slot (argument, return, event param) | `null` — there is no key to omit, and arity must never change | + +A round trip therefore **canonicalises**: a peer that sent `"f": null` gets the key back +omitted. A present-but-wrong-typed value is still an error — optional widens the domain by +exactly one inhabitant, it does not switch type checking off. + +Per surface: + +| Surface | `?T` | Notes | +|---|---|---| +| cdylib / std (`lidlTypeToStd`, `lidl_gen_cdylib`) | `std::optional` | encoded by logos-protocol's `Codec>`; key omission is the record emitter's job (a codec never sees the slot) | +| `?any` / `?{tstr: any}` / `?[any]` | `LogosMap` / `LogosList` | collapses: `nlohmann::json` already carries `null`, so wrapping it would make the slot three-state | +| Qt (`lidlTypeToQt`) | `QVariant` | two-state (an invalid QVariant is Qt's empty inhabitant) but **untyped** — see Known Limitations | +| header-first (`impl_header_parser`) | `std::optional` ↔ `?T` | `std::optional>` has no LIDL type; it collapses to `?T` and is reported on stderr | + ### Client stubs (`lidl_gen_client.h/cpp`) - `lidlMakeHeader(ModuleDecl)` / `lidlMakeSource(ModuleDecl)` — typed `` client wrapper; each method (and its `…Async` twin) carries a Doxygen `///` comment generated from the method's `description` @@ -143,6 +178,7 @@ Flag plumbing: - The literal `logos_events:` token (defined in `logos_module_context.h` as `#define logos_events public`) opens an events section; bare prototypes inside become `EventDecl{name, params, description}` entries appended to `ModuleDecl.events` (the `description` is the doc comment immediately above the declaration, captured via `joinDocLines` exactly as for methods) - Skips: constructors, destructors, typedefs, using, friend, enum, struct, `std::function` declarations - Recognizes `LogosMap` and `LogosList` return types (nlohmann::json aliases) and sets `MethodDecl.jsonReturn = true` +- Recognizes `std::optional` → `?T` (see Optionality). Anything it does *not* recognize still falls back to the opaque `any`, silently — that fallback is why an optional was unexpressible header-first until it was named explicitly - Template-aware parameter splitting (handles `std::vector` correctly) ## CLI Usage @@ -221,9 +257,10 @@ The frontend tests (lexer/parser/validator/serializer) moved to the **logos-lidl | Test file | What it tests | |-----------|---------------| -| `test_lidl_type_mapping.cpp` | `lidlTypeToQt`, `lidlTypeToStd`, `lidlIsStdConvertible`, `lidlToPascalCase` | -| `test_lidl_gen_client.cpp` | Client stub generation: sync/async methods, events, metadata JSON, edge cases | -| `test_impl_header_parser.cpp` | Header parsing: type mapping, access specifiers, skipping private/protected, error cases | +| `test_lidl_type_mapping.cpp` | `lidlTypeToQt`, `lidlTypeToStd`, `lidlIsStdConvertible`, `lidlToPascalCase`, optionality on both surfaces | +| `test_lidl_gen_client.cpp` | Client stub generation: sync/async methods, events, metadata JSON, edge cases, both optional spellings agreeing | +| `test_lidl_gen_cdylib.cpp` | cdylib eligibility + emission: bytes at depth, records, typed maps, optionality (key omission, arity, `?any` collapse) | +| `test_impl_header_parser.cpp` | Header parsing: type mapping, access specifiers, skipping private/protected, error cases, `std::optional` | (The lexer/parser/AST/serializer/validator round-trip + description tests live in logos-lidl's own `tests/test_lidl.cpp`.) @@ -233,6 +270,7 @@ Fixture files in `tests/experimental/fixtures/`: - `complex_impl.h` — module with multiple access specifier sections - `empty_class_impl.h` — class with no public methods - `empty_metadata.json` — minimal metadata +- `optional_impl.h` / `optional_metadata.json` — `std::optional` header-first, incl. an optional over a declared record ## Known Limitations @@ -246,3 +284,18 @@ Fixture files in `tests/experimental/fixtures/`: - LIDL does not support generic/parameterized types or inheritance - `--from-header` emits the **cdylib** backend here (the `qt` glue backend moved to logos-qt-generator); the **Rust** backend lives in logos-rust-sdk's `lidl-gen`, generating over logos-lidl's C ABI - Client stub generation (`lidlMakeHeader`/`lidlMakeSource`) is only available from LIDL files, not from `--from-header` +- **Optionality is untyped on the Qt/Lp consumer surface.** The consumer wrappers real + modules get come from `legacy/main.cpp` → `generateInterfaceWrappers` → `generator_lib`, + and the AST is flattened to a single Qt **type-name string** per slot at + `moduleMethodsToJson` / `moduleRecordsToJson` / `moduleEventsToJson` — a boundary that + optionality (like nesting, map key types and descriptions) cannot cross. `?T` therefore + arrives as `QVariant`: the right *shape* (an invalid QVariant is Qt's empty inhabitant, + and the wire's `null` becomes exactly that) with no *type*, so a consumer gets no + compile-time check and cannot tell `?tstr` from `?uint` or recover a `?Record`'s struct. + Carrying it further means widening that JSON surface with a per-slot optional flag and + teaching both the Qt and Lp emitters to honour it. Until then the generator prints a + `Note:` naming every flattened slot, so an affected build is never silent. +- `lidlRecordCollidesWithBytesTag` reads *through* an optional (via `fieldValueType`), so a + single-`_bytes`-field record is refused under both spellings. It used to read `f.type`, + which refused `? _bytes: tstr` and let `_bytes: ?tstr` through — the same declaration, + two answers. `?bstr` is unaffected either way: the tag lives in the value, not the slot. diff --git a/cpp-generator/experimental/impl_header_parser.cpp b/cpp-generator/experimental/impl_header_parser.cpp index 5c43639..d5210e3 100644 --- a/cpp-generator/experimental/impl_header_parser.cpp +++ b/cpp-generator/experimental/impl_header_parser.cpp @@ -46,6 +46,11 @@ static QString stripDeclarationSpecifiers(QString string) // `any` it always was. static QSet g_recordNames; +// C++ spellings seen that have NO LIDL type, and were mapped onto the nearest +// one that does. Collected here because cppTypeToLidl has no diagnostic channel +// (same reason g_recordNames is file-static); parseImplHeader drains it. +static QStringList g_unmappableSpellings; + static TypeExpr cppTypeToLidl(const QString& raw) { // Normalize: strip const, &, leading/trailing whitespace @@ -140,6 +145,38 @@ static TypeExpr cppTypeToLidl(const QString& raw) return { TypeExpr::Map, "", { {TypeExpr::Primitive, "tstr", {}}, val } }; } + // std::optional -> ?T. Absent before, and the failure was silent: the + // fallback at the bottom of this function maps ANY unrecognised spelling to + // the opaque `any`, so a header-first C++ provider could not express + // optionality at all — it declared `std::optional` and + // published a contract saying `any`, with no diagnostic. + // + // The derived contract uses the type-kind spelling (`name: ?T`). C++ has + // only one spelling, LIDL has two, and they are bound to the same meaning — + // so which one is emitted is a serialization choice, not a semantic one. + static QRegularExpression optRe("^std::optional\\s*<\\s*(.+)\\s*>$"); + QRegularExpressionMatch om = optRe.match(t); + if (om.hasMatch()) { + TypeExpr inner = cppTypeToLidl(om.captured(1).trimmed()); + // std::optional> has NO LIDL type. + // + // `?T` is two-state, and optionality is idempotent under that rule — so + // the nearest contract type is plain `?T`, and that is what gets + // published. But the author's C++ has THREE states (nullopt / an engaged + // outer holding nullopt / a value), and the wire has two: accepting the + // declaration as-is would make `optional(nullopt)` and `nullopt` encode + // to the same null and decode back as one of them, silently. So it maps + // down, the generated codec is written for std::optional, and the + // author's own declaration stops compiling against it — deliberately. + // Say why here, where the reason is known, rather than leaving a + // conversion error in generated code the author never wrote. + if (inner.kind == TypeExpr::Optional) { + g_unmappableSpellings << t; + return inner; + } + return { TypeExpr::Optional, "", { inner } }; + } + // A record the header declared. Checked LAST so it can never shadow a // builtin spelling, and gated on the declared set so an unknown type keeps // the historical `any` fallback rather than naming a struct nobody emits. @@ -376,6 +413,11 @@ ImplParseResult parseImplHeader(const QString& headerPath, { ImplParseResult result; + // Both file-statics are per-parse state: one process generates for more than + // one module. Cleared here rather than next to their first use because the + // metadata's event params are typed before the header is even read. + g_unmappableSpellings.clear(); + // --- Read metadata.json --- { QFile mf(metadataPath); @@ -703,6 +745,16 @@ done: // contract types. keepOnlyReferencedRecords(result.module); + if (!g_unmappableSpellings.isEmpty()) { + g_unmappableSpellings.removeDuplicates(); + err << "Warning: " << headerPath << ": " << g_unmappableSpellings.join(", ") + << " has no LIDL type. `?T` is TWO-state — a value or empty — so a " + "nested optional cannot denote a third state; the contract " + "publishes the collapsed `?T`, and the generated codec is " + "written for std::optional. Declare it that way, or the " + "generated code will not compile against this header.\n"; + } + if (result.module.methods.empty()) { err << "Warning: no public methods found in class " << className << " in " << headerPath << "\n"; diff --git a/cpp-generator/experimental/lidl_compat.h b/cpp-generator/experimental/lidl_compat.h index b91c523..9e0372c 100644 --- a/cpp-generator/experimental/lidl_compat.h +++ b/cpp-generator/experimental/lidl_compat.h @@ -28,6 +28,23 @@ using lidl::EventDecl; using lidl::TypeDecl; using lidl::ModuleDecl; +// Optionality accessors, from the SAME header — never re-derived here. +// +// `?T` has two equivalent spellings for a record field: the flag (`? name: T`, +// which leaves `FieldDecl::type` as T and sets `FieldDecl::optional`) and the +// type kind (`name: ?T`, which leaves the flag false and makes the type an +// Optional). logos-lidl's docs/spec.md binds them to the same meaning, so they +// MUST emit identical code — and the only way that holds is if no backend +// answers the question itself. Reading `f.optional` alone is a bug; reading +// `f.type.kind == Optional` alone is the same bug from the other side. +// fieldIsOptional() / fieldValueType() are the answer. +using lidl::typeIsOptional; +using lidl::optionalValueType; +using lidl::fieldIsOptional; +using lidl::fieldValueType; +using lidl::paramIsOptional; +using lidl::paramValueType; + // std::string -> QString, and let QTextStream accept std::string directly so // emission of AST string fields (`s << md.name`) keeps compiling unchanged. inline QString qs(const std::string& s) { return QString::fromStdString(s); } @@ -62,12 +79,18 @@ inline lidl::ValidationResult lidlValidate(const ModuleDecl& module) // to name a map key `_bytes` — but a generator can at least refuse to emit the // one shape that is guaranteed to misdecode, instead of leaving it to be // discovered at runtime. +// Optionality does not rescue it: a PRESENT `? _bytes: tstr` still encodes to +// {"_bytes": "..."}, which is the ambiguous shape. So the check reads through +// the optional — via fieldValueType, not f.type — and refuses both spellings. +// Reading f.type here refused `? _bytes: tstr` (whose type stays Primitive +// tstr) while letting `_bytes: ?tstr` straight through: one declaration, two +// answers, which is the exact drift the accessors exist to prevent. inline bool lidlRecordCollidesWithBytesTag(const TypeDecl& t) { - return t.fields.size() == 1 - && t.fields[0].name == "_bytes" - && t.fields[0].type.kind == TypeExpr::Primitive - && t.fields[0].type.name == "tstr"; + if (t.fields.size() != 1 || t.fields[0].name != "_bytes") + return false; + const TypeExpr& vt = fieldValueType(t.fields[0]); + return vt.kind == TypeExpr::Primitive && vt.name == "tstr"; } // Returns false and fills `error` when any declared record cannot round-trip. diff --git a/cpp-generator/experimental/lidl_emit_common.cpp b/cpp-generator/experimental/lidl_emit_common.cpp index 77aa918..3cc2385 100644 --- a/cpp-generator/experimental/lidl_emit_common.cpp +++ b/cpp-generator/experimental/lidl_emit_common.cpp @@ -54,6 +54,22 @@ QString lidlTypeToQt(const TypeExpr& te) return "QMap"; return "QVariantMap"; case TypeExpr::Optional: + // `?T` on the QT surface, deliberately, and this is the one mapping in + // this table that LOSES the value type. + // + // 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. + // + // 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"; } return "QVariant"; @@ -103,7 +119,21 @@ QString lidlTypeToStd(const TypeExpr& te) return "QVariantList"; } if (te.kind == TypeExpr::Map) return "QVariantMap"; - if (te.kind == TypeExpr::Optional) return "QVariant"; + // `?T` -> std::optional. The std surface HAS an optional, so unlike the + // Qt table above this one keeps the value type. std::nullopt is C++'s single + // empty inhabitant, which is what makes the mapping two-state; the encoder + // that pairs with it is logos-protocol's Codec>. + // + // Recurse through optionalValueType() rather than elements[0]: optionality + // is idempotent under the two-state rule, so `??T` denotes the same two + // states as `?T` and must not become std::optional>. + // A degenerate Optional carrying no element (unreachable from the parser, + // constructible by hand or over the JSON bridge) keeps the opaque fallback + // instead of recursing forever. + if (te.kind == TypeExpr::Optional) { + if (te.elements.empty()) return "QVariant"; + return "std::optional<" + lidlTypeToStd(optionalValueType(te)) + ">"; + } if (te.kind == TypeExpr::Named) return "QVariant"; return "QVariant"; } diff --git a/cpp-generator/experimental/lidl_gen_cdylib.cpp b/cpp-generator/experimental/lidl_gen_cdylib.cpp index 7e00fe3..ae20ca3 100644 --- a/cpp-generator/experimental/lidl_gen_cdylib.cpp +++ b/cpp-generator/experimental/lidl_gen_cdylib.cpp @@ -3,6 +3,7 @@ #include +#include #include #include @@ -49,6 +50,18 @@ bool typeSupported(const TypeExpr& te, bool isReturn, const std::set ?Point` and `-> ?tstr` are the + // real optional returns and stay eligible. + if (te.kind == TypeExpr::Optional) { + if (te.elements.empty()) return false; + return typeSupported(optionalValueType(te), /*isReturn=*/false, recs); + } // Recurse rather than whitelisting element names: that admits [bstr], // [[int]], [Record] and [{tstr: T}] in one rule, and keeps the gate and // the spelling function agreeing about what is expressible. @@ -96,6 +109,31 @@ QString lidlTypeToStdCdylib(const TypeExpr& te, const std::set& rec QString jsonArgToStd(const TypeExpr& te, const QString& expr, const QString& path, const std::set& recs) { + // `?T` — decode is LIBERAL, and only by exactly one inhabitant. + // + // null decodes to empty; anything else is decoded as T by the SAME decoder a + // required T would get, so a present-but-wrong value fails with the same + // message at the same path. Optional widens the domain, it does not switch + // type checking off. + if (te.kind == TypeExpr::Optional && !te.elements.empty()) { + const QString cpp = lidlTypeToStdCdylib(te, recs); + const TypeExpr& vt = optionalValueType(te); + // `?any` collapses onto `any` (see lidlTypeToStdCdylib): untyped JSON + // already carries null, so there is no wrapper to build. + if (!cpp.startsWith("std::optional<")) + return jsonArgToStd(vt, expr, path, recs); + // A scalar `bstr` argument does NOT go through the codec — it gets the + // lenient bytes decode, so a caller may send the tagged form, a plain + // string, a number or a byte array. `?bstr` has to keep that, or the + // identical value would be accepted in a required slot and rejected in + // an optional one. Test for the empty inhabitant here and wrap. + if (vt.kind == TypeExpr::Primitive && vt.name == "bstr") + return "(" + expr + ".is_null() ? " + cpp + "() : " + cpp + "(" + + jsonArgToStd(vt, expr, path, recs) + "))"; + // Everything else names std::optional and lets + // Codec> map null -> nullopt in one expression. + return "logos::fromJson<" + cpp + ">(" + expr + ", \"" + path + "\")"; + } if (te.kind == TypeExpr::Primitive) { if (te.name == "bstr") return "logos::bytesFromJsonLenient(" + expr + ", \"" + path + "\")"; @@ -145,6 +183,20 @@ QString stdReturnToJson(const MethodDecl& md, const QString& var, // compile. QString lidlTypeToStdCdylib(const TypeExpr& te, const std::set& recs) { + // `?T` -> std::optional, EXCEPT over the untyped-JSON aliases. + // + // LogosMap / LogosList are nlohmann::json, and json already has `null` among + // its inhabitants — so std::optional would give `?any` TWO empty + // spellings (nullopt and json(null)) and make it three-state, which is + // exactly what R1 forbids. `?any` therefore collapses onto `any`: same two + // states, one C++ type. (logos-lidl's validator warns on `?any` for the same + // reason, and the warning is about the spelling, not about this mapping.) + if (te.kind == TypeExpr::Optional && !te.elements.empty()) { + const QString inner = lidlTypeToStdCdylib(optionalValueType(te), recs); + if (inner == "LogosMap" || inner == "LogosList") + return inner; + return "std::optional<" + inner + ">"; + } if (te.kind == TypeExpr::Primitive && te.name == "any") return "LogosMap"; // `{tstr: any}` and `[any]` keep their nlohmann aliases: every existing @@ -175,6 +227,50 @@ QString lidlTypeToStdCdylib(const TypeExpr& te, const std::set& rec return lidlTypeToStd(te); } +// The C++ spelling of a RECORD FIELD, honouring both optionality spellings. +// +// `? name: T` and `name: ?T` are the same declaration and must produce +// byte-identical code (logos-lidl docs/spec.md, "Optionality"). That only holds +// because fieldIsOptional()/fieldValueType() reconcile them in the frontend — +// spelling one of the two out here would reintroduce the drift they exist to +// prevent. Never write `f.optional` or `f.type.kind == Optional` in a backend. +QString lidlFieldTypeCdylib(const FieldDecl& f, const std::set& recs) +{ + if (!fieldIsOptional(f)) + return lidlTypeToStdCdylib(f.type, recs); + const QString inner = lidlTypeToStdCdylib(fieldValueType(f), recs); + // Same collapse as lidlTypeToStdCdylib: untyped JSON already has null. + if (inner == "LogosMap" || inner == "LogosList") + return inner; + return "std::optional<" + inner + ">"; +} + +// True when anything in the contract is optional — a record field by either +// spelling, a method parameter or return, or an event parameter. Gates the +// `#include ` in the generated TUs, so a contract that declares no +// optional keeps its output byte-for-byte unchanged. +bool moduleUsesOptional(const ModuleDecl& module) +{ + std::function mentions = [&](const TypeExpr& t) -> bool { + if (t.kind == TypeExpr::Optional) return true; + for (const TypeExpr& e : t.elements) + if (mentions(e)) return true; + return false; + }; + for (const TypeDecl& t : module.types) + for (const FieldDecl& f : t.fields) + if (fieldIsOptional(f) || mentions(f.type)) return true; + for (const MethodDecl& md : module.methods) { + if (mentions(md.returnType)) return true; + for (const ParamDecl& pd : md.params) + if (mentions(pd.type)) return true; + } + for (const EventDecl& ed : module.events) + for (const ParamDecl& pd : ed.params) + if (mentions(pd.type)) return true; + return false; +} + // True when the module declares at least one `bstr` event parameter — the only // reason the events sidecar needs the bytes encoder. Emitting it unconditionally // leaves an unused static function (a -Wunused-function warning) in every module @@ -227,20 +323,45 @@ void emitRecordCodecs(QTextStream& s, const ModuleDecl& module, s << " static nlohmann::json to(const " << name << "& v) {\n"; s << " nlohmann::json out = nlohmann::json::object();\n"; for (const FieldDecl& f : t.fields) { - const QString ft = lidlTypeToStdCdylib(f.type, recs); - s << " out[\"" << qs(f.name) << "\"] = Codec<" << ft << ">::to(v." - << qs(f.name) << ");\n"; + const QString ft = lidlFieldTypeCdylib(f, recs); + const QString fn = qs(f.name); + if (ft.startsWith("std::optional<")) { + // ENCODE: a record field is a NAMED slot, so empty is spelled by + // OMITTING the key — never by writing null. This is the half of + // the rule Codec> deliberately cannot do: a + // codec only ever sees a VALUE, so it emits the positional + // spelling (null) and leaves key omission to the one place that + // knows there IS a key. That place is here. + // + // The round trip is therefore CANONICALISING, not identity: a + // peer that sent `"f": null` gets the key back omitted, and both + // spellings mean the same state. + const QString vt = lidlTypeToStdCdylib(fieldValueType(f), recs); + s << " if (v." << fn << ".has_value())\n"; + s << " out[\"" << fn << "\"] = Codec<" << vt << ">::to(*v." + << fn << ");\n"; + } else { + s << " out[\"" << fn << "\"] = Codec<" << ft << ">::to(v." + << fn << ");\n"; + } } s << " return out;\n }\n"; s << " static " << name << " from(const nlohmann::json& j, const std::string& path) {\n"; s << " if (!j.is_object()) detail::typeError(path, \"object\", j);\n"; s << " " << name << " out;\n"; for (const FieldDecl& f : t.fields) { - const QString ft = lidlTypeToStdCdylib(f.type, recs); + const QString ft = lidlFieldTypeCdylib(f, recs); const QString fn = qs(f.name); // A missing field is reported at its own path rather than // default-constructed: a record that silently loses a field is the // failure mode this whole layer exists to prevent. + // + // DECODE needs no optional branch, and that is the point: an absent + // key is already materialised as null right here, so absent and + // explicit null arrive at the codec indistinguishable. In an + // optional field Codec> answers nullopt for both; + // in a required one Codec still rejects both. One expression, + // both halves of the rule. s << " out." << fn << " = Codec<" << ft << ">::from(\n"; s << " j.contains(\"" << fn << "\") ? j.at(\"" << fn << "\") : nlohmann::json(),\n"; @@ -452,6 +573,12 @@ QString lidlMakeTypesHeaderCdylib(const ModuleDecl& module) s << "#include \n"; // logos::Codec — the ONE definition s << "#include \n"; s << "#include \n"; + // Only when the contract actually declares an optional: logos_codec.h + // already pulls in, so this is documentation of what the emitted + // codec names — and emitting it unconditionally would rewrite the types + // header of every contract that has no optional at all. + if (moduleUsesOptional(module)) + s << "#include \n"; s << "#include \n"; s << "#include \n\n"; @@ -495,6 +622,8 @@ QString lidlMakeModuleImplExports(const ModuleDecl& module, s << "#include \n"; s << "#include \n"; s << "#include \n"; + if (moduleUsesOptional(module)) + s << "#include \n"; s << "#include \n"; s << "#include \n"; // The Qt-free typed dependency surface: LogosModules (behind modules()) @@ -662,12 +791,29 @@ QString lidlMakeModuleImplExports(const ModuleDecl& module, s << " try {\n"; for (const MethodDecl& md : module.methods) { + // The arity gate, and the one place the LIBERAL half of the decode rule + // reaches a POSITIONAL slot. + // + // A canonical encoder never changes arity: an empty positional slot is + // spelled null and still occupies its position. But absent and null are + // the same state on decode, so an optional trailing argument may also + // simply not be there. The gate therefore admits anything from the last + // REQUIRED parameter onwards, and each optional beyond it materialises + // as null exactly the way an absent record field already does. Below + // that point nothing changes: a missing required argument is still a + // hard reject, and a contract with no optional parameters emits the + // byte-identical `args.size() < ` it always did. + size_t minArgs = 0; + for (size_t i = 0; i < md.params.size(); ++i) + if (!paramIsOptional(md.params[i])) minArgs = i + 1; s << " if (m == \"" << md.name << "\") {\n"; - s << " if (args.size() < " << md.params.size() << ") return nullptr;\n"; + s << " if (args.size() < " << minArgs << ") return nullptr;\n"; QString call = "lidlImpl()." + qs(md.name) + "("; - for (int i = 0; i < md.params.size(); ++i) { - call += jsonArgToStd(md.params[i].type, - QString("args.at(%1)").arg(i), + for (size_t i = 0; i < md.params.size(); ++i) { + const QString expr = (i < minArgs) + ? QString("args.at(%1)").arg(i) + : QString("(args.size() > %1 ? args.at(%1) : nlohmann::json())").arg(i); + call += jsonArgToStd(md.params[i].type, expr, QString("arg%1").arg(i), recs); if (i + 1 < md.params.size()) call += ", "; } @@ -757,6 +903,8 @@ QString lidlMakeEventsSourceCdylib(const ModuleDecl& module, s << "#include \n\n"; s << "#include \n"; s << "#include \n"; + if (moduleUsesOptional(module)) + s << "#include \n"; s << "#include \n"; s << "#include \n"; // LogosMap / LogosList (nlohmann aliases) appear in the emitted signatures @@ -786,6 +934,7 @@ QString lidlMakeEventsSourceCdylib(const ModuleDecl& module, // helpful. if (stdType == "std::string" || stdType.startsWith("std::vector") || stdType.startsWith("std::map") + || stdType.startsWith("std::optional") || isRecord(ed.params[i].type, recsEv) || stdType == "LogosMap" || stdType == "LogosList") s << "const " << stdType << "& " << ed.params[i].name; @@ -800,9 +949,16 @@ QString lidlMakeEventsSourceCdylib(const ModuleDecl& module, // A record or a composite carrying bytes rides the generated codec, // exactly like a method return — otherwise an event payload would be // the one place a bstr silently loses its tag. + // + // An optional joins them: an event parameter is a POSITIONAL slot, + // so empty is spelled null and the argument list keeps its length. + // Codec>::to answers exactly that. (`?any` collapsed + // to LogosMap above and is excluded by the same guard the untyped + // aliases always were.) if (evStd != "LogosMap" && evStd != "LogosList" && (isRecord(pd.type, recsEv) - || pd.type.kind == TypeExpr::Array || pd.type.kind == TypeExpr::Map)) { + || pd.type.kind == TypeExpr::Array || pd.type.kind == TypeExpr::Map + || pd.type.kind == TypeExpr::Optional)) { s << " args.push_back(logos::toJson<" << evStd << ">(" << pd.name << "));\n"; continue; diff --git a/cpp-generator/experimental/lidl_gen_client.cpp b/cpp-generator/experimental/lidl_gen_client.cpp index 31b49eb..9a9c94a 100644 --- a/cpp-generator/experimental/lidl_gen_client.cpp +++ b/cpp-generator/experimental/lidl_gen_client.cpp @@ -180,6 +180,18 @@ static QString qtArgExpr(const TypeExpr& te, const QString& name) return holdsRecord ? 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. +static QString lidlFieldTypeQt(const FieldDecl& f) +{ + return fieldIsOptional(f) ? QString("QVariant") : lidlTypeToQt(f.type); +} + static void emitRecords(QTextStream& s, const ModuleDecl& module) { if (module.types.empty()) return; @@ -188,7 +200,7 @@ static void emitRecords(QTextStream& s, const ModuleDecl& module) s << "/// `" << n << "` — a record declared by the `" << qs(module.name) << "` contract.\n"; s << "struct " << n << " {\n"; for (const FieldDecl& f : t.fields) - s << " " << lidlTypeToQt(f.type) << " " << qs(f.name) << "{};\n"; + s << " " << lidlFieldTypeQt(f) << " " << qs(f.name) << "{};\n"; s << "};\n\n"; } // Conversions come after ALL structs so records may reference each other. @@ -196,17 +208,35 @@ static void emitRecords(QTextStream& s, const ModuleDecl& module) const QString n = qs(t.name); s << "inline QVariant " << n << "ToVariant(const " << n << "& v)\n{\n"; s << " QVariantMap __m;\n"; - for (const FieldDecl& f : t.fields) + 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 + // 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"; + continue; + } s << " __m.insert(\"" << qs(f.name) << "\", " << qtToVariantExpr(f.type, "v." + qs(f.name)) << ");\n"; + } s << " return QVariant(__m);\n}\n\n"; s << "inline " << n << " " << n << "FromVariant(const QVariant& value)\n{\n"; s << " const QVariantMap __m = value.toMap();\n"; s << " " << n << " __out;\n"; - for (const FieldDecl& f : t.fields) + 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"; + continue; + } s << " __out." << qs(f.name) << " = " << qtFromVariantExpr(f.type, "__m.value(\"" + qs(f.name) + "\")") << ";\n"; + } s << " return __out;\n}\n\n"; } } diff --git a/cpp-generator/legacy/main.cpp b/cpp-generator/legacy/main.cpp index f82b143..064010c 100644 --- a/cpp-generator/legacy/main.cpp +++ b/cpp-generator/legacy/main.cpp @@ -50,6 +50,52 @@ static QString lidlTypeExprToQtTypeName(const TypeExpr& te) return lidlTypeToQt(te); } +// Report every optional slot this path is about to flatten. +// +// THE BOUNDARY. Everything below (moduleMethodsToJson / moduleRecordsToJson / +// moduleEventsToJson -> generator_lib) consumes a single Qt TYPE-NAME STRING per +// slot. A TypeExpr's optionality — like its nesting, its map key type and its +// descriptions — does not survive being turned into a name, and Qt has no name +// to survive as: there is no metatype for an optional, so the mapper answers +// QVariant (see lidlTypeToQt). +// +// That leaves a Qt/Lp consumer of an optional slot with the right SHAPE and no +// TYPE: an invalid QVariant is Qt's empty inhabitant, so two-stateness survives, +// but the consumer gets no compile-time check on the value and cannot tell +// `?tstr` from `?uint` or recover a `?Record`'s struct. Carrying optionality +// past this point means widening the JSON surface these three functions produce +// and teaching generator_lib a per-slot optional flag on both the Qt and Lp +// emitters — real work, and not what this change is. +// +// So it stays flattened, and says so. Silence here is what made the gap easy to +// miss in the first place; a build that generates a Qt consumer for an optional +// contract should not look like a build that had nothing to lose. +static void noteOptionalFlattened(const ModuleDecl& mod, const QString& where, + QTextStream& err) +{ + QStringList optSlots; + for (const TypeDecl& td : mod.types) + for (const FieldDecl& fd : td.fields) + if (fieldIsOptional(fd)) + optSlots << (qs(td.name) + "." + qs(fd.name)); + for (const MethodDecl& md : mod.methods) { + for (const ParamDecl& pd : md.params) + if (paramIsOptional(pd)) + optSlots << (qs(md.name) + "(" + qs(pd.name) + ")"); + if (typeIsOptional(md.returnType)) + optSlots << (qs(md.name) + "() return"); + } + for (const EventDecl& ed : mod.events) + for (const ParamDecl& pd : ed.params) + if (paramIsOptional(pd)) + optSlots << (qs(ed.name) + "(" + qs(pd.name) + ")"); + if (optSlots.isEmpty()) return; + err << "Note: " << where << ": optional slot(s) [" << optSlots.join(", ") + << "] are generated as untyped QVariant. The Qt/Lp consumer surface has " + "no optional type, so `?T` keeps its two states (an invalid QVariant " + "is the empty one) but loses T.\n"; +} + static QJsonArray moduleRecordsToJson(const ModuleDecl& mod); // Load events from a `.lidl` sidecar shipped alongside a module's @@ -76,6 +122,7 @@ static QJsonArray loadEventsFromLidl(const QString& lidlPath, QTextStream& err, return result; } + noteOptionalFlattened(pr.module, lidlPath, err); if (outRecords) *outRecords = moduleRecordsToJson(pr.module); for (const EventDecl& ed : pr.module.events) { QJsonObject obj; @@ -300,6 +347,8 @@ static bool generateInterfaceWrappers(const QVector& ifaces, } } + noteOptionalFlattened(mod, spec.path, err); + const QString className = toPascalCase(spec.name); const QJsonArray methods = moduleMethodsToJson(mod); const QJsonArray events = moduleEventsToJson(mod); diff --git a/doctests/cpp-sdk-concurrent-dispatch.test.yaml b/doctests/cpp-sdk-concurrent-dispatch.test.yaml index 60490a4..25b3465 100644 --- a/doctests/cpp-sdk-concurrent-dispatch.test.yaml +++ b/doctests/cpp-sdk-concurrent-dispatch.test.yaml @@ -190,13 +190,14 @@ sections: The `{release}` overrides point the builder's C++ SDK, Qt glue, and protocol at the commits under test, so the concurrent-dispatch codegen + runtime are the ones exercised: - run: "sh -c 'cd slow-worker && printf \"result\\nresult-*\\n\" > .gitignore && git init -q && git add -A && nix flake update && git add flake.lock && nix build .#lgx -o worker-lgx --override-input logos-module-builder \"github:logos-co/logos-module-builder{release}\" --override-input logos-module-builder/logos-cpp-sdk \"github:logos-co/logos-cpp-sdk{release}\" --override-input logos-module-builder/logos-qt-sdk \"github:logos-co/logos-qt-sdk{release}\" --override-input logos-module-builder/logos-protocol \"github:logos-co/logos-protocol{release}\"'" + run: "sh -c 'cd slow-worker && printf \"result\\nresult-*\\n\" > .gitignore && git init -q && git add -A && nix flake update && git add flake.lock && nix build .#lgx -o worker-lgx --override-input logos-module-builder \"github:logos-co/logos-module-builder{release}\" --override-input logos-module-builder/logos-cpp-sdk \"github:logos-co/logos-cpp-sdk{release}\" --override-input logos-module-builder/logos-qt-sdk \"github:logos-co/logos-qt-sdk{release}\" --override-input logos-module-builder/logos-qt-sdk/logos-lidl \"github:logos-co/logos-lidl\" --override-input logos-module-builder/logos-protocol \"github:logos-co/logos-protocol{release}\"'" code_block: | cd slow-worker git init && git add -A && nix flake update && git add flake.lock nix build .#lgx -o worker-lgx \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ --override-input logos-module-builder/logos-qt-sdk 'github:logos-co/logos-qt-sdk' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-module-builder/logos-protocol 'github:logos-co/logos-protocol' check_file: "slow-worker/worker-lgx" @@ -324,7 +325,7 @@ sections: }; } - title: "Build it" - run: "sh -c 'cd fanout-driver && printf \"result\\nresult-*\\n\" > .gitignore && git init -q && git add -A && nix flake update --override-input slow_worker_module path:$PWD/../slow-worker && git add flake.lock && nix build .#lgx -o driver-lgx --override-input slow_worker_module path:$PWD/../slow-worker --override-input logos-module-builder \"github:logos-co/logos-module-builder{release}\" --override-input logos-module-builder/logos-cpp-sdk \"github:logos-co/logos-cpp-sdk{release}\" --override-input logos-module-builder/logos-qt-sdk \"github:logos-co/logos-qt-sdk{release}\" --override-input logos-module-builder/logos-protocol \"github:logos-co/logos-protocol{release}\" --override-input slow_worker_module/logos-module-builder/logos-cpp-sdk \"github:logos-co/logos-cpp-sdk{release}\" --override-input slow_worker_module/logos-module-builder/logos-qt-sdk \"github:logos-co/logos-qt-sdk{release}\" --override-input slow_worker_module/logos-module-builder/logos-protocol \"github:logos-co/logos-protocol{release}\"'" + run: "sh -c 'cd fanout-driver && printf \"result\\nresult-*\\n\" > .gitignore && git init -q && git add -A && nix flake update --override-input slow_worker_module path:$PWD/../slow-worker && git add flake.lock && nix build .#lgx -o driver-lgx --override-input slow_worker_module path:$PWD/../slow-worker --override-input logos-module-builder \"github:logos-co/logos-module-builder{release}\" --override-input logos-module-builder/logos-cpp-sdk \"github:logos-co/logos-cpp-sdk{release}\" --override-input logos-module-builder/logos-qt-sdk \"github:logos-co/logos-qt-sdk{release}\" --override-input logos-module-builder/logos-qt-sdk/logos-lidl \"github:logos-co/logos-lidl\" --override-input logos-module-builder/logos-protocol \"github:logos-co/logos-protocol{release}\" --override-input slow_worker_module/logos-module-builder/logos-cpp-sdk \"github:logos-co/logos-cpp-sdk{release}\" --override-input slow_worker_module/logos-module-builder/logos-qt-sdk \"github:logos-co/logos-qt-sdk{release}\" --override-input slow_worker_module/logos-module-builder/logos-qt-sdk/logos-lidl \"github:logos-co/logos-lidl\" --override-input slow_worker_module/logos-module-builder/logos-protocol \"github:logos-co/logos-protocol{release}\"'" code_block: | cd fanout-driver git init && git add -A @@ -334,8 +335,10 @@ sections: --override-input slow_worker_module path:$PWD/../slow-worker \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ --override-input logos-module-builder/logos-qt-sdk 'github:logos-co/logos-qt-sdk' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-module-builder/logos-protocol 'github:logos-co/logos-protocol' \ - --override-input slow_worker_module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' + --override-input slow_worker_module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input slow_worker_module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' check_file: "fanout-driver/driver-lgx" - title: "Run it: the single driver overlaps the multi worker" diff --git a/doctests/cpp-sdk-module-composition.test.yaml b/doctests/cpp-sdk-module-composition.test.yaml index ba9c3ed..70ea3da 100644 --- a/doctests/cpp-sdk-module-composition.test.yaml +++ b/doctests/cpp-sdk-module-composition.test.yaml @@ -539,12 +539,14 @@ sections: nix build 'path:./greeter_module#lgx' \ --override-input logos-module-builder 'github:logos-co/logos-module-builder{release}' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o greeter-lgx code_block: | # From inside the greeter clone this is simply: # nix build '.#lgx' --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' nix build 'path:./greeter_module#lgx' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o greeter-lgx post_text: "The greeter package is under `./greeter-lgx/`:" extra_run: @@ -562,13 +564,17 @@ sections: --override-input greeter_module 'path:./greeter_module' \ --override-input logos-module-builder 'github:logos-co/logos-module-builder{release}' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input greeter_module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input greeter_module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o orchestrator-lgx code_block: | nix build 'path:./orchestrator_module#lgx' \ --override-input greeter_module 'path:./greeter_module' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input greeter_module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input greeter_module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o orchestrator-lgx post_text: "The orchestrator package is under `./orchestrator-lgx/`:" extra_run: @@ -585,17 +591,23 @@ sections: run: | nix build 'github:logos-co/logos-logoscore-cli{release}' \ --override-input logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-liblogos/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ - --override-input logos-module-client/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-liblogos/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-capability-module/logos-module-builder 'github:logos-co/logos-module-builder{release}' \ --override-input logos-capability-module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-capability-module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ + --override-input logos-capability-module/logos-module-builder/logos-test-framework/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --out-link ./logos code_block: | nix build 'github:logos-co/logos-logoscore-cli' \ --override-input logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-liblogos/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ - --override-input logos-module-client/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-liblogos/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-capability-module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-capability-module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ + --override-input logos-capability-module/logos-module-builder/logos-test-framework/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --out-link ./logos check_file: "logos/bin/logoscore" diff --git a/doctests/cpp-sdk-module-runtime.test.yaml b/doctests/cpp-sdk-module-runtime.test.yaml index 5f1e039..6f2d70a 100644 --- a/doctests/cpp-sdk-module-runtime.test.yaml +++ b/doctests/cpp-sdk-module-runtime.test.yaml @@ -13,11 +13,13 @@ intro: | 1. Build the `logoscore` CLI, **overriding `logos-cpp-sdk` with the commit under test** — and overriding it in the same way for every consumer in - `logoscore`'s closure (`logos-liblogos`, `logos-module-client`, and the - `capability_module`'s `logos-module-builder`). Because the published flakes - pin the SDK independently (there is no single `follows` unifying them), all - of these must point at the same commit so the whole runtime is built and - linked against one consistent SDK ABI. + `logoscore`'s closure (`logos-liblogos` and the `capability_module`'s + `logos-module-builder`). Because the published flakes pin the SDK + independently (there is no single `follows` unifying them), all of these + must point at the same commit so the whole runtime is built and linked + against one consistent SDK ABI. Each `logos-qt-sdk` dragged onto the new + SDK also gets its sibling `logos-lidl` moved with it — see the note in + the first section. 2. Build the `lgpm` local package manager. 3. Build the real [`accounts_module`](https://github.com/logos-co/logos-accounts-module) as an `.lgx` package straight from its own flake — **also built against the @@ -62,31 +64,46 @@ sections: every consumer that pins the SDK in `logoscore`'s closure. The result is symlinked to `./logos/`. - > Unlike a leaf input, the SDK is pinned independently by `logos-liblogos`, - > `logos-module-client`, and the `capability_module`'s `logos-module-builder` - > — there is no single `follows` tying them together in the published - > flakes. So we override it at each of those paths (e.g. + > Unlike a leaf input, the SDK is pinned independently by `logos-liblogos` + > and the `capability_module`'s `logos-module-builder` — there is no single + > `follows` tying them together in the published flakes. So we override it + > at each of those paths (e.g. > `--override-input logos-liblogos/logos-cpp-sdk …`) to keep the whole > runtime on one consistent SDK ABI. Each override URL carries a `{release}` > placeholder the doc-test runner expands to a concrete ref: locally that is > this checkout's `HEAD` (see `run.sh`); in CI it is the commit being > tested. With no pin it falls back to latest `master`. + + > **Why the `logos-qt-sdk/logos-lidl` overrides.** This SDK installs + > `share/lidl-frontend/lidl_compat.h`, and `logos-qt-sdk`'s + > `logos-qt-generator` *compiles* that header against **its own** + > `logos-lidl` input. `logos-lidl` is a **sibling** of `logos-cpp-sdk` + > under `logos-qt-sdk`, not a descendant, so overriding the SDK moves the + > header forward while qt-sdk keeps its older lidl — and the generator + > fails to compile the shim. Each `logos-qt-sdk` node that ends up on the + > SDK under test therefore needs its `logos-lidl` moved with it. steps: - title: "Build the CLI with the SDK override" run: | nix build 'github:logos-co/logos-logoscore-cli{release}' \ --override-input logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-liblogos/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ - --override-input logos-module-client/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-liblogos/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-capability-module/logos-module-builder 'github:logos-co/logos-module-builder{release}' \ --override-input logos-capability-module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-capability-module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ + --override-input logos-capability-module/logos-module-builder/logos-test-framework/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --out-link ./logos code_block: | nix build 'github:logos-co/logos-logoscore-cli' \ --override-input logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-liblogos/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ - --override-input logos-module-client/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-liblogos/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-capability-module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-capability-module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ + --override-input logos-capability-module/logos-module-builder/logos-test-framework/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --out-link ./logos check_file: "logos/bin/logoscore" post_text: | @@ -140,12 +157,14 @@ sections: nix build 'path:./logos-accounts-module#lgx' \ --override-input logos-module-builder 'github:logos-co/logos-module-builder{release}' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o accounts-lgx code_block: | # From inside the clone this is simply: # nix build '.#lgx' --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' nix build 'path:./logos-accounts-module#lgx' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o accounts-lgx post_text: "The `.lgx` package is now under `./accounts-lgx/`:" extra_run: diff --git a/doctests/cpp-sdk-worker-thread-http.test.yaml b/doctests/cpp-sdk-worker-thread-http.test.yaml index 7947877..8bc518b 100644 --- a/doctests/cpp-sdk-worker-thread-http.test.yaml +++ b/doctests/cpp-sdk-worker-thread-http.test.yaml @@ -412,10 +412,12 @@ sections: nix build 'path:./sensor_module#lgx' \ --override-input logos-module-builder 'github:logos-co/logos-module-builder{release}' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o sensor-lgx code_block: | nix build 'path:./sensor_module#lgx' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o sensor-lgx post_text: "The sensor package is under `./sensor-lgx/`:" extra_run: @@ -431,14 +433,18 @@ sections: --override-input sensor_module 'path:./sensor_module' \ --override-input logos-module-builder 'github:logos-co/logos-module-builder{release}' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input sensor_module/logos-module-builder 'github:logos-co/logos-module-builder{release}' \ --override-input sensor_module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input sensor_module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o http-lgx code_block: | nix build 'path:./http_module#lgx' \ --override-input sensor_module 'path:./sensor_module' \ --override-input logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input sensor_module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input sensor_module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ -o http-lgx post_text: "The http package is under `./http-lgx/`:" extra_run: @@ -454,17 +460,23 @@ sections: run: | nix build 'github:logos-co/logos-logoscore-cli{release}' \ --override-input logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-liblogos/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ - --override-input logos-module-client/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-liblogos/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-capability-module/logos-module-builder 'github:logos-co/logos-module-builder{release}' \ --override-input logos-capability-module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk{release}' \ + --override-input logos-capability-module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ + --override-input logos-capability-module/logos-module-builder/logos-test-framework/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --out-link ./logos code_block: | nix build 'github:logos-co/logos-logoscore-cli' \ --override-input logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-liblogos/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ - --override-input logos-module-client/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-liblogos/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --override-input logos-capability-module/logos-module-builder/logos-cpp-sdk 'github:logos-co/logos-cpp-sdk' \ + --override-input logos-capability-module/logos-module-builder/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ + --override-input logos-capability-module/logos-module-builder/logos-test-framework/logos-qt-sdk/logos-lidl 'github:logos-co/logos-lidl' \ --out-link ./logos check_file: "logos/bin/logoscore" diff --git a/flake.lock b/flake.lock index a8e0c88..4a7c800 100644 --- a/flake.lock +++ b/flake.lock @@ -12,11 +12,11 @@ ] }, "locked": { - "lastModified": 1781653473, - "narHash": "sha256-8l2tE2K5nY1grcFROQHC1By1jVI0NrfkS3k2v2y6R2I=", + "lastModified": 1785470541, + "narHash": "sha256-5fVAbSGMRhEfRXfhaoq26LZeJuziRT/NMFsfhtd0ogA=", "owner": "logos-co", "repo": "logos-lidl", - "rev": "8c95d4f0cc6a10195c70ed71e85b7a4cddca02f8", + "rev": "35f33d87b7d6c22d9d4658c58855ae887ce515f1", "type": "github" }, "original": { @@ -55,11 +55,11 @@ ] }, "locked": { - "lastModified": 1785350411, - "narHash": "sha256-SH3BdM6Z6mW0M5DCyIGz+oo8IWq13qSQAdZuLMi916g=", + "lastModified": 1785470728, + "narHash": "sha256-K6rr5ycCWctvtUnktskjRlXHN3GuI7FEbz8/FX+pVY8=", "owner": "logos-co", "repo": "logos-protocol", - "rev": "43595575a3f94b07f1a33deb161ace1f62c37e3b", + "rev": "72754ab9b2ea8a43a3f04c5bdf1411673f3f489e", "type": "github" }, "original": { diff --git a/tests/experimental/fixtures/optional_impl.h b/tests/experimental/fixtures/optional_impl.h new file mode 100644 index 0000000..9a8c11e --- /dev/null +++ b/tests/experimental/fixtures/optional_impl.h @@ -0,0 +1,50 @@ +#pragma once +// Fixture: OPTIONALITY derived from an impl header. +// +// The header-first C++ provider is the one authoring path where the contract is +// read out of the source rather than written by hand, so `std::optional` has +// to be recognised — before this fixture existed it fell through to the opaque +// `any` fallback with no diagnostic, and a module that declared an optional +// published a contract that said something else. +// +// `required` is here to hold the other half down: only the optional slots may +// become `?T`. And `maybeBlob` covers the case that has to compose — an optional +// over a declared record, not just over a scalar. +#include +#include +#include +#include +#include +#include + +struct Blob { + std::string id; + std::vector payload; +}; + +struct Profile { + std::string required; + std::optional nickname; + std::optional age; + std::optional> avatar; + std::optional blob; +}; +// `std::optional>` is deliberately NOT here: it has no LIDL +// type (three C++ states over a two-state wire), so the generated codec is +// written for the collapsed `?T` and does not compile against such a member. +// That is the intended outcome, and it is pinned by +// ImplHeaderParser.NestedOptionalCollapsesAndIsReported — a fixture used to +// prove the emitted code COMPILES cannot also carry a case that must not. + +class OptionalImpl : public LogosModuleContext { +public: + Profile echoProfile(const Profile& v); + std::optional echoOptional(const std::optional& v); + std::optional maybeBlob(const std::string& id); + std::vector> echoOptionalList( + const std::vector>& v); + std::string required(const std::string& v); + +logos_events: + void profileChanged(const std::string& id, const std::optional& nickname); +}; diff --git a/tests/experimental/fixtures/optional_metadata.json b/tests/experimental/fixtures/optional_metadata.json new file mode 100644 index 0000000..8d9dc26 --- /dev/null +++ b/tests/experimental/fixtures/optional_metadata.json @@ -0,0 +1,10 @@ +{ + "name": "optional_module", + "version": "1.0.0", + "description": "Header-first optionality: std::optional <-> ?T", + "author": "Test", + "type": "core", + "category": "testing", + "main": "optional_module_plugin", + "dependencies": [] +} diff --git a/tests/experimental/test_impl_header_parser.cpp b/tests/experimental/test_impl_header_parser.cpp index d405afc..f5588da 100644 --- a/tests/experimental/test_impl_header_parser.cpp +++ b/tests/experimental/test_impl_header_parser.cpp @@ -564,3 +564,128 @@ TEST_F(ImplHeaderParserTest, ParsesMultiLineSignature) EXPECT_TRUE(names.contains("wrapped")) << "got: " << names.join(",").toStdString(); } + +// --------------------------------------------------------------------------- +// Optionality, header-first +// +// `std::optional` used to fall through to the opaque `any` fallback with no +// diagnostic, so a header-first C++ provider could not express an optional at +// all: it declared one and published a contract that said something else. +// --------------------------------------------------------------------------- + +TEST_F(ImplHeaderParserTest, StdOptionalBecomesOptional) +{ + auto r = parseImplHeader( + fixturesDir() + "/optional_impl.h", + "OptionalImpl", + fixturesDir() + "/optional_metadata.json", + err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + + auto findType = [&](const char* n) -> const TypeDecl* { + for (const auto& t : r.module.types) + if (t.name == n) return &t; + return nullptr; + }; + auto findMethod = [&](const char* n) -> const MethodDecl* { + for (const auto& m : r.module.methods) + if (m.name == n) return &m; + return nullptr; + }; + + const TypeDecl* profile = findType("Profile"); + ASSERT_NE(profile, nullptr); + auto fieldNamed = [&](const char* n) -> const FieldDecl* { + for (const auto& f : profile->fields) + if (f.name == n) return &f; + return nullptr; + }; + + // A plain member stays required. + ASSERT_NE(fieldNamed("required"), nullptr); + EXPECT_FALSE(fieldIsOptional(*fieldNamed("required"))); + + // Optional members carry the value type, not `any`. + ASSERT_NE(fieldNamed("nickname"), nullptr); + EXPECT_TRUE(fieldIsOptional(*fieldNamed("nickname"))); + EXPECT_EQ(fieldValueType(*fieldNamed("nickname")).name, "tstr"); + EXPECT_EQ(fieldValueType(*fieldNamed("age")).name, "uint"); + EXPECT_EQ(fieldValueType(*fieldNamed("avatar")).name, "bstr"); + + // Optional composes with a declared record — and `std::optional` is + // still a MENTION of Blob, so the record survives the + // keep-only-referenced-records pass instead of being dropped as unused. + ASSERT_NE(fieldNamed("blob"), nullptr); + EXPECT_EQ(fieldValueType(*fieldNamed("blob")).kind, TypeExpr::Named); + EXPECT_EQ(fieldValueType(*fieldNamed("blob")).name, "Blob"); + EXPECT_NE(findType("Blob"), nullptr); + + // Parameters and returns, and optional nested inside a container. + const MethodDecl* echo = findMethod("echoOptional"); + ASSERT_NE(echo, nullptr); + ASSERT_EQ(echo->params.size(), 1u); + EXPECT_TRUE(paramIsOptional(echo->params[0])); + EXPECT_EQ(paramValueType(echo->params[0]).name, "tstr"); + EXPECT_TRUE(typeIsOptional(echo->returnType)); + + const MethodDecl* lst = findMethod("echoOptionalList"); + ASSERT_NE(lst, nullptr); + ASSERT_EQ(lst->params.size(), 1u); + EXPECT_EQ(lst->params[0].type.kind, TypeExpr::Array); + ASSERT_EQ(lst->params[0].type.elements.size(), 1u); + EXPECT_EQ(lst->params[0].type.elements[0].kind, TypeExpr::Optional); + + // A required method is untouched. + const MethodDecl* req = findMethod("required"); + ASSERT_NE(req, nullptr); + EXPECT_FALSE(paramIsOptional(req->params[0])); + EXPECT_FALSE(typeIsOptional(req->returnType)); + + // The event parameter, likewise. + ASSERT_EQ(r.module.events.size(), 1u); + ASSERT_EQ(r.module.events[0].params.size(), 2u); + EXPECT_FALSE(paramIsOptional(r.module.events[0].params[0])); + EXPECT_TRUE(paramIsOptional(r.module.events[0].params[1])); +} + +// std::optional> has NO LIDL type: three C++ states over a +// two-state wire. It maps down to `?T` — which is what makes the author's own +// declaration stop compiling against the generated codec, deliberately — and +// says so here, rather than leaving a conversion error in generated code. +TEST_F(ImplHeaderParserTest, NestedOptionalCollapsesAndIsReported) +{ + QTemporaryDir dir; + ASSERT_TRUE(dir.isValid()); + const QString hp = dir.filePath("nested_impl.h"); + { + QFile f(hp); + ASSERT_TRUE(f.open(QIODevice::WriteOnly | QIODevice::Text)); + f.write( + "#pragma once\n" + "#include \n" + "#include \n" + "struct Rec {\n" + " std::optional> collapsed;\n" + "};\n" + "class NestedImpl {\n" + "public:\n" + " Rec echo(const Rec& v);\n" + "};\n"); + } + auto r = parseImplHeader(hp, "NestedImpl", + fixturesDir() + "/sample_metadata.json", err); + ASSERT_FALSE(r.hasError()) << r.error.toStdString(); + + ASSERT_EQ(r.module.types.size(), 1u); + ASSERT_EQ(r.module.types[0].fields.size(), 1u); + const FieldDecl& f = r.module.types[0].fields[0]; + EXPECT_TRUE(fieldIsOptional(f)); + // Collapsed to ONE layer — the contract may not carry a third state. + EXPECT_EQ(fieldValueType(f).kind, TypeExpr::Primitive); + EXPECT_EQ(fieldValueType(f).name, "tstr"); + + err.flush(); + EXPECT_TRUE(errOutput.contains("std::optional>")) + << errOutput.toStdString(); + EXPECT_TRUE(errOutput.contains("no LIDL type")) << errOutput.toStdString(); +} diff --git a/tests/experimental/test_lidl_gen_cdylib.cpp b/tests/experimental/test_lidl_gen_cdylib.cpp index 4e5b3c0..35f36dd 100644 --- a/tests/experimental/test_lidl_gen_cdylib.cpp +++ b/tests/experimental/test_lidl_gen_cdylib.cpp @@ -335,3 +335,222 @@ TEST(LidlGenCdylib, SupportedEventParamsRemainEligible) QString error; EXPECT_TRUE(lidlCdylibSupported(m, &error)) << error.toStdString(); } + +// --------------------------------------------------------------------------- +// Optionality — `?T` on the Qt-free cdylib surface. +// +// Two-state (a value of T, or empty), std::optional, and the wire rule that +// depends on the SLOT: empty omits the key where the slot is named (a record +// field) and is spelled null where it is positional (an argument, a return, an +// event parameter — those have no key to omit and their arity must not change). +// --------------------------------------------------------------------------- + +TypeExpr opt(const TypeExpr& inner) +{ + return {TypeExpr::Optional, "", {inner}}; +} + +FieldDecl field(const char* name, const TypeExpr& type) +{ + FieldDecl f; + f.name = name; + f.type = type; + return f; +} + +// `?T` used to be a HARD REJECT — "module not cdylib-eligible" — so nothing +// downstream could even be reached. The gate opens exactly as far as the value +// type allows. +TEST(LidlGenCdylib, OptionalIsEligibleWhenItsValueTypeIs) +{ + ModuleDecl m; + m.name = "o_module"; + m.methods.push_back(method("echoOptional", opt(prim("tstr")), + {param("v", opt(prim("tstr")))})); + m.methods.push_back(method("nested", opt(TypeExpr{TypeExpr::Array, "", {prim("bstr")}}), + {param("v", TypeExpr{TypeExpr::Array, "", {opt(prim("int"))}})})); + + QString error; + EXPECT_TRUE(lidlCdylibSupported(m, &error)) << error.toStdString(); +} + +// `result` and `void` are return-only spellings, and neither can be optional: +// void is the absence of a value, and result already carries its own +// success/error discriminant. The value type is checked as a non-return +// position, which is what makes both fall out. +TEST(LidlGenCdylib, OptionalResultAndVoidAreRejected) +{ + for (const char* n : {"result", "void"}) { + ModuleDecl m; + m.name = "o_module"; + m.methods.push_back(method("bad", opt(prim(n)), {})); + QString error; + EXPECT_FALSE(lidlCdylibSupported(m, &error)) << n; + EXPECT_TRUE(error.contains("bad")) << error.toStdString(); + } +} + +// R3. `? name: T` (the field flag) and `name: ?T` (the type kind) are the same +// declaration and MUST emit byte-identical code. Nothing enforced that before — +// a backend reading only one of the two would have silently disagreed with the +// next one to try. +TEST(LidlGenCdylib, BothOptionalSpellingsEmitIdenticalCode) +{ + auto moduleWithField = [](const FieldDecl& f) { + ModuleDecl m; + m.name = "o_module"; + TypeDecl t; + t.name = "Opt"; + t.fields = {f}; + m.types.push_back(t); + return m; + }; + + FieldDecl flagged = field("maybe", prim("tstr")); + flagged.optional = true; // `? maybe: tstr` + const FieldDecl typed = field("maybe", opt(prim("tstr"))); // `maybe: ?tstr` + + const QString a = lidlMakeTypesHeaderCdylib(moduleWithField(flagged)); + const QString b = lidlMakeTypesHeaderCdylib(moduleWithField(typed)); + EXPECT_EQ(a, b) << a.toStdString() << "\n---\n" << b.toStdString(); + EXPECT_TRUE(a.contains("std::optional")) << a.toStdString(); +} + +// R2, named slot: empty OMITS the key. Writing null instead would be the +// positional spelling in a slot that has a name — and Codec> +// cannot do this itself, because a codec only ever sees a value, never the slot. +TEST(LidlGenCdylib, OptionalRecordFieldOmitsTheKeyWhenEmpty) +{ + ModuleDecl m; + m.name = "o_module"; + TypeDecl t; + t.name = "Opt"; + t.fields = {field("required", prim("tstr")), field("maybe", opt(prim("tstr")))}; + m.types.push_back(t); + + const QString types = lidlMakeTypesHeaderCdylib(m); + EXPECT_TRUE(types.contains("if (v.maybe.has_value())")) << types.toStdString(); + EXPECT_TRUE(types.contains("out[\"maybe\"] = Codec::to(*v.maybe);")) + << types.toStdString(); + // Decode needs no optional branch: an absent key is ALREADY materialised as + // null right there, so absent and explicit null reach the codec + // indistinguishable — nullopt in an optional field, still an error in a + // required one. + EXPECT_TRUE(types.contains("out.maybe = Codec>::from(")) + << types.toStdString(); + EXPECT_TRUE(types.contains("j.contains(\"maybe\") ? j.at(\"maybe\") : nlohmann::json()")) + << types.toStdString(); + // The required field is untouched by any of this. + EXPECT_TRUE(types.contains("out[\"required\"] = Codec::to(v.required);")) + << types.toStdString(); + EXPECT_TRUE(types.contains("#include ")) << types.toStdString(); +} + +// A contract with no optional keeps its generated output byte-for-byte, down to +// the include list — every cpp-sdk change rebuilds the whole module graph, so a +// gratuitous diff here is a rebuild of everything. +TEST(LidlGenCdylib, NoOptionalMeansNoOptionalInclude) +{ + ModuleDecl m; + m.name = "o_module"; + TypeDecl t; + t.name = "Plain"; + t.fields = {field("id", prim("tstr"))}; + m.types.push_back(t); + + EXPECT_FALSE(lidlMakeTypesHeaderCdylib(m).contains("#include ")); +} + +// R2, positional slot: arity never changes on the way OUT, but absent and null +// are the same state coming IN — so the gate admits a missing trailing optional +// and materialises it as null, exactly the way a missing record field already is. +TEST(LidlGenCdylib, OptionalArgumentMayBeAbsentOrNull) +{ + ModuleDecl m; + m.name = "o_module"; + m.methods.push_back(method("f", prim("tstr"), + {param("required", prim("tstr")), + param("maybe", opt(prim("tstr")))})); + + const QString src = lidlMakeModuleImplExports(m, "OImpl", "o_impl.h"); + EXPECT_TRUE(src.contains("if (args.size() < 1) return nullptr;")) << src.toStdString(); + EXPECT_TRUE(src.contains("(args.size() > 1 ? args.at(1) : nlohmann::json())")) + << src.toStdString(); + EXPECT_TRUE(src.contains("logos::fromJson>")) + << src.toStdString(); + // The REQUIRED argument keeps the hard gate and the plain accessor. + EXPECT_TRUE(src.contains("logos::fromJson(args.at(0), \"arg0\")")) + << src.toStdString(); +} + +// ...and a method with no optional parameter emits the gate it always did. +TEST(LidlGenCdylib, RequiredOnlyArityGateIsUnchanged) +{ + ModuleDecl m; + m.name = "o_module"; + m.methods.push_back(method("f", prim("tstr"), + {param("a", prim("tstr")), param("b", prim("tstr"))})); + + const QString src = lidlMakeModuleImplExports(m, "OImpl", "o_impl.h"); + EXPECT_TRUE(src.contains("if (args.size() < 2) return nullptr;")) << src.toStdString(); + EXPECT_FALSE(src.contains("args.size() > ")) << src.toStdString(); +} + +// R4. Optional widens the accepted domain by exactly ONE inhabitant (empty); a +// present value is still decoded as T. For `bstr` that has to be the LENIENT +// decode a bare `bstr` argument gets, or the identical value would be accepted +// in a required slot and rejected in an optional one. +TEST(LidlGenCdylib, OptionalBytesArgumentKeepsTheLenientDecode) +{ + ModuleDecl m; + m.name = "o_module"; + m.methods.push_back(method("f", prim("bool"), {param("v", opt(prim("bstr")))})); + + const QString src = lidlMakeModuleImplExports(m, "OImpl", "o_impl.h"); + EXPECT_TRUE(src.contains("logos::bytesFromJsonLenient")) << src.toStdString(); + EXPECT_TRUE(src.contains(".is_null() ? std::optional>()")) + << src.toStdString(); +} + +// `?any` collapses onto `any`. nlohmann::json already HAS null among its +// inhabitants, so std::optional would give the slot two spellings of +// empty — three-state, which is the one thing `?T` may never be. +TEST(LidlGenCdylib, OptionalAnyCollapsesOntoAny) +{ + ModuleDecl m; + m.name = "o_module"; + TypeDecl t; + t.name = "Loose"; + t.fields = {field("blob", opt(prim("any")))}; + m.types.push_back(t); + m.methods.push_back(method("f", prim("bool"), {param("v", opt(prim("any")))})); + + QString error; + ASSERT_TRUE(lidlCdylibSupported(m, &error)) << error.toStdString(); + + const QString types = lidlMakeTypesHeaderCdylib(m); + EXPECT_TRUE(types.contains("Codec::to(v.blob)")) << types.toStdString(); + EXPECT_FALSE(types.contains("std::optional")) << types.toStdString(); + + const QString src = lidlMakeModuleImplExports(m, "OImpl", "o_impl.h"); + EXPECT_FALSE(src.contains("std::optional")) << src.toStdString(); +} + +// An event parameter is a POSITIONAL slot: empty is null, and the argument list +// keeps its length. It is also taken by const reference, like every other +// non-scalar, so the generated definition matches the author's declaration in +// the `logos_events:` block. +TEST(LidlGenCdylib, OptionalEventParamIsConstRefAndNullWhenEmpty) +{ + const ModuleDecl m = moduleWithEvent("changed", { + param("name", prim("tstr")), + param("instance", opt(prim("tstr"))), + }); + + const QString src = eventsSourceFor(m); + EXPECT_TRUE(src.contains("const std::optional& instance")) + << src.toStdString(); + EXPECT_TRUE(src.contains("args.push_back(logos::toJson>(instance));")) + << src.toStdString(); + EXPECT_TRUE(src.contains("#include ")) << src.toStdString(); +} diff --git a/tests/experimental/test_lidl_gen_client.cpp b/tests/experimental/test_lidl_gen_client.cpp index 8d1758f..9dcf4be 100644 --- a/tests/experimental/test_lidl_gen_client.cpp +++ b/tests/experimental/test_lidl_gen_client.cpp @@ -380,3 +380,106 @@ TEST(LidlGenClient, RecordsBecomeStructsWithConversions) EXPECT_TRUE(h.contains("QList makeStatuses(")) << h.toStdString(); } + +// --------------------------------------------------------------------------- +// Optionality +// +// This backend is not on any live build path (real Qt consumers come from the +// legacy interface-wrapper path), so the bar here is CONSISTENCY, not features: +// the two spellings of an optional field must not generate different structs +// from the same declaration. +// --------------------------------------------------------------------------- + +static ModuleDecl makeOptionalRecordModule(bool useFlagSpelling) +{ + ModuleDecl m; + m.name = "opt_module"; + m.version = "1.0.0"; + + TypeDecl t; + t.name = "Profile"; + { + FieldDecl f; f.name = "required"; f.type = { TypeExpr::Primitive, "tstr", {} }; + t.fields.push_back(f); + } + { + FieldDecl f; + f.name = "nickname"; + if (useFlagSpelling) { // `? nickname: tstr` + f.type = { TypeExpr::Primitive, "tstr", {} }; + f.optional = true; + } else { // `nickname: ?tstr` + f.type = { TypeExpr::Optional, "", { { TypeExpr::Primitive, "tstr", {} } } }; + } + t.fields.push_back(f); + } + m.types.push_back(t); + + MethodDecl md; + md.name = "echoProfile"; + md.returnType = { TypeExpr::Named, "Profile", {} }; + ParamDecl p; p.name = "v"; p.type = { TypeExpr::Named, "Profile", {} }; + md.params.push_back(p); + m.methods.push_back(md); + return m; +} + +TEST(LidlGenClient, BothOptionalSpellingsEmitIdenticalCode) +{ + const QString flagged = lidlMakeHeader(makeOptionalRecordModule(true), BindMode::Bound); + const QString typed = lidlMakeHeader(makeOptionalRecordModule(false), BindMode::Bound); + // Reading `f.type` alone made the flag spelling emit a bare `QString` — a + // type with no empty inhabitant at all — from the same declaration that the + // type spelling turned into a QVariant. + EXPECT_EQ(flagged, typed) << flagged.toStdString() << "\n---\n" << typed.toStdString(); +} + +TEST(LidlGenClient, OptionalRecordFieldIsTwoStateQVariant) +{ + 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(); + EXPECT_TRUE(h.contains("QString required{};")) << 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 + // `.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_FALSE(h.contains("__out.nickname = __m.value(\"nickname\").toString();")) + << h.toStdString(); +} + +// The `_bytes` collision check must read through an optional too. A PRESENT +// `? _bytes: tstr` still encodes to {"_bytes": "..."} — the shape that decodes +// as a byte string and loses the record — so both spellings have to be refused. +// Reading `f.type` refused only the flag one. +TEST(LidlGenClient, BytesTagCollisionIsRefusedThroughAnOptional) +{ + auto sneaky = [](bool useFlagSpelling) { + ModuleDecl m; + m.name = "sneaky_module"; + TypeDecl t; + t.name = "Sneaky"; + FieldDecl f; + f.name = "_bytes"; + if (useFlagSpelling) { + f.type = { TypeExpr::Primitive, "tstr", {} }; + f.optional = true; + } else { + f.type = { TypeExpr::Optional, "", { { TypeExpr::Primitive, "tstr", {} } } }; + } + t.fields.push_back(f); + m.types.push_back(t); + return m; + }; + + for (bool flag : {true, false}) { + QString error; + EXPECT_FALSE(lidlCheckRecords(sneaky(flag), &error)) << "flagSpelling=" << flag; + EXPECT_TRUE(error.contains("Sneaky")) << error.toStdString(); + } +} diff --git a/tests/experimental/test_lidl_type_mapping.cpp b/tests/experimental/test_lidl_type_mapping.cpp index 43053db..445076f 100644 --- a/tests/experimental/test_lidl_type_mapping.cpp +++ b/tests/experimental/test_lidl_type_mapping.cpp @@ -227,3 +227,53 @@ TEST(LidlToPascalCase, LeadingUnderscore) { EXPECT_EQ(lidlToPascalCase("_test"), "Test"); } + +// --------------------------------------------------------------------------- +// Optionality +// +// `?T` is TWO-state: a value of T, or empty. The two surfaces answer it +// differently because only one of them HAS an optional type — the std surface +// keeps T inside std::optional, the Qt surface loses it, because Qt has no +// optional metatype and an invalid QVariant is its single empty inhabitant. +// --------------------------------------------------------------------------- + +static TypeExpr opt(const TypeExpr& inner) +{ + return { TypeExpr::Optional, "", { inner } }; +} + +TEST(LidlTypeToStd, OptionalKeepsTheValueType) +{ + EXPECT_EQ(lidlTypeToStd(opt({ TypeExpr::Primitive, "tstr", {} })), + "std::optional"); + EXPECT_EQ(lidlTypeToStd(opt({ TypeExpr::Primitive, "uint", {} })), + "std::optional"); + EXPECT_EQ(lidlTypeToStd(opt({ TypeExpr::Primitive, "bstr", {} })), + "std::optional>"); +} + +// Two-state, so optionality is idempotent: `??T` denotes the same two states as +// `?T` and must not become std::optional> — that would be a +// third state on the C++ side with nowhere to put it on the wire. +TEST(LidlTypeToStd, NestedOptionalCollapses) +{ + const TypeExpr t = opt(opt({ TypeExpr::Primitive, "tstr", {} })); + EXPECT_EQ(lidlTypeToStd(t), "std::optional"); +} + +// A degenerate Optional carrying no element is unreachable from the parser but +// constructible by hand and over the JSON bridge. It must not recurse forever. +TEST(LidlTypeToStd, DegenerateOptionalTerminates) +{ + EXPECT_EQ(lidlTypeToStd(TypeExpr{ TypeExpr::Optional, "", {} }), "QVariant"); +} + +TEST(LidlTypeToQt, OptionalIsUntypedQVariant) +{ + // 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"); +}