From 56fb15888fb98e3a6a535f5734bb8ce82fcd76c3 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 29 Jan 2017 17:54:30 +1100 Subject: [PATCH] Fix #1750: Handle illegal class overrides better Illegal class overrides are fundamentally at odds with the way dotty represents types and therefore can cause lots of low-level problems. Two measures in this commit First, we detect direct illegal class overrides on completion instead of during RefChecks. Break the override by making the previously overriding type private. This fixes i1750.scala, but still fails for indirect overrides between two unrelated outer traits/classes that are inherited by the same class or trait. We fix this by catching the previously thrown ClassCastException in both ExtractAPI and RefChecks. Test case for indirect overrides is in i1750a.scala. --- .../src/dotty/tools/dotc/sbt/ExtractAPI.scala | 12 +++++++++++- .../tools/dotc/transform/OverridingPairs.scala | 17 +++++++++++++---- .../src/dotty/tools/dotc/typer/Checking.scala | 5 +++++ .../src/dotty/tools/dotc/typer/RefChecks.scala | 2 ++ tests/neg/customArgs/overrideClass.scala | 3 +-- tests/neg/i1750.scala | 12 ++++++++++++ tests/neg/i1750a.scala | 13 +++++++++++++ tests/neg/i1806.scala | 2 +- tests/neg/overrides.scala | 5 ----- tests/neg/t7278.scala | 2 +- 10 files changed, 59 insertions(+), 14 deletions(-) create mode 100644 tests/neg/i1750.scala create mode 100644 tests/neg/i1750a.scala diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala index 1fffe6841b6b..7f44af486ac6 100644 --- a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala +++ b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala @@ -6,6 +6,7 @@ import core._, core.Decorators._ import Annotations._, Contexts._, Flags._, Phases._, Trees._, Types._, Symbols._ import Names._, NameOps._, StdNames._ import typer.Inliner +import typer.ErrorReporting.cyclicErrorMsg import dotty.tools.io.Path import java.io.PrintWriter @@ -190,7 +191,16 @@ private class ExtractAPICollector(implicit val ctx: Context) extends ThunkHolder def apiClassStructure(csym: ClassSymbol): api.Structure = { val cinfo = csym.classInfo - val bases = linearizedAncestorTypes(cinfo) + val bases = + try linearizedAncestorTypes(cinfo) + catch { + case ex: CyclicReference => + // See neg/i1750a for an example where a cyclic error can arise. + // The root cause in this example is an illegal "override" of an inner trait + ctx.error(cyclicErrorMsg(ex), csym.pos) + defn.ObjectType :: Nil + } + val apiBases = bases.map(apiType) // Synthetic methods that are always present do not affect the API diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index 650a03054e05..1f1f371a6ec8 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -6,6 +6,7 @@ import Flags._, Symbols._, Contexts._, Types._, Scopes._, Decorators._ import util.HashSet import collection.mutable import collection.immutable.BitSet +import typer.ErrorReporting.cyclicErrorMsg import scala.annotation.tailrec /** A module that can produce a kind of iterator (`Cursor`), @@ -122,10 +123,18 @@ object OverridingPairs { if (nextEntry ne null) { nextEntry = decls.lookupNextEntry(nextEntry) if (nextEntry ne null) { - overridden = nextEntry.sym - if (overriding.owner != overridden.owner && matches(overriding, overridden)) { - visited += overridden - if (!hasCommonParentAsSubclass(overriding.owner, overridden.owner)) return + try { + overridden = nextEntry.sym + if (overriding.owner != overridden.owner && matches(overriding, overridden)) { + visited += overridden + if (!hasCommonParentAsSubclass(overriding.owner, overridden.owner)) return + } + } + catch { + case ex: CyclicReference => + // See neg/i1750a for an example where a cyclic error can arise. + // The root cause in this example is an illegal "override" of an inner trait + ctx.error(cyclicErrorMsg(ex), base.pos) } } else { curEntry = curEntry.prev diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 41d9f9572029..321c275d763b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -338,6 +338,11 @@ object Checking { checkNoConflict(Final, Sealed) checkNoConflict(Private, Protected) checkNoConflict(Abstract, Override) + if (sym.isType && !sym.is(Deferred)) + for (cls <- sym.allOverriddenSymbols.filter(_.isClass)) { + fail(i"$sym cannot have the same name as ${cls.showLocated} -- class definitions cannot be overridden") + sym.setFlag(Private) // break the overriding relationship by making sym Private + } } /** Check the type signature of the symbol `M` defined by `tree` does not refer diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 3192546cd37f..e113399c5b72 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -289,6 +289,8 @@ object RefChecks { if (!isOverrideAccessOK) { overrideAccessError() } else if (other.isClass) { + // direct overrides were already checked on completion (see Checking.chckWellFormed) + // the test here catches indirect overriddes between two inherited base types. overrideError("cannot be used here - class definitions cannot be overridden") } else if (!other.is(Deferred) && member.isClass) { overrideError("cannot be used here - classes can only override abstract types") diff --git a/tests/neg/customArgs/overrideClass.scala b/tests/neg/customArgs/overrideClass.scala index 803d97dd92d7..431b846d90ce 100644 --- a/tests/neg/customArgs/overrideClass.scala +++ b/tests/neg/customArgs/overrideClass.scala @@ -8,8 +8,7 @@ } trait FooB extends FooA { type A <: Ax; - trait Ax extends super.Ax { def xxx : Int; } // error: cyclic inheritance: trait Ax extends itself - // (Note that inheriting a class of the same name is no longer allowed) + trait Ax extends super.Ax { def xxx : Int; } // error: cyclic inheritance: trait Ax extends itself) // error: class definitions cannot be overridden abstract class InnerB extends InnerA { // type B <: A; val a : A = doB; diff --git a/tests/neg/i1750.scala b/tests/neg/i1750.scala new file mode 100644 index 000000000000..a2ebe8f2b76e --- /dev/null +++ b/tests/neg/i1750.scala @@ -0,0 +1,12 @@ +trait Lang1 { + trait Exp + trait Visitor { def f(left: Exp): Unit } + class Eval1 extends Visitor { self => + def f(left: Exp) = () + } +} +trait Lang2 extends Lang1 { + class Visitor extends Eval1 { Visitor => // error: classes cannot be overridden + } +} + diff --git a/tests/neg/i1750a.scala b/tests/neg/i1750a.scala new file mode 100644 index 000000000000..cbb5bec954e3 --- /dev/null +++ b/tests/neg/i1750a.scala @@ -0,0 +1,13 @@ +trait Lang1 { + trait Exp + trait Visitor { def f(left: Exp): Unit } + class Eval1 extends Visitor { self => + def f(left: Exp) = () + } +} +trait Lang3 { self: Lang1 => + class Visitor extends Eval1 { Visitor => // error: classes cannot be overridden + } +} +trait Lang4 extends Lang1 with Lang3 + diff --git a/tests/neg/i1806.scala b/tests/neg/i1806.scala index 7e5e132f2e14..199ab47916f0 100644 --- a/tests/neg/i1806.scala +++ b/tests/neg/i1806.scala @@ -2,6 +2,6 @@ trait A { class Inner } trait B extends A { - class Inner extends super.Inner // error + class Inner extends super.Inner // error // error } diff --git a/tests/neg/overrides.scala b/tests/neg/overrides.scala index 81a93a7a2833..149220bd560d 100644 --- a/tests/neg/overrides.scala +++ b/tests/neg/overrides.scala @@ -34,11 +34,6 @@ package p2 { // all being in the same package compiles fine } } - abstract class T3 extends T2 { - class A { // error: classes cannot be overridden - bug() - } - } } class A[T] { diff --git a/tests/neg/t7278.scala b/tests/neg/t7278.scala index 643a3c858386..7b13535f0a60 100644 --- a/tests/neg/t7278.scala +++ b/tests/neg/t7278.scala @@ -1,5 +1,5 @@ class A { class E } -class B extends A { class E } +class B extends A { class EB } trait C { type E = Int } trait D { type E = String } trait EC { type E }