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

README.md

@@ -344,6 +344,19 @@ so the Qt and Qt-free emitters cannot drift. Anything else stays a value: a
 method may legitimately return a three-string map, and matching the shape alone
 would let user data impersonate a refusal.
 
+**Contract names in C++.** A name that is a C++ keyword, or a parameter named
+like one of the wrapper's own (`err`, `timeout` / `timeout_ms`, `callback`),
+gets a trailing `_` in C++ only: `method delete(class: tstr, timeout_ms: int)`
+is `delete_(const std::string& class_, int64_t timeout_ms_, logos::CallError* err = nullptr, int timeout_ms = 0)`,
+still called `"delete"` on the wire, and its async twins stay `deleteAsync` /
+`deleteAsyncResult`. The rule is in `cpp-generator/docs/project.md` (*C++ names*).
+
+**Recursive records.** A record field that would hold its own record by value
+(`type Node { ? parent: Node }`, or two records holding each other) is a
+`std::shared_ptr<T>` in C++: `node.parent->value`, empty when absent. Lists and
+maps of a record (`children: [Node]`) stay `std::vector` / `std::map`. See
+*Recursive records* in `cpp-generator/docs/project.md`.
+
 ### Universal modules: LogosModuleContext
 
 Universal (codegen-driven) modules — those built from a plain `src/<name>_impl.h` header rather than a handcrafted `QObject` plugin — don't see the raw `LogosAPI` at all. The contract is **derived from that header**: the module's ordinary public methods *are* its API, with no marker of any kind (there used to be a `LOGOS_METHOD` marker under `interface: "provider"`; both are gone). `metadata.json#codegen.impl_class` / `codegen.impl_header` name the class and the header when they differ from the defaults (`<Name>Impl` in `src/<name>_impl.h`). Instead of a `LogosAPI`, the generated C-ABI export TU (`<name>_module_impl.cpp`) populates a narrow `LogosModuleContext` base class with everything an impl typically needs:

cpp-generator/cpp_identifier.h

@@ -0,0 +1,62 @@
+#ifndef CPP_IDENTIFIER_H
+#define CPP_IDENTIFIER_H
+
+// A LIDL name as a C++ identifier. The wire keeps the LIDL spelling; the C++
+// identifier gets `_` appended when it is a C++ keyword or a name the generated
+// code declares in the same scope (`reserved`), until it is neither and no
+// other name of the scope spells it. A usable name never changes, and names
+// derived from one (`<method>Async`, `on<Event>`) keep the LIDL spelling.
+
+#include <QSet>
+#include <QString>
+#include <QStringList>
+
+// C++20, alternative tokens included: generated code must also build as C++20.
+inline bool lidlIsCppKeyword(const QString& name)
+{
+    static const QSet<QString> keywords = {
+        "alignas", "alignof", "and", "and_eq", "asm", "auto", "bitand", "bitor",
+        "bool", "break", "case", "catch", "char", "char8_t", "char16_t", "char32_t",
+        "class", "compl", "concept", "const", "consteval", "constexpr", "constinit",
+        "const_cast", "continue", "co_await", "co_return", "co_yield", "decltype",
+        "default", "delete", "do", "double", "dynamic_cast", "else", "enum",
+        "explicit", "export", "extern", "false", "float", "for", "friend", "goto",
+        "if", "inline", "int", "long", "mutable", "namespace", "new", "noexcept",
+        "not", "not_eq", "nullptr", "operator", "or", "or_eq", "private",
+        "protected", "public", "register", "reinterpret_cast", "requires", "return",
+        "short", "signed", "sizeof", "static", "static_assert", "static_cast",
+        "struct", "switch", "template", "this", "thread_local", "throw", "true",
+        "try", "typedef", "typeid", "typename", "union", "unsigned", "using",
+        "virtual", "void", "volatile", "wchar_t", "while", "xor", "xor_eq",
+    };
+    return keywords.contains(name);
+}
+
+// The C++ spellings of one scope's LIDL names, index-aligned with `names`.
+inline QStringList lidlCppNames(const QStringList& names, const QSet<QString>& reserved = {})
+{
+    auto clashes = [&](const QString& n) { return lidlIsCppKeyword(n) || reserved.contains(n); };
+    QSet<QString> taken;
+    for (const QString& n : names)
+        if (!clashes(n)) taken.insert(n);
+    QStringList out;
+    for (const QString& n : names) {
+        QString cpp = n;
+        if (clashes(cpp)) {
+            cpp += '_';
+            while (clashes(cpp) || taken.contains(cpp)) cpp += '_';
+            taken.insert(cpp);
+        }
+        out << cpp;
+    }
+    return out;
+}
+
+// `name`'s spelling in the scope `scope` (which should contain it).
+inline QString lidlCppName(const QString& name, const QStringList& scope)
+{
+    const int i = scope.indexOf(name);
+    return i < 0 ? lidlCppNames({name}).first() : lidlCppNames(scope).at(i);
+}
+
+#endif // CPP_IDENTIFIER_H

