mirror of
https://github.com/logos-co/logos-cpp-sdk.git
synced 2026-08-31 17:51:07 +00:00
Dario LipicarandClaude Opus 5
44b92b0480
* feat(parser): name nlohmann::json explicitly, ahead of killing the fallback `nlohmann::json` (and the `json` alias) has never had a branch in cppTypeToLidl. It reaches the opaque `any` the same way every unrecognised spelling does: the fallback at the bottom of the function. That is fine while the fallback is silent and wrong the moment it becomes an error, because `any` is the RIGHT answer here. test_fullapi_cpp declares `nlohmann::json echoAny(const nlohmann::json&)`, `bool fireAnyEvent(const nlohmann::json&)` and `logos_events: void anyEvent(const nlohmann::json&)`, and those three are the cross-language conformance chain's `any` cells — they must keep publishing `any`. So this lands first and on its own. It maps to the bare `any` primitive, which is exactly what the fallback already produced, making the change output-neutral: generating over every impl header and every .lidl in the workspace produces 764 byte-identical artifacts. That is what lets the later commit treat everything still reaching the fallback as a silent admission rather than a legitimate `any`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(cdylib): a typed map decodes into the author's own container `{tstr: T}` has two C++ spellings — std::map<std::string, T> and std::unordered_map<std::string, T> — and logos_codec.h specializes Codec for both, because they are the same wire shape. The generated dispatch named one: lidlImpl().echoIntMap(logos::fromJson<std::map<std::string, int64_t>>(...)) fromJson returns a std::map, and a std::map does not convert to an unordered_map parameter. So the container the author picked decided whether generated code they never wrote compiles — with the diagnostic pointing at that generated line, not at their declaration. logos::JsonArg (logos-protocol, already on master) exists for exactly this: it instantiates its conversion operator with the parameter's own type, so the author's declaration drives the decode. The return side is the same problem mirrored, and `logos::toJson(result)` deduces instead of asserting. Restricted to Map because every other LIDL type has one C++ spelling here, and because JsonArg documents one target it cannot serve — std::optional<X>, whose converting constructor out-ranks the proxy's conversion operator. The Optional branch returns before reaching this code. For a std::map author nothing changes: JsonArg instantiates the same Codec<std::map<...>>::from at the same path, and toJson deduces the same T. Compile-checked against both spellings, including a bad element still failing with `expected string at arg0.k, got number`. Across the whole workspace the emitted delta is 5 methods, all in test_fullapi_ext. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(parser): an unknown C++ spelling is a build error, not a silent `any` cppTypeToLidl ended with `// Fallback: treat as opaque` -> `any`, and `any` is ADMITTED by every backend gate. So a spelling nobody had written a branch for was accepted in silence, published as `any`, and dispatched as a bare `lidlImpl().f(args.at(0))` — no logos::fromJson<>, no check. That is the one hole #113-#122 closed for every typed slot and left open for anything that reached `any`. An 11-method hostile probe was admitted wholesale: uint32_t, size_t, float and uint8_t all typed as `any`; std::set, std::vector<std::pair<...>> and a non-string-keyed map as `any`; std::vector<uint32_t> as `[any]`. Now every spelling with no LIDL type is collected with the declaration that carries it, and parseImplHeader turns the list into a parse error naming the offending type and the fix: method 'send_generic_public_transaction': parameter 'instruction' declared `const std::vector<uint32_t>&`, whose element `uint32_t` has no LIDL type. LIDL numbers are 64-bit only. Declare it `uint64_t` (LIDL `uint`). Widening is source-compatible for every caller; a narrow type on the wire is not, which is why LIDL has none. Numbers get that tailored hint (uint8_t its own — it means bytes here, and only as std::vector<uint8_t>); sets, pairs/tuples, non-string map keys, list/deque/array, Qt types and pointers each get theirs; anything else gets the full table of recognised spellings. A hint that does not name a replacement just moves the guesswork, so all of them do. std::unordered_map<std::string, T> joins std::map as a spelling of `{tstr: T}` — the codec has always handled both, and the previous commit made the generated dispatch bind whichever the author declared. Two slots are the exception and say so: a record FIELD and an event PARAMETER, where the generator writes the spelling out into code the author's own declaration has to match and can only pick one name. Three properties keep this from breaking things it should not: * The MAPPING is unchanged. cppTypeToLidl still returns `any` for an unsupported spelling; only a diagnostic is recorded. A diagnostic that is later withdrawn therefore leaves output byte-identical. * Diagnostics are withdrawn for declarations that never reach the contract — a helper struct dropped by keepOnlyReferencedRecords, a reserved lifecycle hook (onContextReady and friends), a struct with no parsed fields. Publishing is what makes a type a promise. * An empty spelling is not a C++ type, it is this line-based parser failing to find one. It keeps the old behaviour rather than reporting `''`. Also fixes a latent ordering bug found while threading the context through: metadata.json's event parameters were typed BEFORE the header was read, i.e. against whatever g_recordNames the previous module's parse had left behind. They are now read there and typed after scanForRecords, in the same position in module.events as before. Verified by generating over every impl header and every .lidl in the workspace: 31 derived contracts byte-identical, 368 consumer-umbrella files byte-identical under both --api-style qt and --api-style lp, 46 generated types headers byte-identical. Exactly two modules now fail, at exactly the four slots a prior scan identified as silent admissions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(cdylib): a wrong argument count reports invalid_args The arity gate was `if (args.size() < N) return nullptr;`. The Qt glue turns a NULL reply into an empty QVariant, so "you passed 2 of 4 arguments" was indistinguishable from a method that legitimately returned nothing. logos-rust-sdk already ships the other half. src/args.rs::invalid_args is documented "Same code and message as the C++ generated glue" and pinned by a test named invalid_args_shape_matches_cpp — both of which were false: Rust answered a structured object and C++ answered NULL. Checked against the JSON that crate actually emits rather than against its comment, the two are now byte-identical: {"code":"invalid_args","message":"expected 4 arguments, got 2","origin":"my_module"} `expected` counts REQUIRED parameters in both, so a trailing optional does not change it. The guard is emitted only when the method has at least one required parameter. args.size() is unsigned, so `< 0` never fires: a zero-argument method carried a dead branch. The Rust generator skips it for the same reason, so the two now agree on when a check exists at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(cdylib): delete the dead emitted base64 codec #117 replaced the codec the generator emitted into every module with logos-protocol's logos_codec.h, but left the base64 pair it had grown around: * lidlB64Idx + lidlBytesFromJson — 55 emitted lines with NO call site at all. Every byte parameter had already moved to logos::bytesFromJsonLenient. Confirmed across the whole workspace: in 48 generated export TUs, all 48 mentions of lidlBytesFromJson are its own definition line. * lidlB64UrlEncode + lidlBytesToJson — 34 emitted lines that are logos::bytesToJson rewritten, in a translation unit that already includes it through "<module>_types.h". Scalar bstr slots now call logos::bytesToJson. Composite ones ([bstr], {tstr: bstr}, records) have gone through logos::Codec since #117, so this removes the last place a module carried its own copy of an encoder — the arrangement that once let the emitted and canonical halves disagree about padded base64, and that #117's own comment set out to end. hasBytesEventParam goes with it: it existed only to keep the emitted copy from sitting unused in the events sidecar of modules whose events carry no binary data, and the `namespace { }` block it gated is gone too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(doctest): the bstr event marshal is logos::bytesToJson now Commit (D) deleted the dead emitted base64 codec, so the cdylib events sidecar calls logos::bytesToJson -- the one in logos_codec.h, included in the same translation unit -- instead of emitting its own lidlBytesToJson. The doctest still pinned the old spelling and failed on the new output: expected 'args.push_back(lidlBytesToJson(frame));' not found in output The prose around it is unchanged and still correct: the payload is still the canonical {"_bytes": "<base64url>"} form, which is the property that assertion exists to guard (#99). Only the symbol moved. Verified against real generator output rather than by search-and-replace: built the branch generator, ran --backend cdylib over a sensor_module contract with a bstr event param, and read the emitted line: args.push_back(logos::bytesToJson(frame)); Checked the rest of the specs for other stale symbols (lidlStrdup, b64Url, hasBytesEventParam, the old 'return nullptr' arity gate) -- this was the only one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>