fix: ICE when emitting #[event] structs with many fields#1516
Open
cburgdorf wants to merge 2 commits into
Open
fix: ICE when emitting #[event] structs with many fields#1516cburgdorf wants to merge 2 commits into
cburgdorf wants to merge 2 commits into
Conversation
Events with 8 or more total fields crashed MIR lowering with "semantic const should reify for runtime lowering". Two bugs combined: 1. The #[event] desugar hashes the signature via core::keccak over a tuple of 2*fields+2 string fragments, but the AsBytes tuple impls stop at arity 16, so the TOPIC0 const never evaluated. 2. The missing impl should have been a type error, but the final obligation pass discharged unsatisfied goals containing string literal types as "not concrete": string literals carry a string-sorted inference variable until the String<N> fallback is applied, so the diagnostic was silently suppressed and the unevaluated const reached MIR. Fixes: - hir: nest the TOPIC0 keccak tuple into chunks of at most 16 elements when the signature is longer. Concatenation is associative, so the hash is byte-identical and signatures of any length compile; tuples with <= 16 elements stay flat, leaving existing events unchanged. - core: raise the AbiSize/Encode/EventAbiEncode tuple impls from arity 12 to 16, bounding events at 16 data (non-indexed) fields. Nesting is not an option for the payload since ABI offsets are frame-relative. Exceeding the bound now reports a proper trait-bound diagnostic spanning the event definition. - ty_check: fold obligation goals through Prober (literal fallbacks) at the final pass so unsatisfied bounds that mention string literals are reported instead of silently discharged. - mir: the reify panic now names the const and expected type as a defensive backstop. Tests: fe_test fixture covering 6/7/8/16 data fields (with and without an indexed field) emitted via log.emit, with every TOPIC0 asserted against externally computed keccak256 values; desugar snapshot for the nested tuple; uitests for the two new diagnostics.
…tting, unneeded_wildcard_pattern) Mechanical fixes so `cargo clippy --workspace --all-targets --all-features -- -D clippy::all` passes on the current toolchain. No behavior changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JT7ontKABrttandXgZy5Xn
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
1 participant
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.
Events with 8 or more total fields crashed MIR lowering with "semantic const should reify for runtime lowering". Two bugs combined:
The #[event] desugar hashes the signature via core::keccak over a tuple of 2*fields+2 string fragments, but the AsBytes tuple impls stop at arity 16, so the TOPIC0 const never evaluated.
The missing impl should have been a type error, but the final obligation pass discharged unsatisfied goals containing string literal types as "not concrete": string literals carry a string-sorted inference variable until the String fallback is applied, so the diagnostic was silently suppressed and the unevaluated const reached MIR.
Fixes:
hir: nest the TOPIC0 keccak tuple into chunks of at most 16 elements when the signature is longer. Concatenation is associative, so the hash is byte-identical and signatures of any length compile; tuples with <= 16 elements stay flat, leaving existing events unchanged.
core: raise the AbiSize/Encode/EventAbiEncode tuple impls from arity 12 to 16, bounding events at 16 data (non-indexed) fields. Nesting is not an option for the payload since ABI offsets are frame-relative. Exceeding the bound now reports a proper trait-bound diagnostic spanning the event definition.
ty_check: fold obligation goals through Prober (literal fallbacks) at the final pass so unsatisfied bounds that mention string literals are reported instead of silently discharged.
mir: the reify panic now names the const and expected type as a defensive backstop.
Tests: fe_test fixture covering 6/7/8/16 data fields (with and without an indexed field) emitted via log.emit, with every TOPIC0 asserted against externally computed keccak256 values; desugar snapshot for the nested tuple; uitests for the two new diagnostics.