From 6886a96891b9ace437bc0dee79983685ccb3f55c Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 14 Sep 2018 16:33:36 +0200 Subject: [PATCH 1/4] Fix #5083: Disallow using trait parameters as prefix for its parents The rationale is to ensure outer-related NPE never happen in Scala. Otherwise, outer NPE may happen, see tests/neg/i5083.scala --- .../dotty/tools/dotc/typer/RefChecks.scala | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 292bbe6cc70d..b5cce4bd04c7 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -93,7 +93,7 @@ object RefChecks { /** Check that self type of this class conforms to self types of parents. * and required classes. */ - private def checkParents(cls: Symbol)(implicit ctx: Context): Unit = cls.info match { + private def checkParents(cls: Symbol, tmpl: Template)(implicit ctx: Context): Unit = cls.info match { case cinfo: ClassInfo => def checkSelfConforms(other: ClassSymbol, category: String, relation: String) = { val otherSelf = other.givenSelfType.asSeenFrom(cls.thisType, other.classSymbol) @@ -101,13 +101,29 @@ object RefChecks { ctx.error(DoesNotConformToSelfType(category, cinfo.selfType, cls, otherSelf, relation, other.classSymbol), cls.pos) } - for (parent <- cinfo.classParents) - checkSelfConforms(parent.classSymbol.asClass, "illegal inheritance", "parent") + for (parent <- tmpl.parents) { + checkSelfConforms(parent.tpe.classSymbol.asClass, "illegal inheritance", "parent") + checkParentPrefix(cls, parent) + } for (reqd <- cinfo.cls.givenSelfType.classSymbols) checkSelfConforms(reqd, "missing requirement", "required") case _ => } + /** Disallow using trait parameters as prefix for its parents. + * + * The rationale is to ensure outer-related NPE never happen in Scala. + * Otherwise, outer NPE may happen, see tests/neg/i5083.scala + */ + private def checkParentPrefix(cls: Symbol, parent: Tree)(implicit ctx: Context): Unit = + parent.tpe.typeConstructor match { + case TypeRef(ref: TermRef, _) => + val paramRefs = ref.namedPartsWith(ntp => ntp.symbol.enclosingClass == cls) + if (paramRefs.nonEmpty && cls.is(Trait)) + ctx.error("trait parameters cannot be used as parent prefixes", parent.pos) + case _ => + } + /** Check that a class and its companion object to not both define * a class or module with same name */ @@ -960,7 +976,7 @@ class RefChecks extends MiniPhase { thisPhase => override def transformTemplate(tree: Template)(implicit ctx: Context) = try { val cls = ctx.owner checkOverloadedRestrictions(cls) - checkParents(cls) + checkParents(cls, tree) checkCompanionNameClashes(cls) checkAllOverrides(cls) tree From 9b087cea1bd479c0b23379dc8c57a5f2c0094777 Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 14 Sep 2018 16:37:07 +0200 Subject: [PATCH 2/4] add tests --- tests/neg/i5083.scala | 25 +++++++++++++++++++++++++ tests/neg/i5083b.scala | 15 +++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 tests/neg/i5083.scala create mode 100644 tests/neg/i5083b.scala diff --git a/tests/neg/i5083.scala b/tests/neg/i5083.scala new file mode 100644 index 000000000000..033b5ce8d784 --- /dev/null +++ b/tests/neg/i5083.scala @@ -0,0 +1,25 @@ +class A(a: Int) { + abstract class X { + def f: Int + val x = a + f // NPE: the outer for `Y` is not yet set + } + + trait Y { + val y = a + def f: Int = A.this.a // NPE: the outer for `Y` is not yet set + } + + trait Z(val o: A) extends o.Y { // error + val z = a + } + + class B extends X with Z(new A(4)) +} + + +object Test { + def main(args: Array[String]): Unit = { + val a = new A(3) + new a.B + } +} diff --git a/tests/neg/i5083b.scala b/tests/neg/i5083b.scala new file mode 100644 index 000000000000..c0623d716ebe --- /dev/null +++ b/tests/neg/i5083b.scala @@ -0,0 +1,15 @@ +class A(a: Int) { + class X { + val x = a + } + + trait Y { + val y = a + } + + val z: Z = ??? + + trait Z(val o: A) extends z.o.Y // error: cyclic reference `z` + + class B extends X with Z(new A(4)) +} From 87f1d9ca849a7d542065d8374d2d2856409bed8b Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Fri, 14 Sep 2018 17:32:20 +0200 Subject: [PATCH 3/4] Add test: class fields cannot be used as parent prefix --- tests/neg/i5083c.scala | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 tests/neg/i5083c.scala diff --git a/tests/neg/i5083c.scala b/tests/neg/i5083c.scala new file mode 100644 index 000000000000..5a6cfc1a29ca --- /dev/null +++ b/tests/neg/i5083c.scala @@ -0,0 +1,25 @@ +class A(a: Int) { + trait X { + def f: Int + val x = a + f // NPE: the outer for `Y` is not yet set + } + + trait Y { + val y = a + def f: Int = A.this.a // NPE: the outer for `Y` is not yet set + } + + class Z(o: A) extends m.Y { // error + val m = o + } + + class B extends Z(new A(4)) +} + + +object Test { + def main(args: Array[String]): Unit = { + val a = new A(3) + new a.B + } +} From 35083fd516bec89d69649fcb0a1c83b59b7d134e Mon Sep 17 00:00:00 2001 From: Liu Fengyun Date: Tue, 18 Sep 2018 13:05:03 +0200 Subject: [PATCH 4/4] Address review --- compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index b5cce4bd04c7..94d35d4417d8 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -93,7 +93,7 @@ object RefChecks { /** Check that self type of this class conforms to self types of parents. * and required classes. */ - private def checkParents(cls: Symbol, tmpl: Template)(implicit ctx: Context): Unit = cls.info match { + private def checkParents(cls: Symbol)(implicit ctx: Context): Unit = cls.info match { case cinfo: ClassInfo => def checkSelfConforms(other: ClassSymbol, category: String, relation: String) = { val otherSelf = other.givenSelfType.asSeenFrom(cls.thisType, other.classSymbol) @@ -101,10 +101,8 @@ object RefChecks { ctx.error(DoesNotConformToSelfType(category, cinfo.selfType, cls, otherSelf, relation, other.classSymbol), cls.pos) } - for (parent <- tmpl.parents) { - checkSelfConforms(parent.tpe.classSymbol.asClass, "illegal inheritance", "parent") - checkParentPrefix(cls, parent) - } + for (parent <- cinfo.classParents) + checkSelfConforms(parent.classSymbol.asClass, "illegal inheritance", "parent") for (reqd <- cinfo.cls.givenSelfType.classSymbols) checkSelfConforms(reqd, "missing requirement", "required") case _ => @@ -119,7 +117,7 @@ object RefChecks { parent.tpe.typeConstructor match { case TypeRef(ref: TermRef, _) => val paramRefs = ref.namedPartsWith(ntp => ntp.symbol.enclosingClass == cls) - if (paramRefs.nonEmpty && cls.is(Trait)) + if (paramRefs.nonEmpty) ctx.error("trait parameters cannot be used as parent prefixes", parent.pos) case _ => } @@ -976,7 +974,8 @@ class RefChecks extends MiniPhase { thisPhase => override def transformTemplate(tree: Template)(implicit ctx: Context) = try { val cls = ctx.owner checkOverloadedRestrictions(cls) - checkParents(cls, tree) + checkParents(cls) + if (cls.is(Trait)) tree.parents.foreach(checkParentPrefix(cls, _)) checkCompanionNameClashes(cls) checkAllOverrides(cls) tree