From ff8c3003a494cf01d93d28e030e041e78f3dbbda Mon Sep 17 00:00:00 2001 From: Dario Lipicar Date: Wed, 29 Jul 2026 12:13:30 -0300 Subject: [PATCH] fix(cdylib): typed scalars go through the codec, and the emitted codec checks signedness (#115) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cdylib): typed scalars go through the codec, and the codec checks signedness The cdylib dispatch decoded composites with the generated codec but scalars with a bare nlohmann accessor. Two silent conversions lived in that gap: echoUint(-1) -> 18446744073709551615 (.get() wraps) echoInt(3.7) -> 3 (.get() truncates) The Rust provider rejects both. So a contract both providers share answered differently depending on which one a consumer resolved to, and one of the two answers was a sign flip on a nominal value. The reason this was left in place was circular, and it was written in the source: the leniency "is pinned by the conformance matrix (`hostile/int/fractional` expects 3 from 3.7 on this provider)". Those cells exist to DOCUMENT the divergence — their own `why` text says the strict behaviour is correct. The expectations moved with this change. TWO sites, because fixing one relocates the bug rather than closing it: * jsonArgToStd no longer special-cases int/uint/float64/bool/tstr — everything typed goes through Codec. `any` still passes through, since it declares nothing to check against; bstr keeps its tagged-bytes decoder. * the EMITTED codec (this generator writes its own copy into _types.h, separate from logos_codec.h) gated integers on `is_number()`, which admits floats AND negatives. Routing scalars into it without fixing it would have changed nothing. The integer specializations are now spelled out rather than driven from the scalar table, because a category check is not enough for them. Co-Authored-By: Claude Opus 5 * chore: bump logos-protocol to the signedness + sentinel fixes logos-protocol c0df466 (#31): * Codec checks integer signedness and range, so a negative can no longer wrap into an unsigned and a wide value can no longer truncate. * the pending-call sentinel is matched by shape rather than key presence, so a user map merely carrying that key no longer hangs the call. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- .../experimental/lidl_gen_cdylib.cpp | 72 ++++++++++++++----- flake.lock | 6 +- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/cpp-generator/experimental/lidl_gen_cdylib.cpp b/cpp-generator/experimental/lidl_gen_cdylib.cpp index 62c3d7a..7a6abeb 100644 --- a/cpp-generator/experimental/lidl_gen_cdylib.cpp +++ b/cpp-generator/experimental/lidl_gen_cdylib.cpp @@ -75,24 +75,30 @@ QString lidlTypeToStdCdylib(const TypeExpr& te, const std::set& rec // json arg expression -> std-typed C++ expression // A method argument, decoded into the author's C++ type. // -// Everything that is not a plain scalar goes through the generated codec, which -// recurses — so a bstr keeps its canonical tag at ANY depth and a record -// decodes field by field with a path in the error. The scalars keep their -// nlohmann accessor verbatim: `.get()` TRUNCATES a float rather than -// throwing, and that leniency is pinned by the conformance matrix -// (`hostile/int/fractional` expects 3 from 3.7 on this provider). Routing them -// through the codec would silently change behaviour that something depends on. +// EVERY typed value goes through the generated codec, which recurses — so a bstr +// keeps its canonical tag at ANY depth, a record decodes field by field with a +// path in the error, and a scalar is checked against its declared type. +// +// The scalars used to keep their nlohmann accessor verbatim, and that was the +// last hole in the type contract on this backend: `.get()` on -1 wraps +// to 18446744073709551615 with no exception, so `echoUint(-1)` answered +// 18446744073709551615 here and `dispatch_failed` on the Rust provider — a +// silent sign flip on a nominal type, in a contract both providers share. +// `.get()` on 3.7 likewise truncated to 3 instead of rejecting. +// +// The comment that used to sit here justified the leniency by pointing at the +// conformance matrix cells that pinned it. That was circular: those cells exist +// to DOCUMENT the divergence, and their own `why` text says the strict behaviour +// is the correct one. The expectations moved with this change. +// +// `any` still passes through untouched — it is the one LIDL type that declares +// nothing, so there is nothing to check it against. QString jsonArgToStd(const TypeExpr& te, const QString& expr, const QString& path, const std::set& recs) { if (te.kind == TypeExpr::Primitive) { - if (te.name == "tstr") return expr + ".get()"; - if (te.name == "bstr") return "lidlBytesFromJson(" + expr + ")"; - if (te.name == "int") return expr + ".get()"; - if (te.name == "uint") return expr + ".get()"; - if (te.name == "float64") return expr + ".get()"; - if (te.name == "bool") return expr + ".get()"; - if (te.name == "any") return expr; + if (te.name == "bstr") return "lidlBytesFromJson(" + expr + ")"; + if (te.name == "any") return expr; } const QString cpp = lidlTypeToStdCdylib(te, recs); if (cpp == "LogosMap" || cpp == "LogosList") @@ -201,13 +207,10 @@ void emitGeneratedCodec(QTextStream& s, const ModuleDecl& module, s << " throw std::runtime_error(std::string(\"expected \") + want + \" at \" + path\n"; s << " + \", got \" + std::string(got.type_name()));\n}\n\n"; - // Scalars. Their leniency matches what the dispatch did before the codec - // existed, so behaviour for already-working modules is unchanged. + // Scalars that need no more than a category check. struct Scalar { const char* cpp; const char* want; const char* check; const char* get; }; const Scalar scalars[] = { {"std::string", "string", "is_string()", "get()"}, - {"int64_t", "integer", "is_number()", "get()"}, - {"uint64_t", "integer", "is_number()", "get()"}, {"double", "number", "is_number()", "get()"}, {"bool", "boolean", "is_boolean()", "get()"}, }; @@ -219,6 +222,38 @@ void emitGeneratedCodec(QTextStream& s, const ModuleDecl& module, s << " return j." << sc.get << ";\n }\n};\n\n"; } + // The integers are spelled out rather than driven from the table above, + // because a category check is not enough for them and the shortcuts are + // silent rather than loud: + // + // is_number() admits a FLOAT, and .get() TRUNCATES it — 3.7 + // arrived as 3 instead of being rejected. + // is_number() admits a NEGATIVE, and .get() WRAPS it — -1 + // arrived as 18446744073709551615, a sign flip on a nominal value. + // + // Both used to be pinned as conformance expectations, which made the C++ + // provider disagree with the Rust one (which rejects) on a contract they + // share. Rejecting is the correct half of that disagreement: a value the + // declared type cannot represent must not reach the author wearing another. + s << "template <> struct Codec {\n"; + s << " static nlohmann::json to(const int64_t& v) { return nlohmann::json(v); }\n"; + s << " static int64_t from(const nlohmann::json& j, const std::string& path) {\n"; + s << " if (!j.is_number_integer() && !j.is_number_unsigned())\n"; + s << " lidlTypeError(\"integer\", path, j);\n"; + s << " if (j.is_number_unsigned()\n"; + s << " && j.get() > uint64_t(std::numeric_limits::max()))\n"; + s << " lidlTypeError(\"signed integer in range\", path, j);\n"; + s << " return j.get();\n }\n};\n\n"; + + s << "template <> struct Codec {\n"; + s << " static nlohmann::json to(const uint64_t& v) { return nlohmann::json(v); }\n"; + s << " static uint64_t from(const nlohmann::json& j, const std::string& path) {\n"; + s << " if (!j.is_number_integer() && !j.is_number_unsigned())\n"; + s << " lidlTypeError(\"integer\", path, j);\n"; + s << " if (!j.is_number_unsigned() && j.get() < 0)\n"; + s << " lidlTypeError(\"unsigned integer\", path, j);\n"; + s << " return j.get();\n }\n};\n\n"; + // bstr. The FULL specialization wins over the generic vector rule below, // which is what keeps bytes tagged at every depth instead of being // serialized as a plain array of numbers. @@ -491,6 +526,7 @@ QString lidlMakeTypesHeaderCdylib(const ModuleDecl& module) s << "#pragma once\n"; s << "#include \n"; s << "#include \n"; + s << "#include \n"; // the integer codecs range-check s << "#include \n"; s << "#include \n"; s << "#include \n"; diff --git a/flake.lock b/flake.lock index d5967e7..4b0e5b7 100644 --- a/flake.lock +++ b/flake.lock @@ -55,11 +55,11 @@ ] }, "locked": { - "lastModified": 1785295592, - "narHash": "sha256-CQ7QNiBaEf/s+om7pK9NtmYnZ4oDWHgPwX3muZ5uYdE=", + "lastModified": 1785336618, + "narHash": "sha256-s+3ZB2cVUJe1+dEZzpQlD46w2xZWXu5L4K1v15HNhBk=", "owner": "logos-co", "repo": "logos-protocol", - "rev": "8b8a358c8bfdf92e54c5164fd460c4ab31e2f400", + "rev": "c0df466172497741dfaa25d2e74a98947df60ada", "type": "github" }, "original": {