Skip to content

Guard against invalid prefixes in argForParam #23508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jul 11, 2025

Fixes #23504

@odersky
Copy link
Contributor Author

odersky commented Jul 11, 2025

@jchyb Best effort from tasty fails here. The problem is that there is a deeply wrong condition in basic types which leads to a TypeError being thrown. During normal compilation the TypeError is caught and handled, but it seems duing best effort from tasty it is not. Should this handling be added to the fromTasty compiler and/or tests?

@jchyb
Copy link
Contributor

jchyb commented Jul 11, 2025

@odersky Thank you for letting me know. I suspect I will have to catch and wrap the type with PreviousErrorType in best-effort-tasty unpickling (but I'll also look for a more elegant solution for that).
I think it's also fine to add newly added tests to the exclude list: compiler/test/dotc/neg-best-effort-unpickling.excludelist, including here, since I don't want the compiler development to be slowed down with the experimental feature.

@som-snytt
Copy link
Contributor

@jchyb I took that expedient 31c5b75 thinking I'd learn more and get back to it. Interesting stuff.

We should come back to this and allow this test by catching the TypeError
and reporting it.
@odersky
Copy link
Contributor Author

odersky commented Jul 11, 2025

@jchyb Thanks for the tip. I saw there are quite a few tests in the exclude list udne "cyclic reference crash". maybe these can be handled the same way? After all the CyclicReference exception is a subclass of TypeError.

@odersky
Copy link
Contributor Author

odersky commented Jul 12, 2025

It looks like there are many situations where an illegal higher-kinded type in an argument for
a value type parameter causes illegal TermRefs and TypeRefs to be constructed, leading to an
assertion error. We now turn the assertion error into a specialized exception which eventually
leads to a TypeError being thrown.

The root problem is that we cannot detect illegal kinds in arguments early enough to prevent these
situations (it would lead to spurious cyclic references). We do detect them later, but the damage can already be done before that.

It looks like there are many situations where an illegal higher-kinded type in an argument for
a value type parameter causes illegal TermRefs and TypeRefs to be constructed, leading to an
assertion error. We now turn the assertion error into a specialized exception which eventually
leads to a TypeError being thrown.

The problem is we cannot detect illegal kinds in arguments early enough to prevent these
situations. We do detect them later, but the damage can already be done before that.
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.

Compiler crashes with "assertion failed: invalid prefix HKTypeLambda(...)"
3 participants