Skip to content

codegen: More robustness for invalid :invoke IR#58631

Merged
Keno merged 1 commit intomasterfrom
kf/codegenbadinvoke
Jun 4, 2025
Merged

codegen: More robustness for invalid :invoke IR#58631
Keno merged 1 commit intomasterfrom
kf/codegenbadinvoke

Conversation

@Keno
Copy link
Copy Markdown
Member

@Keno Keno commented Jun 4, 2025

See #58429 for general discussion of these kinds of errors. We don't verify this one in the IR verifier, because it's currently not really part of the structural invariants of the IR. One could imagine using IRCode with a non-codegen backend that uses something else here. Nevertheless, codegen needs to complain of course if someone put something here it doesn't understand.

See #58429 for general discussion of these kinds of errors.
We don't verify this one in the IR verifier, because it's currently
not really part of the structural invariants of the IR. One could
imagine using IRCode with a non-codegen backend that uses something
else here. Nevertheless, codegen needs to complain of course if
someone put something here it doesn't understand.
assert(jl_is_code_instance(ci));
mi = jl_get_ci_mi((jl_code_instance_t*)ci);
} else {
emit_error(ctx, "(Internal ERROR - IR Validity): Invoke target is not a method instance or code instance");
Copy link
Copy Markdown
Member

@topolarity topolarity Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably panic, instead of being an error

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, panics are annoying because they delete all your state, so you can't introspect what went wrong. Obviously continuing to run after this is undefined behavior, but at least you can maybe look at a thing or two at the REPL.

@Keno Keno merged commit edb2aa8 into master Jun 4, 2025
8 checks passed
@Keno Keno deleted the kf/codegenbadinvoke branch June 4, 2025 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants