Fix compiler crash when a behavior satisfies a non-tag interface method#5191
Draft
SeanTAllen wants to merge 1 commit intomainfrom
Draft
Fix compiler crash when a behavior satisfies a non-tag interface method#5191SeanTAllen wants to merge 1 commit intomainfrom
SeanTAllen wants to merge 1 commit intomainfrom
Conversation
When an actor's `be` is used to satisfy an interface method declared with
a non-tag receiver cap (e.g. `fun box apply`), the reach pass was keying
the new method entry under the interface cap ("box_apply") in
`r_methods`, but later `reach_method` lookups normalize the cap to
`n->cap` — which is `tag` for a behavior — and search for "tag_apply",
finding nothing. `genfun_forward` then asserts on the NULL result.
Normalize to `tag` inside `add_rmethod` for behaviors so the concrete
entry ends up keyed where later lookups expect it. The forwarding entry
in `r_mangled` is independent and keeps its own interface-cap mangled
name.
Member
Author
|
@jemc this PR demonstrates "a solution" to the problem. thoughts on how to address? |
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.
Draft for discussion. This fixes the compiler crash, but I'm not confident it's the best layer for the fix — opening to discuss the approach.
The crash. When an actor's
beis used to satisfy an interface method with a non-tag receiver cap (fun box/fun ref), the reach pass asserts ingenfun_forward. Minimal repro (no lambdas):This is a valid subtype relationship — a
beisfun tag, andbox/refare subcaps oftag, so the contravariant receiver check holds.Root cause.
add_rmethodcreates the concrete method entry inn->r_methodskeyed by the caller-supplied cap (e.g.box_apply). Later,reach_methodlooks up the same method but normalizes the cap ton->cap, which istagfor a behavior — so it searches fortag_applyand finds nothing.genfun_forwardasserts on the NULL result.The fix. Normalize the cap to
taginadd_rmethodwhen the method is a behavior, so the concrete entry lands where later lookups expect it. The forwarding entry inn->r_mangledis independent and keeps its interface-cap mangled name, so forwarding stubs still work.Why I'm not sure this is the right layer. The asymmetry between
r_methodsandr_mangledlookup is the real smell — this fix papers over it at the insertion site. The right fix might instead be inreach_method's lookup normalization, or somewhere earlier. Happy to pivot based on review.Tests. Not added yet — I'd rather wait on approach confirmation before writing a test I'm going to throw away.
Relationship to #2922. This PR does not close #2922. The compiler crash was hiding the original 2018 runtime bug:
{(s: String) => this(s)}compiles (after this fix) but then infinite-recurses at runtime becausethisinside the lambda refers to the lambda object itself, not the enclosing actor. That's a separate, deeper problem.