mirror of
https://github.com/logos-co/logos-cpp-sdk.git
synced 2026-08-30 17:21:15 +00:00
Dario LipicarandClaude Opus 5
b4c2e5bb2d
`? maybe: tstr` and `maybe: ?tstr` are the same declaration. logos-lidl's
docs/spec.md says so and requires them to produce byte-identical code, and
logos-lidl#7 added fieldIsOptional()/fieldValueType() precisely so no backend
re-derives the answer. The cdylib, client-stub and both Rust backends honour
that. The LEGACY consumer path — the one every real C++ module builds through,
`logos-cpp-generator --general-only --dep <name>=<lidl>` from
logos-plugin-qt's buildPlugin.nix — did not:
? maybe: tstr -> QString maybe{}; __m.value("maybe").toString()
maybe: ?tstr -> QVariant maybe{}; __m.value("maybe")
and on `--api-style lp`, `std::string maybe{}` vs `LogosMap maybe{}` — neither
of them std::optional. The flag spelling is the one production contracts use:
logos-chat-module writes all five of its optionals that way, so every one of
them landed on the branch that silently defaults. A `QString` has no empty
inhabitant at all; an absent `nickname` and an empty-string one were the same
value by the time the consumer saw them.
ROOT CAUSE. legacy/main.cpp's moduleRecordsToJson / moduleMethodsToJson /
moduleEventsToJson flatten every TypeExpr into a single Qt TYPE-NAME STRING.
Answering "is this optional" from a name means answering it from the verbatim
spelling, which is the one thing the accessors exist to stop.
WHAT THIS CHANGES. The record-field half of that boundary, and only it. A field
object now carries `optional` alongside `type`, where `type` is the VALUE type
(fieldValueType) and `optional` is true for either spelling (fieldIsOptional).
Both spellings arrive at the emitter as the same object, so they leave as the
same code. Per surface, matching the sibling backend that already serves it:
Qt — QVariant, as lidl_gen_client.cpp already emits. Qt has no optional
template; an invalid QVariant is its single empty inhabitant. Two-state,
untyped.
Lp — std::optional<T>, as lidl_gen_cdylib.cpp already emits, encoded by
logos-protocol's Codec<std::optional<T>>. Keeps the value type.
`?any` / `?{K:V}` / `?[any]` collapse onto the bare LogosMap/LogosList:
nlohmann::json already carries null, so wrapping it would give the slot
two empty spellings — three states, which the two-state rule forbids.
Encode omits the key when empty (a record field is a NAMED slot); decode treats
an absent key and an explicit null as the same state, so neither turns empty
into "" or 0. The round trip is canonicalising, as the spec requires.
The three functions move to legacy/lidl_to_json.{h,cpp}. Not cosmetic: the rule
is a property of frontend -> JSON -> emitter, and while they sat inside a TU
with main() no test could observe it. tests/generator/test_optional_spellings.cpp
now runs that composition end to end.
WHAT THIS DOES NOT CHANGE, and why. POSITIONAL slots — method parameters,
return types, event parameters — are still flattened to QVariant (Qt) /
LogosMap (Lp). They have no name to hang a flag on, so they only ever had the
type-kind spelling and there is no divergence there to fix; what they lose is
the value type. Closing that changes generated method SIGNATURES, i.e. a source
break for every existing call site, for a defect this commit is not about.
`OptionalSpellings.PositionalSlotsAreStillFlattened` pins the current behaviour
so closing it later is deliberate, and the generator still prints a `Note:`
naming every slot it flattens. Nesting, map key types and descriptions still do
not cross the boundary either — the flag is per-field, not a general widening.
VERIFIED BY RUNNING, each check shown to fail when the property does not hold:
- Two contracts identical but for the spelling, generated with `--dep` on both
`--api-style qt` and `--api-style lp`: byte-identical. The SAME comparison on
a generator built from this base without the fix reports a difference on
both styles.
- Harness sensitivity: two contracts differing only in `count: uint` ->
`count: int` are reported as different, so the compare is not vacuous.
- A contract with no optional field generates byte-identically to the
unpatched generator, both styles — and the same compare reports a difference
when given genuinely different output.
- The 5 new assertions FAIL on this base with only the extraction grafted in
(generator_lib.cpp pristine) and pass with the fix; the 3 control assertions
pass on both, which is what makes them controls.
- Generated wrappers compile on both surfaces, for the probe contracts and for
the real logos-chat-module and test_fullapi_ext_rust contracts.
- Emitted lp record codec exercised at runtime: empty omits the key, an
explicit null decodes to nullopt rather than "", uint64 above 2^63 survives,
bstr stays canonically tagged, and `{"maybe": null}` re-encodes omitted.
- `nix build .#tests` green (107 tests), `nix build .#cpp-generator` green.
logos-qt-sdk's qt-generator has the same defect in lidl_gen_qt_consumer.cpp
(record fields read `f.type` directly); it is a separate repo and not touched
here.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>