cpp-generator/docs/project.md

@@ -112,6 +112,40 @@ Per surface:
 | legacy consumer, **positional** slot (param, return, event param) | `QVariant` (Qt) / `LogosMap` (Lp) | still flattened — see Known Limitations |
 | header-first (`impl_header_parser`) | `std::optional<T>` ↔ `?T` | `std::optional<std::optional<T>>` has no LIDL type; it collapses to `?T` and is reported on stderr |
 
+### C++ names
+
+A LIDL name becomes a C++ identifier, but the wire keeps it as written: method names,
+record field keys and event names are never renamed. Only the identifier changes, by one
+rule (`cpp_identifier.h`): a name that clashes gets `_` appended until it no longer does
+and no other name of its scope spells it. A name that does not clash never changes.
+
+A **C++ keyword** clashes everywhere (the C++20 list, alternative tokens such as `and`
+included): a record, field, method, parameter or event named `class`, `delete` or
+`default` is `class_`, `delete_`, `default_` in C++. Names derived from a LIDL name keep
+it, so `method delete` gives `delete_()`, `deleteAsync()` and `deleteAsyncResult()`, and
+`event class` gives `onClass()`. A contract-first cdylib provider's structs and impl class
+are the author's, and use the same spellings: a field `class_`, a method `delete_()`, a
+`logos_events:` entry `class_(...)`.
+
+A method **parameter** also clashes with what the wrapper declares in the same scope: its
+own `err` and deadline (`timeout_ms` on lp, `timeout` on Qt), the async `callback`, and the
+locals and members its bodies use. So `method timed(sleep_ms: int, timeout_ms: int)` takes
+`int64_t timeout_ms_` ahead of the wrapper's `int timeout_ms = 0`. A cdylib provider's
+event bodies declare `args` and call `emitEventImpl_`, so its event parameters keep off both.
+
+### Recursive records
+
+`children: [Node]` inside `Node` is fine as it is: `std::vector` / `std::map` (and `QList` /
+`QMap`) take an incomplete type. A record held **by value** cannot contain itself, though:
+`? parent: Node` as `std::optional<Node>` inside `Node`, `next: Loop` inside `Loop`, or two
+records holding each other. A field on such a cycle is a `std::shared_ptr<T>` instead
+(`std::shared_ptr<Node> parent`): empty is absent, and a copied record shares the pointee.
+By value means `T` and, on lp, `?T`; on Qt an optional field is already a `QVariant`, so only
+a required cycle gets a pointer there. The wire is unchanged, and an empty pointer omits its
+key even for a required field (a provider refuses that call). Structs are emitted after the
+records they hold by value, and all are declared first when one names a record defined
+after it. A contract with neither emits what it did.
+
 ### Client stubs (`lidl_gen_client.h/cpp`)
 
 - `lidlMakeHeader(ModuleDecl)` / `lidlMakeSource(ModuleDecl)` — typed `<Module>` client wrapper; each method (and its `…Async` twin) carries a Doxygen `///` comment generated from the method's `description`
@@ -312,6 +346,9 @@ In `tests/generator/`, alongside the wrapper-emitter tests:
 | Test file | What it tests |
 |-----------|---------------|
 | `test_make_umbrella.cpp` | The `LogosModules` aggregate: both dependency forms on both API styles, that every member's type is included, dropped nameless entries, empty deps |
+| `test_reserved_param_names.cpp` | Parameters named like the wrapper's own (`timeout_ms`, `err`, `callback`, …): spelled apart, and the lp wrapper and cdylib provider compile (`generated_code.h` runs this suite's compiler over the output) |
+| `test_keyword_names.cpp` | Records, fields, methods, parameters and events named with C++ keywords: `_` in C++, the LIDL name on the wire; the lp wrapper and the provider compile as C++17 and C++20 |
+| `test_recursive_records.cpp` | Self-, mutually- and forward-referencing records: `std::shared_ptr` on by-value cycles, definition order, forward declarations; the lp wrapper is built and round-trips a tree through the generated codec over a stubbed `lp_*` ABI |
 
 Fixture files in `tests/experimental/fixtures/`:
 - `sample_impl.h` — module with all supported type variations
