Skip to content

Commit da8da4d

Browse files
committed
Fix scala#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. Test case for indirect overrides is in i1750a.scala.
1 parent ba7e129 commit da8da4d

File tree

7 files changed

+47
-7
lines changed

7 files changed

+47
-7
lines changed

compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import Flags._, Symbols._, Contexts._, Types._, Scopes._, Decorators._
66
import util.HashSet
77
import collection.mutable
88
import collection.immutable.BitSet
9+
import typer.ErrorReporting.cyclicErrorMsg
910
import scala.annotation.tailrec
1011

1112
/** A module that can produce a kind of iterator (`Cursor`),
@@ -122,10 +123,18 @@ object OverridingPairs {
122123
if (nextEntry ne null) {
123124
nextEntry = decls.lookupNextEntry(nextEntry)
124125
if (nextEntry ne null) {
125-
overridden = nextEntry.sym
126-
if (overriding.owner != overridden.owner && matches(overriding, overridden)) {
127-
visited += overridden
128-
if (!hasCommonParentAsSubclass(overriding.owner, overridden.owner)) return
126+
try {
127+
overridden = nextEntry.sym
128+
if (overriding.owner != overridden.owner && matches(overriding, overridden)) {
129+
visited += overridden
130+
if (!hasCommonParentAsSubclass(overriding.owner, overridden.owner)) return
131+
}
132+
}
133+
catch {
134+
case ex: CyclicReference =>
135+
// See neg/i1750a for an example where a cyclic error can arise.
136+
// The root cause in this example is an illegal "override" of an inner trait
137+
ctx.error(cyclicErrorMsg(ex), base.pos)
129138
}
130139
} else {
131140
curEntry = curEntry.prev

compiler/src/dotty/tools/dotc/typer/Checking.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,11 @@ object Checking {
338338
checkNoConflict(Final, Sealed)
339339
checkNoConflict(Private, Protected)
340340
checkNoConflict(Abstract, Override)
341+
if (sym.isType && !sym.is(Deferred))
342+
for (cls <- sym.allOverriddenSymbols.filter(_.isClass)) {
343+
fail(i"$sym cannot have the same name as ${cls.showLocated} -- class definitions cannot be overridden")
344+
sym.setFlag(Private) // break the overriding relationship by making sym Private
345+
}
341346
}
342347

343348
/** Check the type signature of the symbol `M` defined by `tree` does not refer

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,8 @@ object RefChecks {
289289
if (!isOverrideAccessOK) {
290290
overrideAccessError()
291291
} else if (other.isClass) {
292+
// direct overrides were already checked on completion (see Checking.chckWellFormed)
293+
// the test here catches indirect overriddes between two inherited base types.
292294
overrideError("cannot be used here - class definitions cannot be overridden")
293295
} else if (!other.is(Deferred) && member.isClass) {
294296
overrideError("cannot be used here - classes can only override abstract types")

tests/neg/customArgs/overrideClass.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@
88
}
99
trait FooB extends FooA {
1010
type A <: Ax;
11-
trait Ax extends super.Ax { def xxx : Int; } // error: cyclic inheritance: trait Ax extends itself
12-
// (Note that inheriting a class of the same name is no longer allowed)
11+
trait Ax extends super.Ax { def xxx : Int; } // error: cyclic inheritance: trait Ax extends itself) // error: class definitions cannot be overridden
1312
abstract class InnerB extends InnerA {
1413
// type B <: A;
1514
val a : A = doB;

tests/neg/i1750.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
trait Lang1 {
2+
trait Exp
3+
trait Visitor { def f(left: Exp): Unit }
4+
class Eval1 extends Visitor { self =>
5+
def f(left: Exp) = ()
6+
}
7+
}
8+
trait Lang2 extends Lang1 {
9+
class Visitor extends Eval1 { Visitor => // error: classes cannot be overridden
10+
}
11+
}
12+

tests/neg/i1750a.scala

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
trait Lang1 {
2+
trait Exp
3+
trait Visitor { def f(left: Exp): Unit }
4+
class Eval1 extends Visitor { self =>
5+
def f(left: Exp) = ()
6+
}
7+
}
8+
trait Lang3 { self: Lang1 =>
9+
class Visitor extends Eval1 { Visitor => // error: classes cannot be overridden
10+
}
11+
}
12+
trait Lang4 extends Lang1 with Lang3
13+

tests/neg/i1806.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@ trait A {
22
class Inner
33
}
44
trait B extends A {
5-
class Inner extends super.Inner // error
5+
class Inner extends super.Inner // error // error
66
}
77

0 commit comments

Comments
 (0)