-
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
Make PC more resilient to crashes #19488
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.
LGTM! Would be great to set up some kind of linter to report any methods that could throw,
Could you port that to Metals also? |
Why are these typed trees, if they don't have a type? That's half the reason to have the whole typed and untyped trees modelling, so we can define the implementation once and not have to allocate, but define APIs based on whether they're statically known to be typed or not. Some part of the code path must be casting the trees unsafely, somewhere. But even more importantly, I don't think we should be wrapping the call to |
Remember that what we're working is usually trees recovered from error like: class Wrapper(n: Int):
extension (x: Int)
def + (y: Int) = Wrap@@per(x) + y with its tree after the typer: package <empty> {
import scala.collection.immutable.ListSet
class Wrapper(n: Int) extends Object() {
private[this] val n: Int
extension (x: Int) def +(y: Int):
<error Overloaded or recursive method + needs return type>
= Wrapper(x).+(y)
}
} Even tho it is a typed tree, at cursor position, its type is null. I don't know whether this is a bug in compiler, but in my opinion change to typeOpt is something that should help in these cases. As of adding try clause, it also slipped through my mind that this is not the best idea. As presentation compiler has raports system, we could use it to notify whether this happens. |
Backports #19488 to the LTS branch. PR submitted by the release tooling. [skip ci]
When working with code that is recovered from errors, we should use
typeOpt
instead oftpe
as it can sometimes be null. The added test cases were the exact snippet that previously failed. I took this opportunity to change all calls totpe
totypeOpt
.