Skip to content
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

Fix handling of AppliedType aliases in outerPrefix #20190

Merged
merged 3 commits into from
Apr 25, 2024

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Apr 15, 2024

Fixes #20184

The overlooked case was an AppliedType where we don't dealias correctly (i.e. at phase at most erasure).

odersky added 3 commits April 15, 2024 15:36
Fixes scala#10184

The overlooked case was an AppliedType where we don't dealias correctly (i.e.
at phase at most erasure).
@odersky
Copy link
Contributor Author

odersky commented Apr 15, 2024

This also seems to fix the underlying issue with parboiled2, which required a workaround previously.

Copy link
Contributor

@EugeneFlesselle EugeneFlesselle left a comment

Choose a reason for hiding this comment

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

Edited: It's is hard to come up with a simple test case causing the issue with match types, since they get simplified in Typer for now.

trait Color

trait A:
  type C
  type R = C match
    case Color => C

def outer3Local =
  object Inner:
    case class Rgb() extends Color

  val b = new A:
    type C = Inner.Rgb
    // compiles if definition of R is moved here

  new b.R() // Error: b.R is not a class type

This is not the same problem, but I think once it is fixed we would get the current issue in outerPrefix again.

case tpe: TypeProxy =>
outerPrefix(tpe.underlying)
}
atPhaseNoLater(erasurePhase)(outerPrefix(tpe.superType))
Copy link
Contributor

Choose a reason for hiding this comment

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

superType will reduce AppliedTypes but not match types, so I think we could still trigger the issue.

@odersky
Copy link
Contributor Author

odersky commented Apr 24, 2024

@EugeneFlesselle ready to merge?

Copy link
Contributor

@EugeneFlesselle EugeneFlesselle left a comment

Choose a reason for hiding this comment

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

The previous example still fails, having reviewed the spec with sjrd, it is clear that it should indeed fail, for the same reasons as in #20259, even though similar cases with match types pass since they go though simplified.

It actually seems that the spec isn't clear about if type Id[X] = X should be accepted either, it addresses more specifically cases such as type A[X] = Array[X]. Either way, there is no simple was of differentiating them, so I think we can merge this.

@odersky odersky merged commit 912d886 into scala:main Apr 25, 2024
19 checks passed
@odersky odersky deleted the fix-20184 branch April 25, 2024 17:44
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
WojciechMazur added a commit that referenced this pull request Jul 6, 2024
…21082)

Backports #20190 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
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.

-Ycheck:erasure crashes when an alias of an applied type is instantiated
3 participants