Change early typeparams2 #1028
Change early typeparams2 #1028
Conversation
|
Fixes #970 and a couple other problems that were discovered after applying the fix. Review by @smarter. Supersedes #1027. Compared to #1027 I dropped unnecessary changes and reordered commits. Btw "Beta reduce #Apply applications over classes" is still needed. IndexedSeq won't compile standalone without it. |
| else tsym.infoOrCompleter match { | ||
| case completer: TypeParamsCompleter => | ||
| val tparams = completer.completerTypeParams(tsym) | ||
| if (tsym.isClass) tparams |
There was a problem hiding this comment.
I think this will always be false since we check if (tsym.isClass) above and go in another branch.
There was a problem hiding this comment.
That is fixed in a subsequent commit.
There was a problem hiding this comment.
In which subsequent commit? I'm commenting on the whole diff here and I still see:
if (tsym.isClass) ...
else tsym.infoOrCompleter match {
case ... =>
val tparams = ...
if (tsym.isClass) ...
Indeed, there's a real issue here, I think the problem is that the parent IndexedSeqFactory {
type scala$collection$generic$IndexedSeqFactory$$CC = scala.collection.immutable.IndexedSeq
}Which is wrong because IndexedSeqFactory {
type scala$collection$generic$IndexedSeqFactory$$CC = [X0] -> scala.collection.immutable.IndexedSeq[X0]
}Instead of fixing this a posteriori by doing a special kind beta-reduction, it seems cleaner to try to eta-expand everything correctly in the first place, I have a branch on top of this branch which reverts the beta-reduction patch and instead changes |
Need to also look info refined types. Need to handle case where we hit a NoCompleter again. Fixes scala#974 and makes MutableSortedSetFactory in stdlib compile.
Through substitutions we might end up with code like
C[hk0 = T] # Apply
where C is a non-lambda class. This case has to be handled specially.
If we don't do that, we get a stackoverflow when compiling IndexedSeq.
Type params should be computed before computing the whole info of a type. Without the patch we get a cyclic reference in the compileMixed test.
I observed in a local partest a file with was a java.io.Path, not an SFile. They should be treated like SFiles. Not clear why this came up. The file in question (partest-generated/pos/Patterns_v1.scala) looked just like all the others that were read as SFiles.
odersky
force-pushed
the
change-early-typeparams2
branch
from
96c6aad to
5936190
Compare
|
Rebased to master |
There was no reason in the first case to cache them, as their equality is reference identity. Maybe this fixes the OOM errors we encounter.
|
@smarter I think the -force branch is much better as a solution if it indeed passes all the tests. Can you make it a PR? |
|
[info] total memory = 910mb So clearly increasing with -J-X had no effect. @DarkDimius please advise how to fix this. Also: is junit, bootstrapped partest and partest all run on the same JVM? That would increase the memory pressure of course. |
OK: #1031
I have an attempt at #1030 |
|
Superseded by #1031 |
No description provided.