Fix generator escaping / name-collision compile-break class - #51
Merged
Merged
Conversation
Several generator paths emitted C++ that silently fails to compile for legal-but-unusual Smithy models, with no generation-time diagnostic (issue #43): - Enum value-set validation message interpolated raw wire values into a string literal; a value containing " or \ broke it. Escape the values in place (byte-identical output for the common safe case) via a new CppLiterals.escapeStringBody. - @pattern was emitted verbatim inside R"__smithy(...)__smithy"; a pattern containing the closing delimiter )__smithy" terminated the raw literal early. Reject such patterns at generation time. - @default and @range int64 minimum emitted -9223372036854775808, which C++ cannot parse (negation of a value one past int64 max). Emit the header-free INT64_MIN idiom via CppLiterals.int64Literal; only the minimum changes, everything else stays byte-identical. - Enum-constant and union-factory name folding could map two distinct members (or a member named "unknown") to one C++ name, producing a duplicate enumerator/method. Detect the collision and fail generation with a diagnostic naming both members and the fix. Tests: codegen unit tests for each — enum-value escaping, raw-string delimiter rejection, enum name-fold collision, the kUnknown-sentinel collision, and the int64-min range/default idiom. Regeneration produces zero golden churn (guards only fire on previously-broken input). Closes #43 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SyQAo21Pv6GYhHrkbQj8xQ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #43 (top-tier from the whole-project review). Four generator paths emitted C++ that silently fails to compile for legal-but-unusual Smithy models, with no generation-time diagnostic.
Fixes
ValidationGenerator.writeEnumCheck); a value containing"or\broke it. Now escaped in place via a newCppLiterals.escapeStringBody— byte-identical output for the common safe case, so no golden churn.@patternraw-string delimiter: the pattern was emitted verbatim insideR"__smithy(...)__smithy", so a value containing the closing sequence)__smithy"terminated the literal early. Rejected at generation time with a message naming the shape and the fix.@defaultand@rangebounds emitted-9223372036854775808, which C++ can't parse (negation of a value one pastint64_tmax). Now emitted via the header-freeINT64_MINidiom (CppLiterals.int64Literal); only the minimum changes, every other magnitude stays byte-identical.unknown) could fold to one C++ name → duplicate enumerator/method. Now detected and failed at generation time with a diagnostic naming both members (or the reservedkUnknown).Testing
@patternas a real regex), enum name-fold collision (foo_barvsfoo__bar, both fold tokFooBar— reachable because Smithy shape IDs are case-insensitive so case-only collisions can't occur), thekUnknown-sentinel collision, and the int64-min@range/@defaultidiom.gradle build spotlessCheckgreen; regeneration produces zero golden diff (git diff --stat -- examples protocol-testsis empty — the guards only fire on previously-broken input).bazel test //protocol-tests/... //examples/cafe/...(the enum/validation paths): 20/20 pass.Checklist
bazel test //...and(cd codegen && gradle build spotlessCheck)pass locally (Beast/benchmark excluded per docs)Note: a compile-the-generated-output test harness — which would catch this whole class structurally rather than case-by-case — is tracked separately in #48.
🤖 Generated with Claude Code
https://claude.ai/code/session_01SyQAo21Pv6GYhHrkbQj8xQ
Generated by Claude Code