Files
logos-cpp-sdk/cpp-generator/legacy/lidl_to_json.cpp
Dario LipicarandClaude Opus 5 b4c2e5bb2d fix(generator): one declaration, one binding — optional record fields on the legacy path (#130)
`? 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>
2026-08-03 09:43:28 -03:00

149 lines
6.3 KiB
C++

#include "lidl_to_json.h"
#include <QJsonObject>
#include <QStringList>
#include "../experimental/lidl_emit_common.h" // lidlTypeToQt — the one Qt type mapper
// Convert a TypeExpr → Qt-typed string name (same surface the
// metaobject-introspection path produces for methods, so generator_lib
// can consume both via one code path).
//
// ONE Qt type mapper. This used to be a near-duplicate of `lidlTypeToQt`
// (experimental/lidl_emit_common.cpp) and the two disagreed: this copy had no
// `void` case, so a `-> void` method reaching it as Primitive("void") from the
// impl-header parser fell through to QVariant and generated
// `QVariant doVoid(...)`. (The .lidl parser spells the same thing
// Named("void"), which survived only by accident — mapReturnType's
// `base == "void"` early-out.) The lp/std tables are DERIVED from this name, so
// the same bug produced `LogosMap doVoid(...)` on the Qt-free surface: not a
// Qt-only defect, a front-end one. It is now a delegation, so there is one
// table to disagree with.
QString lidlTypeExprToQtTypeName(const TypeExpr& te)
{
return lidlTypeToQt(te);
}
// Report every optional slot this path still flattens into a bare type name.
//
// A record FIELD no longer does: moduleRecordsToJson below carries `optional`
// alongside the value type, and the emitter reconstitutes it (QVariant on the
// Qt surface, std::optional<T> on the Lp one). What is still flattened is every
// POSITIONAL slot — a method parameter, a return type, an event parameter.
// Those have no name to hang a flag on, so they only ever had the type-kind
// spelling and there is no spelling divergence to fix; what they lose is the
// value TYPE, exactly as `lidlTypeToQt` documents (`?T` -> QVariant, and via
// the derived std table -> LogosMap). Two-stateness survives — an invalid
// QVariant / a JSON null is the empty inhabitant — but the consumer gets no
// compile-time check on the value and cannot tell `?tstr` from `?uint`.
//
// Widening those means changing the generated method SIGNATURES, which is a
// source break for every existing caller and buys nothing for the
// one-declaration-two-spellings rule. So they stay flattened, and say so.
void noteOptionalPositionalSlots(const ModuleDecl& mod, const QString& where,
QTextStream& err)
{
QStringList optSlots;
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 positional slot(s) ["
<< optSlots.join(", ")
<< "] are generated as untyped QVariant (LogosMap on the lp surface). A "
"positional slot has no name to carry an optional flag, so `?T` keeps "
"its two states (an invalid QVariant / a JSON null is the empty one) "
"but loses T. Record fields are unaffected — they carry optionality "
"through.\n";
}
// Build a getMethods()-shaped QJsonArray (the surface makeHeader/makeSource
// consume) from a parsed ModuleDecl. Every interface method is invokable.
QJsonArray moduleMethodsToJson(const ModuleDecl& mod)
{
QJsonArray arr;
for (const MethodDecl& m : mod.methods) {
QJsonObject o;
o["name"] = qs(m.name);
o["returnType"] = lidlTypeExprToQtTypeName(m.returnType);
o["isInvokable"] = true;
QJsonArray params;
for (const ParamDecl& p : m.params) {
QJsonObject po;
po["type"] = lidlTypeExprToQtTypeName(p.type);
po["name"] = qs(p.name);
params.append(po);
}
o["parameters"] = params;
arr.append(o);
}
return arr;
}
// Build the records QJsonArray ({ name, fields:[{name,type,optional}] }) from a
// parsed ModuleDecl — the contract's `type Foo { ... }` declarations, which
// generator_lib turns into structs nested in the wrapper class.
//
// OPTIONALITY SURVIVES HERE, and it is the whole reason this object has three
// keys instead of two. A field has two equivalent spellings — the flag
// (`? name: T`) and the type kind (`name: ?T`) — which logos-lidl's docs/spec.md
// binds to ONE meaning and requires to produce byte-identical code. Flattening
// `fd.type` into a name answered that question two different ways from one
// contract: the flag spelling kept T (so `? maybe: tstr` became a bare `QString`
// that cannot be empty at all, silently defaulting), the type spelling collapsed
// to QVariant. Both answers came from reading the verbatim spelling instead of
// asking.
//
// So: `type` is the value type with optionality stripped (fieldValueType), and
// `optional` is true for either spelling (fieldIsOptional). Those accessors are
// the frontend's, and are the ONLY correct source — `fd.optional` alone and
// `fd.type.kind == Optional` alone are the same bug from opposite sides.
QJsonArray moduleRecordsToJson(const ModuleDecl& mod)
{
QJsonArray arr;
for (const TypeDecl& td : mod.types) {
QJsonObject o;
o["name"] = qs(td.name);
QJsonArray fields;
for (const FieldDecl& fd : td.fields) {
QJsonObject f;
f["name"] = qs(fd.name);
f["type"] = lidlTypeExprToQtTypeName(fieldValueType(fd));
f["optional"] = fieldIsOptional(fd);
fields.append(f);
}
o["fields"] = fields;
arr.append(o);
}
return arr;
}
// Build the events QJsonArray ({ name, params:[{name,type}] }) — same shape
// loadEventsFromLidl produces — from a parsed ModuleDecl.
QJsonArray moduleEventsToJson(const ModuleDecl& mod)
{
QJsonArray arr;
for (const EventDecl& ed : mod.events) {
QJsonObject o;
o["name"] = qs(ed.name);
QJsonArray params;
for (const ParamDecl& pd : ed.params) {
QJsonObject p;
p["name"] = qs(pd.name);
p["type"] = lidlTypeExprToQtTypeName(pd.type);
params.append(p);
}
o["params"] = params;
arr.append(o);
}
return arr;
}