Disallow implicit composed-base upcasts; require explicit cast#1205
Merged
Conversation
A struct that composes another struct as its first field was implicitly convertible to that base (and to implemented interfaces) in assignments, arguments and returns. This conversion was silent and layout-dependent: when the struct also implements an interface, a vtable pointer is placed in front of the composed base, so the base no longer sits at offset 0 and the implicitly emitted identity pointer copy was wrong. Disallow the implicit struct-to-composed-base upcast entirely (interface upcasts are unaffected). Instead, allow the explicit conversion 'cast<Base*>(derived)' / 'cast<Interface*>(struct)' in a safe (non-unsafe) context for the upcast direction only. The IR generator advances the pointer to the embedded subobject via GEPs, following the first-field composition chain transitively and correctly skipping any vtable prefix. Down/sibling pointer casts still require an unsafe block. Migrate the bootstrap compiler (parser, block allocator, generic type) to the explicit cast / explicit composed-field access accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
What changed
cast<Base*>(derived)/cast<Interface*>(struct)in a safe (non-unsafe) context, but only for the upcast direction. The IR generator advances the pointer to the embedded subobject via GEPs, following the first-field composition chain transitively and correctly skipping any vtable prefix added by an implemented interface. Down/sibling pointer casts still requireunsafe.parser, block allocator, generic type) to the explicit cast / explicit composed-field access.Why
The implicit composed-base upcast was silent and layout-dependent. When a struct also implements an interface, a vtable pointer is placed in front of the composed base, so the base no longer sits at offset 0 — the implicitly emitted identity pointer copy then aliased the vtable region (garbage reads / segfault). Making the conversion explicit keeps pointer identity visible, and routing it through
cast<>lets the compiler emit the correct, vtable-aware pointer adjustment.How it was validated
spicetest --gtest_filter='TypeCheckerTests.*:IRGeneratorTests.*:BootstrapCompilerTests.*'— all pass except the pre-existingBootstrapCompilerTests.standaloneCommonUtilTest(local version/build-string golden mismatch, unrelated).irgenerator/structs/success-explicit-composed-base-cast(cout + IR snapshot) verifies the GEP adjustments: single-level cast skips the vtable (field index 1), two-level cast emits two GEPs (Leaf -> Derived -> Base), and an interface cast stays at offset 0.typechecker/structs/error-no-implicit-composed-base-upcastasserts the implicit upcast is rejected.unsafe.Follow-up / known limitations
unsafeby design (no runtime check).commonUtilbootstrap test failure is pre-existing and environmental.🤖 Generated with Claude Code