Skip to content
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

Suppress "extension method will never be selected" for overrides #20164

Merged
merged 2 commits into from
Apr 11, 2024
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
53 changes: 28 additions & 25 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1108,6 +1108,8 @@ object RefChecks {
* An extension method is hidden if it does not offer a parameter that is not subsumed
* by the corresponding parameter of the member with the same name (or of all alternatives of an overload).
*
* This check is suppressed if this method is an override.
*
* For example, it is not possible to define a type-safe extension `contains` for `Set`,
* since for any parameter type, the existing `contains` method will compile and would be used.
*
Expand All @@ -1125,31 +1127,32 @@ object RefChecks {
* If the extension method is nullary, it is always hidden by a member of the same name.
* (Either the member is nullary, or the reference is taken as the eta-expansion of the member.)
*/
def checkExtensionMethods(sym: Symbol)(using Context): Unit = if sym.is(Extension) then
extension (tp: Type)
def strippedResultType = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true).resultType
def firstExplicitParamTypes = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true).firstParamTypes
def hasImplicitParams = tp.stripPoly match { case mt: MethodType => mt.isImplicitMethod case _ => false }
val target = sym.info.firstExplicitParamTypes.head // required for extension method, the putative receiver
val methTp = sym.info.strippedResultType // skip leading implicits and the "receiver" parameter
def hidden =
target.nonPrivateMember(sym.name)
.filterWithPredicate:
member =>
val memberIsImplicit = member.info.hasImplicitParams
val paramTps =
if memberIsImplicit then methTp.stripPoly.firstParamTypes
else methTp.firstExplicitParamTypes

paramTps.isEmpty || memberIsImplicit && !methTp.hasImplicitParams || {
val memberParamTps = member.info.stripPoly.firstParamTypes
!memberParamTps.isEmpty
&& memberParamTps.lengthCompare(paramTps) == 0
&& memberParamTps.lazyZip(paramTps).forall((m, x) => x frozen_<:< m)
}
.exists
if !target.typeSymbol.denot.isAliasType && !target.typeSymbol.denot.isOpaqueAlias && hidden
then report.warning(ExtensionNullifiedByMember(sym, target.typeSymbol), sym.srcPos)
def checkExtensionMethods(sym: Symbol)(using Context): Unit =
if sym.is(Extension) && !sym.nextOverriddenSymbol.exists then
Copy link
Contributor

Choose a reason for hiding this comment

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

the actual diff

extension (tp: Type)
def strippedResultType = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true).resultType
def firstExplicitParamTypes = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true).firstParamTypes
def hasImplicitParams = tp.stripPoly match { case mt: MethodType => mt.isImplicitMethod case _ => false }
val target = sym.info.firstExplicitParamTypes.head // required for extension method, the putative receiver
val methTp = sym.info.strippedResultType // skip leading implicits and the "receiver" parameter
def hidden =
target.nonPrivateMember(sym.name)
.filterWithPredicate:
member =>
val memberIsImplicit = member.info.hasImplicitParams
val paramTps =
if memberIsImplicit then methTp.stripPoly.firstParamTypes
else methTp.firstExplicitParamTypes

paramTps.isEmpty || memberIsImplicit && !methTp.hasImplicitParams || {
val memberParamTps = member.info.stripPoly.firstParamTypes
!memberParamTps.isEmpty
&& memberParamTps.lengthCompare(paramTps) == 0
&& memberParamTps.lazyZip(paramTps).forall((m, x) => x frozen_<:< m)
}
.exists
if !target.typeSymbol.denot.isAliasType && !target.typeSymbol.denot.isOpaqueAlias && hidden
then report.warning(ExtensionNullifiedByMember(sym, target.typeSymbol), sym.srcPos)
end checkExtensionMethods

/** Verify that references in the user-defined `@implicitNotFound` message are valid.
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotc/pos-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ strict-pattern-bindings-3.0-migration.scala
i17186b.scala
i11982a.scala
i17255
i17735.scala

# Tree is huge and blows stack for printing Text
i7034.scala
Expand Down
12 changes: 12 additions & 0 deletions tests/pos/ext-override.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//> using options -Xfatal-warnings

trait Foo[T]:
extension (x: T)
def hi: String

class Bla:
def hi: String = "hi"
object Bla:
given Foo[Bla] with
extension (x: Bla)
def hi: String = x.hi
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//> using options -Wvalue-discard
//> using options -Xfatal-warnings -Wvalue-discard

Copy link
Contributor

Choose a reason for hiding this comment

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

i.e. -Werror

import scala.collection.mutable
import scala.annotation.nowarn
Expand All @@ -21,4 +21,4 @@ object Foo:
// here @nowarn is effective without -Wfatal-warnings (i.e. no warning)
// But with -Wfatal-warnings we get an error
messageBuilder.append("\n").append(s): @nowarn("msg=discarded non-Unit value*")
messageBuilder.result()
messageBuilder.result()
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//> using options -Wvalue-discard -Wconf:msg=non-Unit:s
//> using options -Xfatal-warnings -Wvalue-discard -Wconf:msg=non-Unit:s

import scala.collection.mutable
import scala.annotation.nowarn
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//> using options -Wnonunit-statement
//> using options -Xfatal-warnings -Wnonunit-statement

class Node()
class Elem(
Expand Down Expand Up @@ -29,4 +29,4 @@ object Main {
)
}
}: @annotation.nowarn()
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//> using options -Xfatal-warnings -Wvalue-discard

case class F(i: Int)

object Main {
Expand Down
Loading