Fix #3703: Various fixes to make printing more robust in face of errors #3705
Conversation
|
|
There was a problem hiding this comment.
@odersky Both added test cases now compile on master without the changes in this PR. Do you still want to merge the changes?
|
@allanrenucci Yes, these were some random problems encountered when playing with diagnostics while trying to understand previous bugs. |
| if (ctx.settings.uniqid.value) "#" + pt.hashCode else "" | ||
| if (ctx.settings.uniqid.value) | ||
| try "#" + pt.hashCode | ||
| catch { case ex: NullPointerException => "" } |
There was a problem hiding this comment.
Couldn't we do a null check first instead of catching NullPointerException ? This is fishy in general and is also not something that can be done on Scala.js by default (https://www.scala-js.org/doc/semantics.html)
There was a problem hiding this comment.
No, hashCode is computed, the null could be in a referenced type.
| def refTxt = | ||
| try toTextGlobal(tp.ref) | ||
| catch { | ||
| case ex: Throwable => Str("...") |
There was a problem hiding this comment.
Is this just to catch infinite loops in LazyRefs? If so I'd just catch StackOverflowException, though I think it'd be cleaner to do loop detection in LazyRef#ref.
There was a problem hiding this comment.
Yes, it's to detect stack overflows. Evaluation of LazyRefs is not tail-recursive, so we will always get a stack overflow. Maybe we can do a better job in loop detection but I believe the two issues are separate.
It succeeds now, still worth keeping.
No description provided.