From aceb060f3de753575faabd7916a1224362455272 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 15 Jan 2016 16:48:29 +0100 Subject: [PATCH 1/7] Fix checkNonCyclic. Need to also look info refined types. Need to handle case where we hit a NoCompleter again. Fixes #974 and makes MutableSortedSetFactory in stdlib compile. --- src/dotty/tools/dotc/typer/Checking.scala | 12 ++++++------ test/dotc/scala-collections.whitelist | 2 +- test/dotc/tests.scala | 1 + tests/neg/i974.scala | 8 ++++++++ tests/pos/i974.scala | 2 ++ 5 files changed, 18 insertions(+), 7 deletions(-) create mode 100644 tests/neg/i974.scala create mode 100644 tests/pos/i974.scala diff --git a/src/dotty/tools/dotc/typer/Checking.scala b/src/dotty/tools/dotc/typer/Checking.scala index 57032c4d9ecb..6ded7c10959a 100644 --- a/src/dotty/tools/dotc/typer/Checking.scala +++ b/src/dotty/tools/dotc/typer/Checking.scala @@ -138,9 +138,7 @@ object Checking { case SuperType(thistp, _) => isInteresting(thistp) case AndType(tp1, tp2) => isInteresting(tp1) || isInteresting(tp2) case OrType(tp1, tp2) => isInteresting(tp1) && isInteresting(tp2) - case _: RefinedType => false - // Note: it's important not to visit parents of RefinedTypes, - // since otherwise spurious #Apply projections might be inserted. + case _: RefinedType => true case _ => false } // If prefix is interesting, check info of typeref recursively, marking the referred symbol @@ -148,10 +146,12 @@ object Checking { // is hit again. Without this precaution we could stackoverflow here. if (isInteresting(pre)) { val info = tp.info - val symInfo = tp.symbol.info - if (tp.symbol.exists) tp.symbol.info = SymDenotations.NoCompleter + val sym = tp.symbol + if (sym.infoOrCompleter == SymDenotations.NoCompleter) throw CyclicReference(sym) + val symInfo = sym.info + if (sym.exists) sym.info = SymDenotations.NoCompleter try checkInfo(info) - finally if (tp.symbol.exists) tp.symbol.info = symInfo + finally if (sym.exists) sym.info = symInfo } tp } catch { diff --git a/test/dotc/scala-collections.whitelist b/test/dotc/scala-collections.whitelist index 2abb16c1ec20..f6a13cc72926 100644 --- a/test/dotc/scala-collections.whitelist +++ b/test/dotc/scala-collections.whitelist @@ -266,7 +266,7 @@ ./scala-scala/src/library/scala/collection/generic/ParFactory.scala # https://github.com/lampepfl/dotty/issues/974 -> @smarter -#./scala-scala/src/library/scala/collection/generic/MutableSortedSetFactory.scala +./scala-scala/src/library/scala/collection/generic/MutableSortedSetFactory.scala # cyclic reference, maybe related to #974 -> @smarter #./scala-scala/src/library/scala/collection/generic/ParSetFactory.scala diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 2810a8b551e9..b61783b0b442 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -158,6 +158,7 @@ class tests extends CompilerTest { @Test def neg_finalSealed = compileFile(negDir, "final-sealed", xerrors = 2) @Test def neg_i705 = compileFile(negDir, "i705-inner-value-class", xerrors = 7) @Test def neg_i866 = compileFile(negDir, "i866", xerrors = 2) + @Test def neg_i974 = compileFile(negDir, "i974", xerrors = 2) @Test def neg_moduleSubtyping = compileFile(negDir, "moduleSubtyping", xerrors = 4) @Test def neg_escapingRefs = compileFile(negDir, "escapingRefs", xerrors = 2) @Test def neg_instantiateAbstract = compileFile(negDir, "instantiateAbstract", xerrors = 8) diff --git a/tests/neg/i974.scala b/tests/neg/i974.scala new file mode 100644 index 000000000000..89db4b2d9339 --- /dev/null +++ b/tests/neg/i974.scala @@ -0,0 +1,8 @@ +trait Foo[T <: Bar[T]#Elem] // error: illegal cyclic reference +trait Bar[T] { + type Elem = T +} +trait Foo2[T <: Bar2[T]#Elem] // error: illegal cyclic reference +trait Bar2[T] { + type Elem = T +} diff --git a/tests/pos/i974.scala b/tests/pos/i974.scala new file mode 100644 index 000000000000..4c7c15e8d856 --- /dev/null +++ b/tests/pos/i974.scala @@ -0,0 +1,2 @@ +class Foo[A] +class Bar[CC[X] <: Foo[CC[X]]] From a07dda6b7ae11bd9bc8e354b1536d23a03aa798b Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 15 Jan 2016 14:07:48 +0100 Subject: [PATCH 2/7] Beta reduce #Apply applications over classes 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. --- src/dotty/tools/dotc/core/Types.scala | 30 ++++++++++++++++++++++++++- test/dotc/tests.scala | 2 ++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 45897dd643ad..497d2494c242 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -942,7 +942,35 @@ object Types { NoType } - loop(this) + /** Reduce C[hki = Argi] # Apply where C is a non-lambda class + * to C[Argi], renaming references as necessary */ + def betaReduceCls(cls: Symbol, tp: Type): Type = tp.stripTypeVar match { + case tp @ RefinedType(parent, rname) => + val parent1 = betaReduceCls(cls, parent) + def paramName(hkArgName: TypeName): TypeName = + cls.typeParams.apply(rname.hkArgIndex).name + def mapArg(rt: RefinedType) = new TypeMap { + def apply(t: Type): Type = t match { + case TypeRef(RefinedThis(`tp`), name) if name.isHkArgName => + TypeRef(RefinedThis(rt), paramName(name)) + case _ => + mapOver(t) + } + } + if (rname.isHkArgName) + RefinedType(parent1, paramName(rname.asTypeName), mapArg(_)(tp.refinedInfo)) + else parent1 + case _ => + tp + } + + if (name == tpnme.hkApply) { + val cref = underlyingClassRef(refinementOK = true) + val cls = cref.typeSymbol + if (cref.isEtaExpandable && !defn.isBottomClass(cls)) betaReduceCls(cls, this) + else loop(this) + } + else loop(this) } /** The type , reduced if possible */ diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index b61783b0b442..2be88a9fad26 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -186,6 +186,8 @@ class tests extends CompilerTest { .toList @Test def compileStdLib = compileList("compileStdLib", stdlibFiles, "-migration" :: scala2mode) + @Test def compileIndexedSeq = compileLine("./scala-scala/src/library/scala/collection/immutable/IndexedSeq.scala") + @Test def dotty = compileDir(dottyDir, ".", List("-deep", "-Ycheck-reentrant"))(allowDeepSubtypes) // note the -deep argument @Test def dotc_ast = compileDir(dotcDir, "ast") From e7964f8a74587b1916ae7b06b482688be7d20db4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 15 Jan 2016 17:39:15 +0100 Subject: [PATCH 3/7] Compute type params in namer without completing the whole info 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. --- .../tools/dotc/core/SymDenotations.scala | 8 +++++ .../tools/dotc/core/TypeApplications.scala | 25 ++++++++++------ src/dotty/tools/dotc/core/TypeOps.scala | 2 +- src/dotty/tools/dotc/typer/Namer.scala | 29 +++++++++++++++---- test/dotc/tests.scala | 7 +++++ tests/pos/B.scala | 6 ++++ 6 files changed, 62 insertions(+), 15 deletions(-) create mode 100644 tests/pos/B.scala diff --git a/src/dotty/tools/dotc/core/SymDenotations.scala b/src/dotty/tools/dotc/core/SymDenotations.scala index 8016c57f3e6c..d384bd5c4951 100644 --- a/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1802,6 +1802,14 @@ object SymDenotations { def withModuleClass(moduleClassFn: Context => Symbol): this.type = { myModuleClassFn = moduleClassFn; this } } + /** A subclass of LazyTypes where type parameters can be completed independently of + * the info. + */ + abstract class TypeParamsCompleter extends LazyType { + /** The type parameters computed by the completer before completion has finished */ + def completerTypeParams(sym: Symbol): List[TypeSymbol] + } + val NoSymbolFn = (ctx: Context) => NoSymbol /** A missing completer */ diff --git a/src/dotty/tools/dotc/core/TypeApplications.scala b/src/dotty/tools/dotc/core/TypeApplications.scala index aab327ce9a21..e09328205028 100644 --- a/src/dotty/tools/dotc/core/TypeApplications.scala +++ b/src/dotty/tools/dotc/core/TypeApplications.scala @@ -4,6 +4,7 @@ package core import Types._ import Contexts._ import Symbols._ +import SymDenotations.TypeParamsCompleter import Decorators._ import util.Stats._ import util.common._ @@ -240,15 +241,21 @@ class TypeApplications(val self: Type) extends AnyVal { case self: TypeRef => val tsym = self.symbol if (tsym.isClass) tsym.typeParams - else if (tsym.isAliasType) self.underlying.typeParams - else if (tsym.isCompleting) - // We are facing a problem when computing the type parameters of an uncompleted - // abstract type. We can't access the bounds of the symbol yet because that - // would cause a cause a cyclic reference. So we return `Nil` instead - // and try to make up for it later. The acrobatics in Scala2Unpicker#readType - // for reading a TypeRef show what's neeed. - Nil - else tsym.info.typeParams + else tsym.infoOrCompleter match { + case completer: TypeParamsCompleter => + val tparams = completer.completerTypeParams(tsym) + if (tsym.isClass) tparams + else defn.LambdaTrait(tparams.map(_.variance)).typeParams + case _ => + if (!tsym.isCompleting || tsym.isAliasType) tsym.info.typeParams + else + // We are facing a problem when computing the type parameters of an uncompleted + // abstract type. We can't access the bounds of the symbol yet because that + // would cause a cause a cyclic reference. So we return `Nil` instead + // and try to make up for it later. The acrobatics in Scala2Unpicker#readType + // for reading a TypeRef show what's needed. + Nil + } case self: RefinedType => // inlined and optimized version of // val sym = self.LambdaTrait diff --git a/src/dotty/tools/dotc/core/TypeOps.scala b/src/dotty/tools/dotc/core/TypeOps.scala index 04cd2249dbae..6896468ba884 100644 --- a/src/dotty/tools/dotc/core/TypeOps.scala +++ b/src/dotty/tools/dotc/core/TypeOps.scala @@ -35,7 +35,7 @@ trait TypeOps { this: Context => // TODO: Make standalone object. * Instead we produce an annotated type that marks the prefix as unsafe: * * (x: (C @ UnsafeNonvariant)#T)C#T - + * We also set a global state flag `unsafeNonvariant` to the current run. * When typing a Select node, typer will check that flag, and if it * points to the current run will scan the result type of the select for diff --git a/src/dotty/tools/dotc/typer/Namer.scala b/src/dotty/tools/dotc/typer/Namer.scala index b24916be8005..406e7378f218 100644 --- a/src/dotty/tools/dotc/typer/Namer.scala +++ b/src/dotty/tools/dotc/typer/Namer.scala @@ -478,10 +478,31 @@ class Namer { typer: Typer => } /** The completer of a symbol defined by a member def or import (except ClassSymbols) */ - class Completer(val original: Tree)(implicit ctx: Context) extends LazyType { + class Completer(val original: Tree)(implicit ctx: Context) extends TypeParamsCompleter { protected def localContext(owner: Symbol) = ctx.fresh.setOwner(owner).setTree(original) + private var myTypeParams: List[TypeSymbol] = null + private var nestedCtx: Context = null + + def completerTypeParams(sym: Symbol): List[TypeSymbol] = { + if (myTypeParams == null) { + //println(i"completing type params of $sym in ${sym.owner}") + myTypeParams = original match { + case tdef: TypeDef => + nestedCtx = localContext(sym).setNewScope + locally { + implicit val ctx: Context = nestedCtx + completeParams(tdef.tparams) + tdef.tparams.map(symbolOfTree(_).asType) + } + case _ => + Nil + } + } + myTypeParams + } + private def typeSig(sym: Symbol): Type = original match { case original: ValDef => if (sym is Module) moduleValSig(sym) @@ -492,7 +513,7 @@ class Namer { typer: Typer => typer1.defDefSig(original, sym)(localContext(sym).setTyper(typer1)) case original: TypeDef => assert(!original.isClassDef) - typeDefSig(original, sym)(localContext(sym).setNewScope) + typeDefSig(original, sym, completerTypeParams(sym))(nestedCtx) case imp: Import => try { val expr1 = typedAheadExpr(imp.expr, AnySelectionProto) @@ -840,9 +861,7 @@ class Namer { typer: Typer => else valOrDefDefSig(ddef, sym, typeParams, paramSymss, wrapMethType) } - def typeDefSig(tdef: TypeDef, sym: Symbol)(implicit ctx: Context): Type = { - completeParams(tdef.tparams) - val tparamSyms = tdef.tparams map symbolOfTree + def typeDefSig(tdef: TypeDef, sym: Symbol, tparamSyms: List[TypeSymbol])(implicit ctx: Context): Type = { val isDerived = tdef.rhs.isInstanceOf[untpd.DerivedTypeTree] //val toParameterize = tparamSyms.nonEmpty && !isDerived //val needsLambda = sym.allOverriddenSymbols.exists(_ is HigherKinded) && !isDerived diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 2be88a9fad26..354bc62d20b5 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -186,6 +186,13 @@ class tests extends CompilerTest { .toList @Test def compileStdLib = compileList("compileStdLib", stdlibFiles, "-migration" :: scala2mode) + @Test def compileMixed = compileLine( + """tests/pos/B.scala + |./scala-scala/src/library/scala/collection/immutable/Seq.scala + |./scala-scala/src/library/scala/package.scala + |./scala-scala/src/library/scala/collection/GenSeqLike.scala + |./scala-scala/src/library/scala/collection/SeqLike.scala + |./scala-scala/src/library/scala/collection/generic/GenSeqFactory.scala""".stripMargin) @Test def compileIndexedSeq = compileLine("./scala-scala/src/library/scala/collection/immutable/IndexedSeq.scala") @Test def dotty = compileDir(dottyDir, ".", List("-deep", "-Ycheck-reentrant"))(allowDeepSubtypes) // note the -deep argument diff --git a/tests/pos/B.scala b/tests/pos/B.scala new file mode 100644 index 000000000000..7e4f26e7de88 --- /dev/null +++ b/tests/pos/B.scala @@ -0,0 +1,6 @@ +object B{ + def main(args: Array[String]): Unit = { + val s = List(1,2,3) + () + } +} From 1d63b78b369e5e9c38d69dfe6961ed4c4fc7470f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 12 Jan 2016 19:16:01 +0100 Subject: [PATCH 4/7] Survive files that are not SFiles in CompilerTest 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. --- test/test/CompilerTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test/CompilerTest.scala b/test/test/CompilerTest.scala index 09b608f22e1a..c65710e7d718 100644 --- a/test/test/CompilerTest.scala +++ b/test/test/CompilerTest.scala @@ -221,8 +221,8 @@ abstract class CompilerTest extends DottyTest { case ExistsSame => // nothing else to do case ExistsDifferent => val nextDest = dest.parent / (dest match { - case f: SFile => SFile(replaceVersion(f.stripExtension, nr)).addExtension(f.extension) case d: Directory => Directory(replaceVersion(d.name, nr)) + case f => SFile(replaceVersion(f.stripExtension, nr)).addExtension(f.extension) }) computeDestAndCopyFiles(source, nextDest, kind, flags, nerr, nr + 1, partestOutput) } From 593619063b9435c91b48fb1c1e1a07b3a15000b2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 15 Jan 2016 17:44:06 +0100 Subject: [PATCH 5/7] Increase heap size to 2G --- scripts/common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/common b/scripts/common index 77da64d83542..b69c65717f71 100755 --- a/scripts/common +++ b/scripts/common @@ -16,4 +16,4 @@ update() { export LC_ALL=en_US.UTF-8 -sbtArgs="-Ddotty.jenkins.build=yes -Dfile.encoding=UTF-8 -ivy $baseDir/ivy2 -Dsbt.global.base=$HOME/.sbt/0.13 -sbt-dir $HOME/.sbt/0.13" +sbtArgs="-Ddotty.jenkins.build=yes -J-Xmx2G -Dfile.encoding=UTF-8 -ivy $baseDir/ivy2 -Dsbt.global.base=$HOME/.sbt/0.13 -sbt-dir $HOME/.sbt/0.13" From 311e585577b9590071ede263865cc4272280cb7f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 16 Jan 2016 13:00:46 +0100 Subject: [PATCH 6/7] Make skolems uncached 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. --- src/dotty/tools/dotc/core/Types.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 497d2494c242..9ec211a270a0 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -2534,11 +2534,11 @@ object Types { // ----- Skolem types ----------------------------------------------- /** A skolem type reference with underlying type `binder`. */ - abstract case class SkolemType(info: Type) extends CachedProxyType with ValueType with SingletonType { + abstract case class SkolemType(info: Type) extends UncachedProxyType with ValueType with SingletonType { override def underlying(implicit ctx: Context) = info def derivedSkolemType(info: Type)(implicit ctx: Context) = if (info eq this.info) this else SkolemType(info) - override def computeHash: Int = identityHash + override def hashCode: Int = identityHash override def equals(that: Any) = this eq that.asInstanceOf[AnyRef] override def toString = s"Skolem($info)" } From 35624d012f907ca67a440cf5163e7b612eef8a01 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 16 Jan 2016 13:44:16 +0100 Subject: [PATCH 7/7] Print total memory when exiting because of OOM --- test/dotty/partest/DPDirectCompiler.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/dotty/partest/DPDirectCompiler.scala b/test/dotty/partest/DPDirectCompiler.scala index ca56ac3e991e..d1f9e9a3779d 100644 --- a/test/dotty/partest/DPDirectCompiler.scala +++ b/test/dotty/partest/DPDirectCompiler.scala @@ -35,7 +35,9 @@ class DPDirectCompiler(runner: DPTestRunner) extends nest.DirectCompiler(runner) } } catch { case t: Throwable => - t.printStackTrace + if (t.isInstanceOf[java.lang.OutOfMemoryError]) + println(s"total memory = "+ Runtime.getRuntime().totalMemory / (1024*1024) +"mb") + t.printStackTrace() t.printStackTrace(clogWriter) runner.genCrash(t) } finally {