Skip to content

Commit

Permalink
Fix for bad bug with accidental overrides.
Browse files Browse the repository at this point in the history
An object in a subclass would silently override an inherited
method, then throw a CCE at runtime.  I blamed this on matchesType
and altered it accordingly.  There's a pretty extensive test case
which reflects my expectations.  Review by @odersky please.

Closes SI-5429.
  • Loading branch information
paulp committed Feb 1, 2012
1 parent 37bcff7 commit f55db64
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 2 deletions.
10 changes: 10 additions & 0 deletions src/compiler/scala/reflect/internal/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5477,6 +5477,8 @@ trait Types extends api.Types { self: SymbolTable =>
else matchesType(tp1, res2, alwaysMatchSimple)
case ExistentialType(_, res2) =>
alwaysMatchSimple && matchesType(tp1, res2, true)
case TypeRef(_, sym, Nil) =>
params1.isEmpty && sym.isModuleClass && matchesType(res1, sym.tpe, alwaysMatchSimple)
case _ =>
false
}
Expand All @@ -5488,6 +5490,8 @@ trait Types extends api.Types { self: SymbolTable =>
matchesType(res1, res2, alwaysMatchSimple)
case ExistentialType(_, res2) =>
alwaysMatchSimple && matchesType(tp1, res2, true)
case TypeRef(_, sym, Nil) if sym.isModuleClass =>
matchesType(res1, sym.tpe, alwaysMatchSimple)
case _ =>
matchesType(res1, tp2, alwaysMatchSimple)
}
Expand All @@ -5508,6 +5512,12 @@ trait Types extends api.Types { self: SymbolTable =>
if (alwaysMatchSimple) matchesType(res1, tp2, true)
else lastTry
}
case TypeRef(_, sym, Nil) if sym.isModuleClass =>
tp2 match {
case MethodType(Nil, res2) => matchesType(sym.tpe, res2, alwaysMatchSimple)
case NullaryMethodType(res2) => matchesType(sym.tpe, res2, alwaysMatchSimple)
case _ => lastTry
}
case _ =>
lastTry
}
Expand Down
10 changes: 8 additions & 2 deletions src/compiler/scala/tools/nsc/transform/OverridingPairs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,14 @@ abstract class OverridingPairs {
* Types always match. Term symbols match if their membertypes
* relative to <base>.this do
*/
protected def matches(sym1: Symbol, sym2: Symbol): Boolean =
sym1.isType || (self.memberType(sym1) matches self.memberType(sym2))
protected def matches(sym1: Symbol, sym2: Symbol): Boolean = {
def tp_s(s: Symbol) = self.memberType(s) + "/" + self.memberType(s).getClass
val result = sym1.isType || (self.memberType(sym1) matches self.memberType(sym2))
debuglog("overriding-pairs? %s matches %s (%s vs. %s) == %s".format(
sym1.fullLocationString, sym2.fullLocationString, tp_s(sym1), tp_s(sym2), result))

result
}

/** An implementation of BitSets as arrays (maybe consider collection.BitSet
* for that?) The main purpose of this is to implement
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
* of class `clazz` are met.
*/
def checkOverride(member: Symbol, other: Symbol) {
debuglog("Checking validity of %s overriding %s".format(member.fullLocationString, other.fullLocationString))

def memberTp = self.memberType(member)
def otherTp = self.memberType(other)
def noErrorType = other.tpe != ErrorType && member.tpe != ErrorType
Expand Down
132 changes: 132 additions & 0 deletions test/files/neg/t5429.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
t5429.scala:20: error: overriding value value in class A of type Int;
object value needs `override' modifier
object value // fail
^
t5429.scala:21: error: overriding lazy value lazyvalue in class A of type Int;
object lazyvalue needs `override' modifier
object lazyvalue // fail
^
t5429.scala:22: error: overriding method nullary in class A of type => Int;
object nullary needs `override' modifier
object nullary // fail
^
t5429.scala:23: error: overriding method emptyArg in class A of type ()Int;
object emptyArg needs `override' modifier
object emptyArg // fail
^
t5429.scala:27: error: overriding value value in class A0 of type Any;
object value needs `override' modifier
object value // fail
^
t5429.scala:28: error: overriding lazy value lazyvalue in class A0 of type Any;
object lazyvalue needs `override' modifier
object lazyvalue // fail
^
t5429.scala:29: error: overriding method nullary in class A0 of type => Any;
object nullary needs `override' modifier
object nullary // fail
^
t5429.scala:30: error: overriding method emptyArg in class A0 of type ()Any;
object emptyArg needs `override' modifier
object emptyArg // fail
^
t5429.scala:35: error: overriding value value in class A of type Int;
object value has incompatible type
override object value // fail
^
t5429.scala:36: error: overriding lazy value lazyvalue in class A of type Int;
object lazyvalue must be declared lazy to override a concrete lazy value
override object lazyvalue // fail
^
t5429.scala:37: error: overriding method nullary in class A of type => Int;
object nullary has incompatible type
override object nullary // fail
^
t5429.scala:38: error: overriding method emptyArg in class A of type ()Int;
object emptyArg has incompatible type
override object emptyArg // fail
^
t5429.scala:39: error: object oneArg overrides nothing
override object oneArg // fail
^
t5429.scala:43: error: overriding lazy value lazyvalue in class A0 of type Any;
object lazyvalue must be declared lazy to override a concrete lazy value
override object lazyvalue // !!! this fails, but should succeed (lazy over lazy)
^
t5429.scala:46: error: object oneArg overrides nothing
override object oneArg // fail
^
t5429.scala:50: error: overriding value value in class A of type Int;
value value needs `override' modifier
val value = 0 // fail
^
t5429.scala:51: error: overriding lazy value lazyvalue in class A of type Int;
value lazyvalue needs `override' modifier
val lazyvalue = 0 // fail
^
t5429.scala:52: error: overriding method nullary in class A of type => Int;
value nullary needs `override' modifier
val nullary = 5 // fail
^
t5429.scala:53: error: overriding method emptyArg in class A of type ()Int;
value emptyArg needs `override' modifier
val emptyArg = 10 // fail
^
t5429.scala:58: error: overriding lazy value lazyvalue in class A0 of type Any;
value lazyvalue must be declared lazy to override a concrete lazy value
override val lazyvalue = 0 // fail (non-lazy)
^
t5429.scala:61: error: value oneArg overrides nothing
override val oneArg = 15 // fail
^
t5429.scala:65: error: overriding value value in class A of type Int;
method value needs `override' modifier
def value = 0 // fail
^
t5429.scala:66: error: overriding lazy value lazyvalue in class A of type Int;
method lazyvalue needs `override' modifier
def lazyvalue = 2 // fail
^
t5429.scala:67: error: overriding method nullary in class A of type => Int;
method nullary needs `override' modifier
def nullary = 5 // fail
^
t5429.scala:68: error: overriding method emptyArg in class A of type ()Int;
method emptyArg needs `override' modifier
def emptyArg = 10 // fail
^
t5429.scala:72: error: overriding value value in class A0 of type Any;
method value needs to be a stable, immutable value
override def value = 0 // fail
^
t5429.scala:73: error: overriding lazy value lazyvalue in class A0 of type Any;
method lazyvalue needs to be a stable, immutable value
override def lazyvalue = 2 // fail
^
t5429.scala:76: error: method oneArg overrides nothing
override def oneArg = 15 // fail
^
t5429.scala:80: error: overriding value value in class A of type Int;
lazy value value needs `override' modifier
lazy val value = 0 // fail
^
t5429.scala:81: error: overriding lazy value lazyvalue in class A of type Int;
lazy value lazyvalue needs `override' modifier
lazy val lazyvalue = 2 // fail
^
t5429.scala:82: error: overriding method nullary in class A of type => Int;
lazy value nullary needs `override' modifier
lazy val nullary = 5 // fail
^
t5429.scala:83: error: overriding method emptyArg in class A of type ()Int;
lazy value emptyArg needs `override' modifier
lazy val emptyArg = 10 // fail
^
t5429.scala:87: error: overriding value value in class A0 of type Any;
lazy value value cannot override a concrete non-lazy value
override lazy val value = 0 // fail (strict over lazy)
^
t5429.scala:91: error: value oneArg overrides nothing
override lazy val oneArg = 15 // fail
^
34 errors found
93 changes: 93 additions & 0 deletions test/files/neg/t5429.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// /scala/trac/5429/a.scala
// Wed Feb 1 08:05:27 PST 2012

class A {
val value = 0
lazy val lazyvalue = 2
def nullary = 5
def emptyArg() = 10
def oneArg(x: String) = 15
}
class A0 {
val value: Any = 0
lazy val lazyvalue: Any = 2
def nullary: Any = 5
def emptyArg(): Any = 10
def oneArg(x: String): Any = 15
}

class B extends A {
object value // fail
object lazyvalue // fail
object nullary // fail
object emptyArg // fail
object oneArg // overload
}
class B0 extends A0 {
object value // fail
object lazyvalue // fail
object nullary // fail
object emptyArg // fail
object oneArg // overload
}

class C extends A {
override object value // fail
override object lazyvalue // fail
override object nullary // fail
override object emptyArg // fail
override object oneArg // fail
}
class C0 extends A0 {
override object value // !!! this succeeds, but should fail (lazy over strict)
override object lazyvalue // !!! this fails, but should succeed (lazy over lazy)
override object nullary // override
override object emptyArg // override
override object oneArg // fail
}

class D extends A {
val value = 0 // fail
val lazyvalue = 0 // fail
val nullary = 5 // fail
val emptyArg = 10 // fail
val oneArg = 15 // overload
}
class D0 extends A0 {
override val value = 0 // override
override val lazyvalue = 0 // fail (non-lazy)
override val nullary = 5 // override
override val emptyArg = 10 // override
override val oneArg = 15 // fail
}

class E extends A {
def value = 0 // fail
def lazyvalue = 2 // fail
def nullary = 5 // fail
def emptyArg = 10 // fail
def oneArg = 15 // overload
}
class E0 extends A0 {
override def value = 0 // fail
override def lazyvalue = 2 // fail
override def nullary = 5 // override
override def emptyArg = 10 // override
override def oneArg = 15 // fail
}

class F extends A {
lazy val value = 0 // fail
lazy val lazyvalue = 2 // fail
lazy val nullary = 5 // fail
lazy val emptyArg = 10 // fail
lazy val oneArg = 15 // overload
}
class F0 extends A0 {
override lazy val value = 0 // fail (strict over lazy)
override lazy val lazyvalue = 2 // override (lazy over lazy)
override lazy val nullary = 5 // override
override lazy val emptyArg = 10 // override
override lazy val oneArg = 15 // fail
}

0 comments on commit f55db64

Please sign in to comment.