Skip to content

Fix #7597: Refine checks whether a deferred term member is implemented #8332

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
Feb 22, 2020
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
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2377,7 +2377,7 @@ object SymDenotations {
/** is the cache valid in current run at given phase? */
def isValidAt(phase: Phase)(implicit ctx: Context): Boolean

/** Render invalid this cache and all cache that depend on it */
/** Render invalid this cache and all caches that depend on it */
def invalidate(): Unit
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import core._
import Contexts._, Types._, Symbols._, Names._, Decorators._, ProtoTypes._
import Flags._
import NameKinds.FlatName
import config.Printers.implicits
import config.Printers.{implicits, implicitsDetailed}
import util.Spans.Span
import ast.{untpd, tpd}
import Implicits.{hasExtMethod, Candidate}
Expand Down Expand Up @@ -94,7 +94,7 @@ trait ImportSuggestions:
def rootsIn(ref: TermRef)(using Context): List[TermRef] =
if seen.contains(ref) then Nil
else
implicits.println(i"search for suggestions in ${ref.symbol.fullName}")
implicitsDetailed.println(i"search for suggestions in ${ref.symbol.fullName}")
seen += ref
ref :: rootsStrictlyIn(ref)

Expand Down
60 changes: 39 additions & 21 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ object RefChecks {
* TODO This still needs to be cleaned up; the current version is a straight port of what was there
* before, but it looks too complicated and method bodies are far too large.
*/
private def checkAllOverrides(clazz: Symbol)(implicit ctx: Context): Unit = {
private def checkAllOverrides(clazz: ClassSymbol)(implicit ctx: Context): Unit = {
val self = clazz.thisType
val upwardsSelf = upwardsThisType(clazz)
var hasErrors = false
Expand Down Expand Up @@ -470,33 +470,52 @@ object RefChecks {
}
}

def ignoreDeferred(member: SingleDenotation) =
member.isType || {
val mbr = member.symbol
mbr.isSuperAccessor || // not yet synthesized
ShortcutImplicits.isImplicitShortcut(mbr) || // only synthesized when referenced, see Note in ShortcutImplicits
mbr.is(JavaDefined) && hasJavaErasedOverriding(mbr)
}
def ignoreDeferred(mbr: Symbol) =
mbr.isType
|| mbr.isSuperAccessor // not yet synthesized
|| ShortcutImplicits.isImplicitShortcut(mbr) // only synthesized when referenced, see Note in ShortcutImplicits
|| mbr.is(JavaDefined) && hasJavaErasedOverriding(mbr)

def isImplemented(mbr: Symbol) =
val mbrType = clazz.thisType.memberInfo(mbr)
def (sym: Symbol).isConcrete = sym.exists && !sym.is(Deferred)
clazz.nonPrivateMembersNamed(mbr.name)
.filterWithPredicate(
impl => impl.symbol.isConcrete && mbrType.matchesLoosely(impl.info))
.exists

/** The term symbols in this class and its baseclasses that are
* abstract in this class. We can't use memberNames for that since
* a concrete member might have the same signature as an abstract
* member in a base class, yet might not override it.
*/
def missingTermSymbols: List[Symbol] =
val buf = new mutable.ListBuffer[Symbol]
for bc <- clazz.baseClasses
sym <- bc.info.decls.toList
if sym.is(DeferredTerm) && !isImplemented(sym) && !ignoreDeferred(sym)
do buf += sym
buf.toList

// 2. Check that only abstract classes have deferred members
def checkNoAbstractMembers(): Unit = {
// Avoid spurious duplicates: first gather any missing members.
val missing = clazz.thisType.abstractTermMembers.filterNot(ignoreDeferred)
val missing = missingTermSymbols
// Group missing members by the name of the underlying symbol,
// to consolidate getters and setters.
val grouped = missing.groupBy(_.symbol.underlyingSymbol.name)
val grouped = missing.groupBy(_.underlyingSymbol.name)

val missingMethods = grouped.toList flatMap {
case (name, syms) =>
val withoutSetters = syms filterNot (_.symbol.isSetter)
val withoutSetters = syms filterNot (_.isSetter)
if (withoutSetters.nonEmpty) withoutSetters else syms
}

def stubImplementations: List[String] = {
// Grouping missing methods by the declaring class
val regrouped = missingMethods.groupBy(_.symbol.owner).toList
def membersStrings(members: List[SingleDenotation]) =
members.sortBy(_.symbol.name.toString).map(_.showDcl + " = ???")
val regrouped = missingMethods.groupBy(_.owner).toList
def membersStrings(members: List[Symbol]) =
members.sortBy(_.name.toString).map(_.showDcl + " = ???")

if (regrouped.tail.isEmpty)
membersStrings(regrouped.head._2)
Expand All @@ -520,22 +539,21 @@ object RefChecks {
}

for (member <- missing) {
val memberSym = member.symbol
def undefined(msg: String) =
abstractClassError(false, s"${member.showDcl} is not defined $msg")
val underlying = memberSym.underlyingSymbol
val underlying = member.underlyingSymbol

// Give a specific error message for abstract vars based on why it fails:
// It could be unimplemented, have only one accessor, or be uninitialized.
if (underlying.is(Mutable)) {
val isMultiple = grouped.getOrElse(underlying.name(ctx), Nil).size > 1

// If both getter and setter are missing, squelch the setter error.
if (memberSym.isSetter && isMultiple) ()
if (member.isSetter && isMultiple) ()
else undefined(
if (memberSym.isSetter) "\n(Note that an abstract var requires a setter in addition to the getter)"
else if (memberSym.isGetter && !isMultiple) "\n(Note that an abstract var requires a getter in addition to the setter)"
else err.abstractVarMessage(memberSym))
if (member.isSetter) "\n(Note that an abstract var requires a setter in addition to the getter)"
else if (member.isGetter && !isMultiple) "\n(Note that an abstract var requires a getter in addition to the setter)"
else err.abstractVarMessage(member))
}
else if (underlying.is(Method)) {
// If there is a concrete method whose name matches the unimplemented
Expand Down Expand Up @@ -961,7 +979,7 @@ class RefChecks extends MiniPhase { thisPhase =>
}

override def transformTemplate(tree: Template)(implicit ctx: Context): Tree = try {
val cls = ctx.owner
val cls = ctx.owner.asClass
checkOverloadedRestrictions(cls)
checkParents(cls)
if (cls.is(Trait)) tree.parents.foreach(checkParentPrefix(cls, _))
Expand Down
4 changes: 2 additions & 2 deletions tests/explicit-nulls/neg/override-java-object-arg.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import javax.management.{Notification, NotificationEmitter, NotificationListener
class Foo {

def bar(): Unit = {
val listener = new NotificationListener() {
val listener = new NotificationListener() { // error: object creation impossible
override def handleNotification(n: Notification|Null, emitter: Object): Unit = { // error: method handleNotification overrides nothing
}
}
Expand All @@ -17,7 +17,7 @@ class Foo {
}
}

val listener3 = new NotificationListener() {
val listener3 = new NotificationListener() { // error: object creation impossible
override def handleNotification(n: Notification, emitter: Object|Null): Unit = { // error: method handleNotification overrides nothing
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/neg-custom-args/erased/erased-case-class.scala
Original file line number Diff line number Diff line change
@@ -1 +1 @@
case class Foo1(erased x: Int) // error // error
case class Foo1(erased x: Int) // error
14 changes: 14 additions & 0 deletions tests/neg/i7597.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
object Test extends App {
def foo[S <: String]: String => Int =
new (String => Int) { def apply(s: S): Int = 0 } // error

trait Fn[A, B] {
def apply(x: A): B
}

class C[S <: String] extends Fn[String, Int] { // error
def apply(s: S): Int = 0
}

foo("")
}