Add a check for correct Array shape in quotes.reflect.ClassOfConstant #22033
Conversation
This file contains 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
hamzaremmal
reviewed
Jan 9, 2025
Comment on lines
2526
to
2528
| typeRepr match | ||
| case AppliedType(_, Nil) => isArray | ||
| case _ => isArray |
There was a problem hiding this comment.
Why a pattern matching if both branches have a similar result ?
There was a problem hiding this comment.
I forgot about case AppliedType(_, _) where we would not check isArray. I've adjusted this to be clearer anyway.
jchyb
force-pushed
the
fix-i21916-array-check
branch
from
164a8ed to
80ae408
Compare
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.
Closes #21916
I tried to supply the ClassOfConstant with multiple other broken Types, but I was unable to break it beyond the linked issue, so I ended up adding the check for only that one case. This makes sense - the backend (and thus erasure) needs to know if the Array type parameter is a primitive type, but in other cases the erasure phase needs to know only the class, without the type parameters.
It's impossible to call classOf through the quoted code (
'{classOf[t]}with a boundless t will error out), so we don't need that additional check there.There does appear to be an issue with being able to set
'{List[Array]}resulting in a crash, but that is beyond the scope of this fix - I will prepare a separate issue for that (edit: reported here).