Skip to content

Fix #2390: Use normal thisType as base for override checking #2580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ object RefChecks {
* before, but it looks too complicated and method bodies are far too large.
*/
private def checkAllOverrides(clazz: Symbol)(implicit ctx: Context): Unit = {
val self = upwardsThisType(clazz)
val self = clazz.thisType
val upwardsSelf = upwardsThisType(clazz)
var hasErrors = false

case class MixinOverrideError(member: Symbol, msg: String)
Expand Down Expand Up @@ -245,17 +246,24 @@ object RefChecks {
(if (otherAccess == "") "public" else "at least " + otherAccess))
}

def compatibleTypes =
if (member.isType) { // intersection of bounds to refined types must be nonempty
member.is(BaseTypeArg) ||
(memberTp frozen_<:< otherTp) || {
val jointBounds = (memberTp.bounds & otherTp.bounds).bounds
jointBounds.lo frozen_<:< jointBounds.hi
def compatibleTypes(memberTp: Type, otherTp: Type): Boolean =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using other names than memberTp and otherTp here, because memberTp and otherTp are already in scope at this point and they could easily be confused when reading the code.

try
if (member.isType) { // intersection of bounds to refined types must be nonempty
member.is(BaseTypeArg) ||
(memberTp frozen_<:< otherTp) || {
val jointBounds = (memberTp.bounds & otherTp.bounds).bounds
jointBounds.lo frozen_<:< jointBounds.hi
}
}
else
member.name.is(DefaultGetterName) || // default getters are not checked for compatibility
memberTp.overrides(otherTp)
catch {
case ex: MissingType =>
// can happen when called with upwardsSelf as qualifier of memberTp and otherTp,
// because in that case we might access types that are not members of the qualifier.
false
}
else
member.name.is(DefaultGetterName) || // default getters are not checked for compatibility
memberTp.overrides(otherTp)

//Console.println(infoString(member) + " overrides " + infoString(other) + " in " + clazz);//DEBUG

Expand Down Expand Up @@ -356,7 +364,8 @@ object RefChecks {
overrideError("cannot be used here - term macros cannot override abstract methods")
} else if (other.is(Macro) && !member.is(Macro)) { // (1.10)
overrideError("cannot be used here - only term macros can override term macros")
} else if (!compatibleTypes) {
} else if (!compatibleTypes(memberTp, otherTp) &&
!compatibleTypes(upwardsSelf.memberInfo(member), upwardsSelf.memberInfo(other))) {
overrideError("has incompatible type" + err.whyNoMatchStr(memberTp, otherTp))
} else {
checkOverrideDeprecated()
Expand Down
11 changes: 11 additions & 0 deletions tests/pos/i2390.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
trait TestSuite {
trait NoArgTest
}

trait TestSuiteMixin { self: TestSuite =>
def foo(test: self.NoArgTest) = {}
}

trait CancelAfterFailure extends TestSuiteMixin { self: TestSuite =>
override def foo(test: self.NoArgTest) = {}
}