Skip to content

fix(generator): C++ names and recursive records in generated clients - #175

Draft
dlipicar wants to merge 4 commits into
masterfrom
fix/client-emitter-names
Draft

dlipicar wants to merge 4 commits into
masterfrom
fix/client-emitter-names

Conversation

@dlipicar

Copy link
Copy Markdown
Contributor

Three pre-existing bugs in the C++ wrappers generated from a LIDL contract. They showed up when I generated a client for every contract in the workspace and compiled it.

  • A parameter with the same name as one of the wrapper's own (timeout_ms, err, callback). It was emitted next to the wrapper's trailing int timeout_ms = 0, so the compiler reported "redefinition of parameter". The workspace case is sdk_test_caller_module.timed_call(sleep_ms, timeout_ms).
  • C++ keywords used as names (class, delete, default). They were emitted verbatim.
  • Recursive records, which failed at compile time in three ways:
    • ? parent: Node became std::optional<Node> inside Node, an incomplete type.
    • next: Loop became a struct that contains itself.
    • A record naming one declared later in the contract became an unknown type.

Names. cpp_identifier.h applies one rule: a clashing name gets _ appended in C++ only.

  • The wire never changes: method names, field keys and event names keep the contract's spelling, and arguments are positional.
  • Names derived from a method or event keep the LIDL name: delete_(), deleteAsync(), onClass().
  • The rule applies to the lp and Qt consumer wrappers, to the shared record helpers, and to the names in the cdylib provider.

Recursive records.

  • A field on a by-value record cycle is held as std::shared_ptr<T>. An empty pointer means absent on the wire.
  • Structs are defined after the records they hold by value, and forward-declared when one names a struct defined later.
  • [Node] and {tstr: Node} are unchanged. On the Qt surface only required cycles are boxed, because optional fields there are already QVariant.

Output stability, over 100 unique contracts (the workspace's .lidl sources plus built contracts):

Tests. tests/generator/test_{reserved_param_names,keyword_names,recursive_records}.cpp generate from small contracts, then compile the output as C++17 and C++20 and round-trip it. Each one fails without its fix. checks.tests passes on master plus this branch (393/393). On feat/peering it passes 404/404 on aarch64-darwin and x86_64-linux.

Not in this PR:

  • Recursive records on the provider side. The author's struct would have to declare the shared_ptr field, and the header-first parser can't read that back. It's documented as a known limitation.
  • The Qt client stubs (lidl_gen_client.cpp, used for --lidl X without --api-style) have the same three bug classes.

Stacking. The commits cherry-pick cleanly onto feat/peering. They conflict with #173 where it threads stdType. That resolution also has to rename records in #173's typed Codec specializations, which git doesn't flag.

🤖 Generated with Claude Code

dlipicar and others added 4 commits September 26, 2026 23:06
Every generated method appends its own `err` and deadline (`timeout_ms` on the
lp surface, `timeout` on Qt), the async entry points a `callback`, and the
bodies declare locals of their own. A contract parameter with one of those
names was emitted verbatim: logos-rust-sdk's sdk_test_caller_module declares
`timed_call(sleep_ms: int, timeout_ms: int)`, and its client failed with
"redefinition of parameter 'timeout_ms'".

A clashing parameter now gets `_` appended in C++ (cpp_identifier.h), never
onto a name the contract already spells. Arguments are positional, so the wire
is unchanged. The cdylib provider's event bodies get the same rule for `args`
and `emitEventImpl_`. A contract without such names emits the same bytes.
tests/generator now compiles what it generates (generated_code.h).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LIDL admits any identifier as a name, so a contract may call a field `class`
or a method `delete`. Every emitter wrote the name verbatim, and the output
did not compile: logos-json-rpc-bridge-py's names.lidl, with fields `class`
and `int`, failed in the lp client, the Qt wrapper and the cdylib provider.

A name that is a C++ keyword (the C++20 list, alternative tokens included, so
the output also builds as C++20) now gets `_` appended wherever it is a C++
identifier: records, fields, methods, parameters, event parameters, and the
provider's method calls and event bodies. The wire keeps every name, and names
derived from one keep it too (`delete` gives `delete_()`, `deleteAsync()`,
`onClass()`). A contract-first provider's author writes the same spellings.
A contract without keyword names emits the same bytes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A record may name itself, and the consumer wrappers held every record field by
value: `Node { ? parent: Node }` became std::optional<Node> inside Node (an
incomplete type), `Loop { next: Loop }` a struct containing itself, and a
record naming one declared after it an unknown type, in declaration order.
logos-json-rpc-bridge-py's recursive.lidl failed on every one, on lp and Qt.
Nothing recursed forever: the codec only follows the data it is given.

A field whose record, held by value, would hold the record it belongs to is
now a std::shared_ptr<T>: empty is absent, on the wire as for an optional. By
value means T, and ?T on lp; Qt spells an optional field QVariant, so only a
required cycle gets a pointer there. std::vector / std::map / QList / QMap of
an incomplete type are fine, so `[Node]` is unchanged. Structs follow the
records they hold by value, and are declared first when one names a record
defined after it. Every other contract emits the same bytes.

The cdylib provider still spells such a field T / std::optional<T> in its
codec; documented as a known limitation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…esult/bin

CI runs the installed generator_tests after `nix build .#tests`, when the
unpacked source dir the probes' include path named is gone, so every probe
failed with "logos_json.h: No such file or directory". nix now passes the
SDK headers as a store path; other builds keep the source dir.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

📊 cpp-sdk doc-test report

The real accounts module, run through a logoscore daemon with the whole stack built against this commit of the C++ SDK — rendered alongside the commands actually run and their output (updated each run, commit 57b6eee):

Pages can take a minute to update after the run finishes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant