Merge pull request #1913 from dotty-staging/fix-#1750

Fix #1750: Alternative fix for cyclic references due to illegal class overrides

10 files changed, 59 insertions, 14 deletions

diff --git a/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala b/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala
index 1fffe6841..7f44af486 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 650a03054..1f1f371a6 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 41d9f9572..321c275d7 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 3192546cd..e113399c5 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 803d97dd9..431b846d9 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 000000000..a2ebe8f2b
--- /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 000000000..cbb5bec95
--- /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 7e5e132f2..199ab4791 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 81a93a7a2..149220bd5 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 643a3c858..7b13535f0 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 }