-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #1750: Alternative fix for cyclic references due to illegal class overrides #1913
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
trait Lang1 { | ||
trait Exp | ||
trait Visitor { def f(left: Exp): Unit } | ||
class Eval1 extends Visitor { self => | ||
def f(left: Exp) = () | ||
} | ||
} | ||
trait Lang2 extends Lang1 { | ||
class Visitor extends Eval1 { Visitor => // error: classes cannot be overridden | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
trait Lang1 { | ||
trait Exp | ||
trait Visitor { def f(left: Exp): Unit } | ||
class Eval1 extends Visitor { self => | ||
def f(left: Exp) = () | ||
} | ||
} | ||
trait Lang3 { self: Lang1 => | ||
class Visitor extends Eval1 { Visitor => // error: classes cannot be overridden | ||
} | ||
} | ||
trait Lang4 extends Lang1 with Lang3 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,11 +34,6 @@ package p2 { // all being in the same package compiles fine | |
} | ||
} | ||
|
||
abstract class T3 extends T2 { | ||
class A { // error: classes cannot be overridden | ||
bug() | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why remove the class here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It now masks the other errors because the error is raised now in typer, whereas the others would be raised in RefChecks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, it makes sense. |
||
|
||
class A[T] { | ||
|
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.
As a general rule in development, may I understand that
try/catch
can only be used as the last resort? Just want to know.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.
It's complicated. Cyclic references can only be handled with try-catch, because at the points they are raised we typically do not have a way to back out cleanly. So if a cyclic reference is a possibility we need to handle it with try/catch. And we should try to minimize the places where cyclic references can be raised.
I would think twice before we introduce other exceptions and handle them with try/catch.