Typer cycles #1422
Typer cycles #1422
Conversation
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/651/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1359/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1359/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/651/ |
| @@ -1357,6 +1357,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => | |||
| if (!isInitialized) info | |||
| this | |||
| } | |||
| def maybeInitialize = { | |||
| try { initialize ; true } | |||
| catch { case _: CyclicReference => debuglog("Hit cycle in maybeInitialize of $this") ; false } | |||
There was a problem hiding this comment.
Would it be useful to also log CyclicReference#{sym, info} here?
There was a problem hiding this comment.
I was logging more, and I reduced it to where it is; I forget what I found out, but I remember it wasn't being useful.
|
I incorporated the modest feedback, modestly. Review bump bump for @adriaanm. |
|
Something to keep in mind - this for any rubber-stamping LGTMers in the neighborhood - is that all of this commit is hidden behind -Ybreak-cycles, so it is not especially threatening. |
|
sorry I've been staring at this but was distracted by changing continents |
| else { | ||
| //Console.println("computing base classes of " + typeSymbol + " at phase " + phase);//DEBUG | ||
| // optimized, since this seems to be performance critical | ||
| val superclazz = tpe.firstParent |
There was a problem hiding this comment.
I prefer tpe.parents.head since it makes clear no parents are lost. It's equivalent because we know tpe.parents.nonEmpty
There was a problem hiding this comment.
also, since parents actually does some work for existentials, maybe it would be cleaner to compute tpe.parents only once
|
LGTM irrespective of my nitpick comments for the record, github gets first prize on confusing diff display (middle-weight category) |
Overcomes cycles encountered during classfile parsing in possibly sketchy fashion. "illegal cyclic reference involving class Foo" is the watchword. See SI-3809.
I threw this in with the previous commit behind -Ybreak-cycles, but
this one is much less sketchy. Explanation: have to handle f-bounds
more deftly. Namers forces lower bounds to prevent recursion in
that direction, but a light touch is required to handle these two
situations differently:
// This is a cyclic type parameter - an error is correct
class A[T <: Comparable[_ <: T]]
// This is not cyclic - it flips the arrow
class B[T <: Comparable[_ >: T]]
Long have I been haunted by the knowledge that you can
write class B in java, but not in scala:
public class B<T extends Comparable<? super T>> {}
It's over! We've achieved parity with java.
|
PLS REBUILD ALL |
|
I rebased and incorporated your nitpicky comments. |
|
(Of course I studiously ignored your substantive comments.) |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/724/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1434/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1434/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/724/ |
No description provided.