-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 #5044: guard against forward references in the TypeTree of New trees #5178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Have an awesome day! ☀️
Should the following compile?
Scalac doesn't think so. |
checkUndesiredProperties(sym, tree.pos) | ||
currentLevel.enterReference(sym, tree.pos) | ||
tpe.dealias.foreachPart { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is dealias
required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without it the second test still fails (crashes). If you would dealias a bit sooner I think def foo = { type T = Foo; val a = new T; class Foo }
would also emit an error, like in scalac.
tests/neg/i5044b.scala
Outdated
@@ -0,0 +1,8 @@ | |||
class I0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would merge the two tests together:
class I0 {
class I1
def test0 = {
val x = new y.I1 // error: `y` is a forward reference extending over the definition of `x`
val y = new I0
}
def test1 = {
type T = y.I1
val x = new T // error: `y` is a forward reference extending over the definition of `x`
val y = new I0
}
}
tests/neg/i5044b.scala
Outdated
type T = i5.I1 | ||
val i4 = new T // error: `i5` is a forward reference extending over the definition of `i4` | ||
val i5 = new I0 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is another test with a type lambda
class I0 {
class I2[T1, T2]
def test2 = {
type A[T] = i5.I2[T, String]
val i4 = new A[Int]
val i5 = new I0
}
}
This one currently crashes with Scala 2.12.6 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also add:
def test3 = {
val x = new T // error: `T` is a forward reference extending over the definition of `x`
val y = new I0
type T = y.I1
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @Jasper-M
The extra dealiasing step also avoids this scalac bug.