From 88bac0ec158454b683eb04919da91838b627e424 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Thu, 30 Mar 2017 21:52:09 +0200 Subject: [PATCH 1/7] Fix #2165, emit outerChecks on ThisType ThisType doesn't have a termSymbol. And the check is actually too strong, and not needed. --- compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index 7576ccc05e0a..e9bee96cae95 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -863,9 +863,9 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer { override lazy val introducedRebindings = NoRebindings def outerTestNeeded = { - val np = expectedTp.normalizedPrefix + /*val np = expectedTp.normalizedPrefix val ts = np.termSymbol - (ts ne NoSymbol) && needsOuterTest(expectedTp, testedBinder.info, ctx.owner) + (ts ne NoSymbol) && */needsOuterTest(expectedTp, testedBinder.info, ctx.owner) } // the logic to generate the run-time test that follows from the fact that From c5031a7c27d7b714a7ff917ba539a077e4ec215d Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Thu, 30 Mar 2017 21:52:37 +0200 Subject: [PATCH 2/7] PatMat: get rid of unnecessary forwarder --- .../dotty/tools/dotc/transform/PatternMatcher.scala | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index e9bee96cae95..8d25175f2698 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -848,10 +848,10 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer { val nextBinder = afterTest.asTerm - def needsOuterTest(patType: Type, selType: Type, currentOwner: Symbol): Boolean = { + def outerTestNeeded(implicit ctx: Context): Boolean = { // See the test for SI-7214 for motivation for dealias. Later `treeCondStrategy#outerTest` // generates an outer test based on `patType.prefix` with automatically dealises. - patType.dealias match { + expectedTp.dealias match { case tref @ TypeRef(pre, name) => (pre ne NoPrefix) && tref.symbol.isClass && ExplicitOuter.needsOuterIfReferenced(tref.symbol.asClass) @@ -862,12 +862,6 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer { override lazy val introducedRebindings = NoRebindings - def outerTestNeeded = { - /*val np = expectedTp.normalizedPrefix - val ts = np.termSymbol - (ts ne NoSymbol) && */needsOuterTest(expectedTp, testedBinder.info, ctx.owner) - } - // the logic to generate the run-time test that follows from the fact that // a `prevBinder` is expected to have type `expectedTp` // the actual tree-generation logic is factored out, since the analyses generate Cond(ition)s rather than Trees From e9013104a367910ca4970fdde741528e84ced829 Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Thu, 30 Mar 2017 21:54:11 +0200 Subject: [PATCH 3/7] PatMat, Outerchecks: Check outers for selections from singleton type. Otherwise checks are done also on type projections. Same pitfall as https://issues.scala-lang.org/browse/SI-7214 --- .../src/dotty/tools/dotc/transform/PatternMatcher.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index 8d25175f2698..b0ae366125a9 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -852,9 +852,10 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer { // See the test for SI-7214 for motivation for dealias. Later `treeCondStrategy#outerTest` // generates an outer test based on `patType.prefix` with automatically dealises. expectedTp.dealias match { - case tref @ TypeRef(pre, name) => - (pre ne NoPrefix) && tref.symbol.isClass && - ExplicitOuter.needsOuterIfReferenced(tref.symbol.asClass) + case tref @ TypeRef(pre: SingletonType, name) => + val s = tref + s.symbol.isClass && + ExplicitOuter.needsOuterIfReferenced(s.symbol.asClass) case _ => false } From 8a0c3c445cf4d46a87e36c8166707575d82863bf Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Thu, 30 Mar 2017 21:54:23 +0200 Subject: [PATCH 4/7] Test that #2156 is fixed --- tests/run/i2156.scala | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/run/i2156.scala diff --git a/tests/run/i2156.scala b/tests/run/i2156.scala new file mode 100644 index 000000000000..12ce8fa88e7c --- /dev/null +++ b/tests/run/i2156.scala @@ -0,0 +1,37 @@ +class Outer { + + case class Inner() + + val inner: Inner = new Inner + + def checkInstance(o: Outer) = + o.inner.isInstanceOf[this.Inner] + + def checkPattern1(i: Any) = + i match { + case _: Inner => true + case _ => false + } + + def checkPattern2(i: Any) = + i match { + case Inner() => true + case _ => false + } + + def checkEquals(o: Outer) = + o.inner == inner +} + +object Test { + + def main(args: Array[String]) = { + val o1 = new Outer + val o2 = new Outer + assert(o1.checkInstance(o2)) // ok + assert(!o1.checkPattern1(o2.inner)) // ok under scalac, fails for dotc-compiled code + assert(!o1.checkPattern2(o2.inner)) // ok under scalac, fails for dotc-compiled code + assert(!o1.checkEquals(o2)) // ok under scalac, fails for dotc-compiled code + } +} + From 03f13046f0a6cde36cba2a9649aad8996ba7550a Mon Sep 17 00:00:00 2001 From: Dmitry Petrashko Date: Thu, 30 Mar 2017 22:54:15 +0200 Subject: [PATCH 5/7] Fix a bug(I guess?) hidden by scalac sometimes not emitting outer checks --- compiler/src/dotty/tools/io/ClassPath.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/io/ClassPath.scala b/compiler/src/dotty/tools/io/ClassPath.scala index 3afbed838d9a..413d095c434f 100644 --- a/compiler/src/dotty/tools/io/ClassPath.scala +++ b/compiler/src/dotty/tools/io/ClassPath.scala @@ -242,8 +242,8 @@ abstract class ClassPath { case Some((pkg, rest)) => val rep = packages find (_.name == pkg) flatMap (_ findClass rest) rep map { - case x: ClassRep => x - case x => throw new FatalError("Unexpected ClassRep '%s' found searching for name '%s'".format(x, name)) + case x: AnyClassRep => x + case x => throw new FatalError("Unexpected ClassRep '%s' found searching for name '%s'".format(x, name)) } case _ => classes find (_.name == name) @@ -256,6 +256,7 @@ abstract class ClassPath { } def sortString = join(split(asClasspathString).sorted: _*) + override def equals(that: Any) = that match { case x: ClassPath => this.sortString == x.sortString case _ => false From f3707992ee2ed56ce754df8c0760fc3928f0317f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 31 Mar 2017 11:15:51 +0200 Subject: [PATCH 6/7] Fix ClassfileParser #2158 has uncovered flaws in the classfile parser. Matches that used to always miss led to code that made no sense. The function naming was terrible too, that's why nobody understood what was going on. `findSourceFile` to find the class file, seriously? --- .../tools/dotc/core/classfile/ClassfileParser.scala | 2 +- compiler/src/dotty/tools/io/ClassPath.scala | 13 +++---------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index bc140c26b396..e0b233ce875d 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -661,7 +661,7 @@ class ClassfileParser( for (entry <- innerClasses.values) { // create a new class member for immediate inner classes if (entry.outerName == currentClassName) { - val file = ctx.platform.classPath.findSourceFile(entry.externalName.toString) getOrElse { + val file = ctx.platform.classPath.findBinaryFile(entry.externalName.toString) getOrElse { throw new AssertionError(entry.externalName) } enterClassAndModule(entry, file, entry.jflags) diff --git a/compiler/src/dotty/tools/io/ClassPath.scala b/compiler/src/dotty/tools/io/ClassPath.scala index 413d095c434f..5e77c1b61a39 100644 --- a/compiler/src/dotty/tools/io/ClassPath.scala +++ b/compiler/src/dotty/tools/io/ClassPath.scala @@ -240,20 +240,13 @@ abstract class ClassPath { def findClass(name: String): Option[AnyClassRep] = name.splitWhere(_ == '.', doDropIndex = true) match { case Some((pkg, rest)) => - val rep = packages find (_.name == pkg) flatMap (_ findClass rest) - rep map { - case x: AnyClassRep => x - case x => throw new FatalError("Unexpected ClassRep '%s' found searching for name '%s'".format(x, name)) - } + packages find (_.name == pkg) flatMap (_ findClass rest) case _ => classes find (_.name == name) } - def findSourceFile(name: String): Option[AbstractFile] = - findClass(name) match { - case Some(ClassRep(Some(x: AbstractFile), _)) => Some(x) - case _ => None - } + def findBinaryFile(name: String): Option[AbstractFile] = + findClass(name).flatMap(_.binary) def sortString = join(split(asClasspathString).sorted: _*) From bbd9aeb68b8e1690c24a598c55fc7c1acc89b71f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 31 Mar 2017 12:03:26 +0200 Subject: [PATCH 7/7] Avoid NPE in ParallelTesting on junk files Emacs often produces temporary files in directories. These used to cause NPEs in the new testing framework. We now fix this by only compiling file names that designate source files. --- compiler/test/dotty/tools/dotc/ParallelTesting.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/test/dotty/tools/dotc/ParallelTesting.scala b/compiler/test/dotty/tools/dotc/ParallelTesting.scala index 289351d81e63..a81eb4d3a517 100644 --- a/compiler/test/dotty/tools/dotc/ParallelTesting.scala +++ b/compiler/test/dotty/tools/dotc/ParallelTesting.scala @@ -920,9 +920,8 @@ trait ParallelTesting { self => private def compilationTargets(sourceDir: JFile): (List[JFile], List[JFile]) = sourceDir.listFiles.foldLeft((List.empty[JFile], List.empty[JFile])) { case ((dirs, files), f) => if (f.isDirectory) (f :: dirs, files) - else if (f.getName.endsWith(".check")) (dirs, files) - else if (f.getName.endsWith(".flags")) (dirs, files) - else (dirs, f :: files) + else if (isSourceFile(f)) (dirs, f :: files) + else (dirs, files) } /** Gets the name of the calling method via reflection.