From f55db64983edfeb9484b7617e2b59f8994c37ef3 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Wed, 1 Feb 2012 08:06:24 -0800 Subject: Fix for bad bug with accidental overrides. 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. --- src/compiler/scala/reflect/internal/Types.scala | 10 ++ .../tools/nsc/transform/OverridingPairs.scala | 10 +- .../scala/tools/nsc/typechecker/RefChecks.scala | 2 + test/files/neg/t5429.check | 132 +++++++++++++++++++++ test/files/neg/t5429.scala | 93 +++++++++++++++ 5 files changed, 245 insertions(+), 2 deletions(-) create mode 100644 test/files/neg/t5429.check create mode 100644 test/files/neg/t5429.scala diff --git a/src/compiler/scala/reflect/internal/Types.scala b/src/compiler/scala/reflect/internal/Types.scala index 371fb8d585..c8b960ebe8 100644 --- a/src/compiler/scala/reflect/internal/Types.scala +++ b/src/compiler/scala/reflect/internal/Types.scala @@ -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 } @@ -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) } @@ -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 } diff --git a/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala b/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala index 1200e973c5..e49f8d7c0b 100644 --- a/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala +++ b/src/compiler/scala/tools/nsc/transform/OverridingPairs.scala @@ -45,8 +45,14 @@ abstract class OverridingPairs { * Types always match. Term symbols match if their membertypes * relative to .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 diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index a6c2f75d5e..5aa1843188 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -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 diff --git a/test/files/neg/t5429.check b/test/files/neg/t5429.check new file mode 100644 index 0000000000..1b89c59587 --- /dev/null +++ b/test/files/neg/t5429.check @@ -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 diff --git a/test/files/neg/t5429.scala b/test/files/neg/t5429.scala new file mode 100644 index 0000000000..1cd4dcd032 --- /dev/null +++ b/test/files/neg/t5429.scala @@ -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 +} + -- cgit v1.2.3 From f411950e43fbf56306f96bbc8803ba278d25f440 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Thu, 2 Feb 2012 13:13:55 -0800 Subject: matchesType fixup. --- src/compiler/scala/reflect/internal/Types.scala | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/compiler/scala/reflect/internal/Types.scala b/src/compiler/scala/reflect/internal/Types.scala index c8b960ebe8..2c01008efe 100644 --- a/src/compiler/scala/reflect/internal/Types.scala +++ b/src/compiler/scala/reflect/internal/Types.scala @@ -5459,8 +5459,8 @@ trait Types extends api.Types { self: SymbolTable => matchesType(tp1, res2, true) case MethodType(_, _) => false - case PolyType(tparams2, res2) => - tparams2.isEmpty && matchesType(tp1, res2, alwaysMatchSimple) + case PolyType(_, _) => + false case _ => alwaysMatchSimple || tp1 =:= tp2 } @@ -5478,7 +5478,7 @@ trait Types extends api.Types { self: SymbolTable => case ExistentialType(_, res2) => alwaysMatchSimple && matchesType(tp1, res2, true) case TypeRef(_, sym, Nil) => - params1.isEmpty && sym.isModuleClass && matchesType(res1, sym.tpe, alwaysMatchSimple) + params1.isEmpty && sym.isModuleClass && matchesType(res1, tp2, alwaysMatchSimple) case _ => false } @@ -5491,7 +5491,7 @@ trait Types extends api.Types { self: SymbolTable => case ExistentialType(_, res2) => alwaysMatchSimple && matchesType(tp1, res2, true) case TypeRef(_, sym, Nil) if sym.isModuleClass => - matchesType(res1, sym.tpe, alwaysMatchSimple) + matchesType(res1, tp2, alwaysMatchSimple) case _ => matchesType(res1, tp2, alwaysMatchSimple) } @@ -5514,8 +5514,8 @@ trait Types extends api.Types { self: SymbolTable => } 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 MethodType(Nil, res2) => matchesType(tp1, res2, alwaysMatchSimple) + case NullaryMethodType(res2) => matchesType(tp1, res2, alwaysMatchSimple) case _ => lastTry } case _ => -- cgit v1.2.3