@@ -351,6 +388,11 @@ Fixture files in `tests/experimental/fixtures/`:
   pins the current behaviour so closing the gap is a deliberate change.
 - **Nesting, map key types and descriptions still do not cross that boundary either** —
   the flag added for optionality is per-field, not a general widening.
+- **A recursive record cannot yet be implemented by a contract-first cdylib provider.** Its
+  structs are the author's, and the generated codec (`emitRecordCodecs`) still spells a
+  by-value cycle `T` / `std::optional<T>`, which no struct can hold. The consumers box it
+  (*Recursive records* above); the provider would need the same convention, and the
+  header-first parser a way to read `std::shared_ptr<T>` back as `T` or `?T`.
 - `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,

cpp-generator/experimental/lidl_gen_cdylib.cpp

@@ -1,4 +1,5 @@
 #include "lidl_gen_cdylib.h"
+#include "cpp_identifier.h"
 #include "lidl_emit_common.h"
 
 #include <QTextStream>
@@ -50,6 +51,31 @@ bool isRecord(const TypeExpr& te, const std::set<std::string>& recs)
     return te.kind == TypeExpr::Named && recs.count(te.name) > 0;
 }
 
+// The C++ name the author gives a record, a field, a method or an event
+// (cpp_identifier.h): a keyword gains `_`. The wire keeps the LIDL name.
+QString recordCppName(const std::string& name, const std::set<std::string>& recs)
+{
+    QStringList scope;
+    for (const std::string& r : recs) scope << qs(r);
+    return lidlCppName(qs(name), scope);
+}
+
+template <class Decls>
+QStringList declCppNames(const Decls& decls)
+{
+    QStringList names;
+    for (const auto& d : decls) names << qs(d.name);
+    return lidlCppNames(names);
+}
+
+template <class Decls>
+QString declCppName(const Decls& decls, const std::string& name)
+{
+    QStringList scope;
+    for (const auto& d : decls) scope << qs(d.name);
+    return lidlCppName(qs(name), scope);
+}
+
 bool typeSupported(const TypeExpr& te, bool isReturn, const std::set<std::string>& recs)
 {
     if (te.kind == TypeExpr::Primitive) {
@@ -267,7 +293,7 @@ QString lidlTypeToStdCdylib(const TypeExpr& te, const std::set<std::string>& rec
 
     // A declared record is its generated struct.
     if (isRecord(te, recs))
-        return qs(te.name);
+        return recordCppName(te.name, recs);
     // Recurse, so [bstr] is std::vector<std::vector<uint8_t>> and {tstr: Blob}
     // is std::map<std::string, Blob>. lidlTypeToStd() would answer QVariantList
     // / QVariantMap here — a Qt name in a Qt-FREE translation unit, which only
@@ -373,13 +399,17 @@ void emitRecordCodecs(QTextStream& s, const ModuleDecl& module,
     s << "namespace logos { namespace detail {\n\n";
     // One specialization per declared record. Field order follows the contract.
     for (const TypeDecl& t : module.types) {
-        const QString name = qs(t.name);
+        const QString name = recordCppName(t.name, recs);
+        // `fn` is the author's member, `key` the field's name on the wire.
+        const QStringList members = declCppNames(t.fields);
         s << "template <> struct Codec<::" << name << ", void> {\n";
         s << "    static nlohmann::json to(const " << name << "& v) {\n";
         s << "        nlohmann::json out = nlohmann::json::object();\n";
-        for (const FieldDecl& f : t.fields) {
+        for (size_t i = 0; i < t.fields.size(); ++i) {
+            const FieldDecl& f = t.fields[i];
             const QString ft = lidlFieldTypeCdylib(f, recs);
-            const QString fn = qs(f.name);
+            const QString fn = members.at(int(i));
+            const QString key = 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
@@ -393,20 +423,22 @@ void emitRecordCodecs(QTextStream& s, const ModuleDecl& module,
                 // 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."
+                s << "            out[\"" << key << "\"] = Codec<" << vt << ">::to(*v."
                   << fn << ");\n";
             } else {
-                s << "        out[\"" << fn << "\"] = Codec<" << ft << ">::to(v."
+                s << "        out[\"" << key << "\"] = 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) {
+        for (size_t i = 0; i < t.fields.size(); ++i) {
+            const FieldDecl& f = t.fields[i];
             const QString ft = lidlFieldTypeCdylib(f, recs);
-            const QString fn = qs(f.name);
+            const QString fn = members.at(int(i));
+            const QString key = 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.
@@ -418,9 +450,9 @@ void emitRecordCodecs(QTextStream& s, const ModuleDecl& module,
             // in a required one Codec<T> still rejects both. One expression,
             // both halves of the rule.
             s << "        out." << fn << " = Codec<" << ft << ">::from(\n";
-            s << "            j.contains(\"" << fn << "\") ? j.at(\"" << fn
+            s << "            j.contains(\"" << key << "\") ? j.at(\"" << key
               << "\") : nlohmann::json(),\n";
-            s << "            path + \"." << fn << "\");\n";
+            s << "            path + \"." << key << "\");\n";
         }
         s << "        return out;\n    }\n};\n\n";
     }
@@ -625,7 +657,7 @@ QString lidlMakeTypesHeaderCdylib(const ModuleDecl& module)
     // declarations, so the codec below can name them in any order.
     if (!module.types.empty()) {
         for (const TypeDecl& t : module.types)
-            s << "struct " << qs(t.name) << ";\n";
+            s << "struct " << recordCppName(t.name, recs) << ";\n";
         s << "\n";
     }
 
@@ -879,7 +911,7 @@ QString lidlMakeModuleImplExports(const ModuleDecl& module,
             s << "        }\n";
             return;
         }
-        QString call = "lidlImpl()." + qs(md.name) + "(";
+        QString call = "lidlImpl()." + declCppName(module.methods, md.name) + "(";
         for (size_t i = 0; i < md.params.size(); ++i) {
             const QString expr = (i < minArgs)
                 ? QString("args.at(%1)").arg(i)
@@ -1154,7 +1186,11 @@ QString lidlMakeEventsSourceCdylib(const ModuleDecl& module,
     // modules whose events carry no binary data.
 
     for (const EventDecl& ed : module.events) {
-        s << "void " << implClass << "::" << ed.name << "(";
+        // The body declares `args` and calls `emitEventImpl_`; parameters keep off both.
+        QStringList pnames;
+        for (const ParamDecl& pd : ed.params) pnames << qs(pd.name);
+        pnames = lidlCppNames(pnames, {"args", "emitEventImpl_"});
+        s << "void " << implClass << "::" << declCppName(module.events, ed.name) << "(";
         for (int i = 0; i < ed.params.size(); ++i) {
             const QString stdType = lidlTypeToStdCdylib(ed.params[i].type, recsEv);
             // Must match the author's declaration in the `logos_events:` block:
@@ -1169,14 +1205,15 @@ QString lidlMakeEventsSourceCdylib(const ModuleDecl& module,
                 || stdType.startsWith("std::optional")
                 || isRecord(ed.params[i].type, recsEv)
                 || stdType == "LogosMap" || stdType == "LogosList")
-                s << "const " << stdType << "& " << ed.params[i].name;
+                s << "const " << stdType << "& " << pnames.at(i);
             else
-                s << stdType << " " << ed.params[i].name;
+                s << stdType << " " << pnames.at(i);
             if (i + 1 < ed.params.size()) s << ", ";
         }
         s << ")\n{\n";
         s << "    nlohmann::json args = nlohmann::json::array();\n";
-        for (const ParamDecl& pd : ed.params) {
+        for (int i = 0; i < ed.params.size(); ++i) {
+            const ParamDecl& pd = ed.params[i];
             const QString evStd = lidlTypeToStdCdylib(pd.type, recsEv);
             // A record or a composite carrying bytes rides the generated codec,
             // exactly like a method return — otherwise an event payload would be
@@ -1192,13 +1229,13 @@ QString lidlMakeEventsSourceCdylib(const ModuleDecl& module,
                     || 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";
+                  << pnames.at(i) << "));\n";
                 continue;
             }
             if (pd.type.kind == TypeExpr::Primitive && pd.type.name == "bstr")
-                s << "    args.push_back(logos::bytesToJson(" << pd.name << "));\n";
+                s << "    args.push_back(logos::bytesToJson(" << pnames.at(i) << "));\n";
             else
-                s << "    args.push_back(" << pd.name << ");\n";
+                s << "    args.push_back(" << pnames.at(i) << ");\n";
         }
         s << "    emitEventImpl_(\"" << ed.name << "\", &args);\n";
         s << "}\n\n";