Skip to content

Commit 222e9a4

Browse files
committed
Some fixes to override checking in RefChecks.
`override` was not recognized at all on types.
1 parent c1b0ab6 commit 222e9a4

File tree

2 files changed

+13
-10
lines changed

2 files changed

+13
-10
lines changed

src/dotty/tools/dotc/transform/SymUtils.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ class SymUtils(val self: Symbol) extends AnyVal {
2929

3030
def isVolatile(implicit ctx: Context) = self.hasAnnotation(defn.VolatileAnnot)
3131

32+
def isAnyOverride(implicit ctx: Context) = self.is(Override) || self.is(AbsOverride)
33+
// careful: AbsOverride is a term only flag. combining with Override would catch only terms.
34+
3235
/** If this is a constructor, its owner: otherwise this. */
3336
final def skipConstructor(implicit ctx: Context): Symbol =
3437
if (self.isConstructor) self.owner else self

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import transform._
55
import core._
66
import config._
77
import Symbols._, SymDenotations._, Types._, Contexts._, Decorators._, Flags._, Names._, NameOps._
8-
import StdNames._, Denotations._, Scopes._, Constants.Constant
8+
import StdNames._, Denotations._, Scopes._, Constants.Constant, SymUtils._
99
import Annotations._
1010
import util.Positions._
1111
import scala.collection.{ mutable, immutable }
@@ -29,9 +29,6 @@ object RefChecks {
2929
def apply(pre: Type, name: Name)(implicit ctx: Context): Boolean = isDefaultGetter(name)
3030
}
3131

32-
private val AnyOverride = Override | AbsOverride
33-
private val AnyOverrideOrSynthetic = AnyOverride | Synthetic
34-
3532
/** Only one overloaded alternative is allowed to define default arguments */
3633
private def checkOverloadedRestrictions(clazz: Symbol)(implicit ctx: Context): Unit = {
3734
// Using the default getters (such as methodName$default$1) as a cheap way of
@@ -254,8 +251,10 @@ object RefChecks {
254251
overrideError("cannot be used here - classes can only override abstract types")
255252
} else if (other.isEffectivelyFinal) { // (1.2)
256253
overrideError(i"cannot override final member ${other.showLocated}")
257-
} else if (!other.is(Deferred) && !isDefaultGetter(other.name) && !member.is(AnyOverrideOrSynthetic)) {
258-
// (*) Synthetic exclusion for (at least) default getters, fixes SI-5178. We cannot assign the OVERRIDE flag to
254+
} else if (!other.is(Deferred) &&
255+
!isDefaultGetter(other.name) &&
256+
!member.isAnyOverride) {
257+
// (*) Exclusion for default getters, fixes SI-5178. We cannot assign the Override flag to
259258
// the default getter: one default getter might sometimes override, sometimes not. Example in comment on ticket.
260259
if (member.owner != clazz && other.owner != clazz && !(other.owner derivesFrom member.owner))
261260
emitOverrideError(
@@ -266,13 +265,13 @@ object RefChecks {
266265
overrideError("needs `override' modifier")
267266
} else if (other.is(AbsOverride) && other.isIncompleteIn(clazz) && !member.is(AbsOverride)) {
268267
overrideError("needs `abstract override' modifiers")
269-
} else if (member.is(AnyOverride) && other.is(Accessor) &&
268+
} else if (member.is(Override) && other.is(Accessor) &&
270269
other.accessedFieldOrGetter.is(Mutable, butNot = Lazy)) {
271270
// !?! this is not covered by the spec. We need to resolve this either by changing the spec or removing the test here.
272271
// !!! is there a !?! convention? I'm !!!ing this to make sure it turns up on my searches.
273272
if (!ctx.settings.overrideVars.value)
274273
overrideError("cannot override a mutable variable")
275-
} else if (member.is(AnyOverride) &&
274+
} else if (member.isAnyOverride &&
276275
!(member.owner.thisType.baseClasses exists (_ isSubClass other.owner)) &&
277276
!member.is(Deferred) && !other.is(Deferred) &&
278277
intersectionIsEmpty(member.extendedOverriddenSymbols, other.extendedOverriddenSymbols)) {
@@ -550,7 +549,7 @@ object RefChecks {
550549

551550
// 4. Check that every defined member with an `override` modifier overrides some other member.
552551
for (member <- clazz.info.decls)
553-
if (member.is(AnyOverride) && !(clazz.thisType.baseClasses exists (hasMatchingSym(_, member)))) {
552+
if (member.isAnyOverride && !(clazz.thisType.baseClasses exists (hasMatchingSym(_, member)))) {
554553
// for (bc <- clazz.info.baseClasses.tail) Console.println("" + bc + " has " + bc.info.decl(member.name) + ":" + bc.info.decl(member.name).tpe);//DEBUG
555554

556555
val nonMatching = clazz.info.member(member.name).altsWith(alt => alt.owner != clazz && !alt.is(Final))
@@ -563,7 +562,8 @@ object RefChecks {
563562
val superSigs = ms.map(_.showDcl).mkString("\n")
564563
issueError(s".\nNote: the super classes of ${member.owner} contain the following, non final members named ${member.name}:\n${superSigs}")
565564
}
566-
member.resetFlag(AnyOverride)
565+
member.resetFlag(Override)
566+
member.resetFlag(AbsOverride)
567567
}
568568
}
569569

0 commit comments

Comments
 (0)