8000 Judicious shadowing of implicits from scope · scala/scala@2b25baf · GitHub
[go: up one dir, main page]
Skip to content

Commit 2b25baf

Browse files
som-snyttsom-snytt
committed
Judicious shadowing of implicits from scope
1 parent e19b69f commit 2b25baf

File tree

17 files changed

+243
-107
lines changed

17 files changed

+243
-107
lines changed

‎src/compiler/scala/tools/nsc/typechecker/Contexts.scala

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,15 +1018,15 @@ trait Contexts { self: Analyzer with ImportTracking =>
10181018

10191019
/** A symbol `sym` qualifies as an implicit if it has the IMPLICIT flag set,
10201020
* it is accessible, and if it is imported there is not already a local symbol
1021-
* with the same names. Local symbols override imported ones. This fixes #2866.
1021+
* with the same name. Local symbols override imported ones. This fixes #2866.
10221022
*/
10231023
private def isQualifyingImplicit(name: Name, sym: Symbol, pre: Type, imported: Boolean) =
10241024
sym.isImplicit &&
1025-
isAccessible(sym, pre) &&
1025+
isAccessible(sym, pre)/* &&
10261026
!(imported && {
10271027
val e = scope.lookupEntry(name)
10281028
(e ne null) && (e.owner == scope) && e.sym.exists
1029-
})
1029+
})*/
10301030

10311031
/** Do something with the symbols with name `name` imported via the import in `imp`,
10321032
* if any such symbol is accessible from this context and is a qualifying implicit.
@@ -1081,7 +1081,7 @@ trait Contexts { self: Analyzer with ImportTracking =>
10811081
*/
10821082
final def implicitss: List[List[ImplicitInfo]] = implicitssImpl(NoSymbol)
10831083

1084-
private def implicitssImpl(skipClass: Symbol): List[List[ImplicitInfo]] = {
1084+
private def implicitssImpl(skipClass: Symbol): List[List[ImplicitInfo]] =
10851085
if (this == NoContext) Nil
10861086
else if (owner == skipClass) outer.implicitssImpl(NoSymbol)
10871087
else {
@@ -1099,46 +1099,63 @@ trait Contexts { self: Analyzer with ImportTracking =>
10991099
if (implicitsRunId == CycleMarker) {
11001100
debuglog(s"cycle while collecting implicits at owner ${owner}, probably due to an implicit without an explicit return type. Continuing with implicits from enclosing contexts.")
11011101
withOuter(Nil)
1102-
} else if (implicitsRunId != currentRunId) {
1102+
}
1103+
else if (implicitsRunId != currentRunId) {
11031104
implicitsRunId = CycleMarker
11041105
implicits match {
11051106
case None =>
11061107
implicitsRunId = NoRunId
11071108
withOuter(Nil)
11081109
case Some(is) =>
11091110
implicitsRunId = currentRunId
1110-
implicitsCache = is
1111-
withOuter(is)
1111+
val terms = is.filter(_.sym.isTerm)
1112+
implicitsCache = terms
1113+
withOuter(terms)
11121114
}
11131115
}
11141116
else withOuter(implicitsCache)
11151117
}
1116-
}
11171118

1118-
/** @return None if a cycle is detected, or Some(infos) containing the in-scope implicits at this context */
1119+
/** Optionally collect the implicits at this context.
1120+
*
1121+
* This method picks a scope from which implicits are extracted. That can be class members,
1122+
* local scope, an import, or a package object.
1123+
*
1124+
* Some contexts are skipped by implicitssImpl, see above.
1125+
*
1126+
* @return None if a cycle is detected, or Some(infos) containing the in-scope implicits at this context
1127+
*/
11191128
private def implicits: Option[List[ImplicitInfo]] = {
1120-
val firstImport = this.firstImport
11211129
if (unit.isJava) SomeOfNil
11221130
else if (owner != outer.owner && owner.isClass && !owner.isPackageClass) {
11231131
if (!owner.isInitialized) None
11241132
else savingEnclClass(this) {
1125-
// !!! In the body of `class C(implicit a: A) { }`, `implicitss` returns `List(List(a), List(a), List(<predef..)))`
1126-
// it handled correctly by implicit search, which considers the second `a` to be shadowed, but should be
1127-
// remedied nonetheless.
11281133
Some(collectImplicits(owner.thisType.implicitMembers, owner.thisType))
1134+
.tap(res => debuglog(s"collect implicits in ${owner.thisType} from ${owner.thisType.implicitMembers.toList} owner $owner"))
11291135
}
11301136
} else if (scope != outer.scope && !owner.isPackageClass) {
1131-
debuglog("collect local implicits " + scope.toList)//DEBUG
1132-
Some(collectImplicits(scope, NoPrefix))
1133-
} else if (firstImport != outer.firstImport) {
1134-
if (isDeveloper)
1135-
assert(imports.tail.headOption == outer.firstImport, (imports, outer.imports))
1136-
Some(collectImplicitImports(firstImport.get))
1137-
} else if (owner.isPackageClass) {
1138-
// the corresponding package object may contain implicit members.
1139-
val pre = owner.packageObject.typeOfThis
1140-
Some(collectImplicits(pre.implicitMembers, pre))
1141-
} else SomeOfNil
1137+
if (scope.reverseIterator.forall(_.owner == enclClass.owner)) {
1138+
debuglog(s"omit implicit class-owned members in ${scope.toList}")
1139+
SomeOfNil
1140+
}
1141+
else
1142+
Some(collectImplicits(scope, NoPrefix))
1143+
.tap(res => debuglog(s"collect local implicits from ${scope.toList}"))
1144+
} else {
1145+
val firstImport = this.firstImport
1146+
if (firstImport != outer.firstImport) {
1147+
if (isDeveloper)
1148+
assert(imports.tail.headOption == outer.firstImport, (imports, outer.imports))
1149+
Some(collectImplicitImports(firstImport.get))
1150+
.tap(res => debuglog(s"collect implicits from import at $firstImport: $res"))
1151+
} else if (owner.isPackageClass) {
1152+
// the corresponding package object may contain implicit members.
1153+
val pre = owner.packageObject.typeOfThis
1154+
Some(collectImplicits(pre.implicitMembers, pre))
1155+
.tap(res => debuglog(s"collect implicits in package object $pre: $res"))
1156+
} else
1157+
SomeOfNil
1158+
}
11421159
}
11431160

11441161
//

‎src/compiler/scala/tools/nsc/typechecker/Implicits.scala

Lines changed: 78 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ package typechecker
2222
import scala.annotation.{nowarn, tailrec}
2323
import scala.collection.mutable, mutable.{LinkedHashMap, ListBuffer}
2424
import scala.language.implicitConversions
25-
import scala.reflect.internal.util.{ReusableInstance, Statistics, TriState}
25+
import scala.reflect.internal.util.{Statistics, TriState}
2626
import scala.reflect.internal.TypesStats
2727
import scala.tools.nsc.Reporting.WarningCategory.{Scala3Migration, WFlagSelfImplicit}
2828
import symtab.Flags._
@@ -88,11 +88,10 @@ trait Implicits extends splain.SplainData {
8888
*/
8989
def inferImplicit(tree: Tree, pt: Type, reportAmbiguous: Boolean, isView: Boolean, context: Context, saveAmbiguousDivergent: Boolean, pos: Position): SearchResult = {
9090
currentRun.profiler.beforeImplicitSearch(pt)
91-
try {
91+
try
9292
inferImplicit1(tree, pt, reportAmbiguous, isView, context, saveAmbiguousDivergent, pos)
93-
} finally {
93+
finally
9494
currentRun.profiler.afterImplicitSearch(pt)
95-
}
9695
}
9796

9897
private def inferImplicit1(tree: Tree, pt: Type, reportAmbiguous: Boolean, isView: Boolean, context: Context, saveAmbiguousDivergent: Boolean, pos: Position): SearchResult = {
@@ -239,7 +238,7 @@ trait Implicits extends splain.SplainData {
239238
private val improvesCache = perRunCaches.newMap[(ImplicitInfo, ImplicitInfo), Boolean]()
240239
private val implicitSearchId = { var id = 1 ; () => try id finally id += 1 }
241240

242-
private val shadowerUseOldImplementation = java.lang.Boolean.getBoolean("scalac.implicit.shadow.old")
241+
//private val shadowerUseOldImplementation = java.lang.Boolean.getBoolean("scalac.implicit.shadow.old")
243242
def resetImplicits(): Unit = {
244243
implicitsCache.clear()
245244
infoMapCache.clear()
@@ -943,8 +942,7 @@ trait Implicits extends splain.SplainData {
943942
else if (itree3.isErroneous)
944943
fail("error typechecking implicit candidate")
945944
else if (isLocalToCallsite && !hasMatchingSymbol(itree2))
946-
fail("candidate implicit %s is shadowed by %s".format(
947-
info.sym.fullLocationString, itree2.symbol.fullLocationString))
945+
fail(s"candidate implicit ${info.sym.fullLocationString} is shadowed by ${itree2.symbol.fullLocationString}")
948946
else {
949947
val tvars = undetParams map freshVar
950948
val ptInstantiated = pt.instantiateTypeParams(undetParams, tvars)
@@ -1035,7 +1033,8 @@ trait Implicits extends splain.SplainData {
10351033
* - the symbol's type is initialized
10361034
* - the symbol comes from a classfile
10371035
* - the symbol comes from a different sourcefile than the current one
1038-
* - the symbol and the accessed symbol's definitions come before, and do not contain the closest enclosing definition, // see #3373
1036+
* - the symbol comes earlier in the source file but is not an enclosing definition
1037+
* - ditto for the accessed of an accessor (#3373)
10391038
* - the symbol's definition is a val, var, or def with an explicit result type
10401039
* The aim of this method is to prevent premature cyclic reference errors
10411040
* by computing the types of only those implicits for which one of these
@@ -1059,17 +1058,20 @@ trait Implicits extends splain.SplainData {
10591058
if (sym.hasAccessorFlag) {
10601059
val symAcc = sym.accessed // #3373
10611060
symAcc.pos.pointOrElse(0) < ownerPos &&
1062-
!(owner.ownerChain exists (o => (o eq sym) || (o eq symAcc))) // probably faster to iterate only once, don't feel like duplicating hasTransOwner for this case
1063-
} else !(owner hasTransOwner sym)) // faster than owner.ownerChain contains sym
1064-
}
1065-
1066-
sym.isInitialized || {
1067-
val sourceFile = sym.sourceFile
1068-
sourceFile == null ||
1069-
(sourceFile ne context.unit.source.file) ||
1070-
hasExplicitResultType(sym) ||
1071-
comesBefore(sym, context.owner)
1061+
!owner.ownerChain.exists(o => (o eq sym) || (o eq symAcc))
1062+
// probably faster to iterate only once, avoid duplicating hasTransOwner for this case
1063+
}
1064+
else !owner.hasTransOwner(sym) // faster than owner.ownerChain contains sym
1065+
)
10721066
}
1067+
( sym.isInitialized
1068+
|| hasExplicitResultType(sym)
1069+
|| (sym.sourceFile match {
1070+
case null => true
1071+
case sourceFile => sourceFile ne context.unit.source.file
1072+
})
1073+
|| comesBefore(sym, context.owner)
1074+
)
10731075
}
10741076

10751077
/** Prune ImplicitInfos down to either all the eligible ones or the best one.
@@ -1088,7 +1090,7 @@ trait Implicits extends splain.SplainData {
10881090
|| (!context.macrosEnabled && info.sym.isTermMacro)
10891091
)
10901092

1091-
/** True if a given ImplicitInfo (already known isValid) is eligible.
1093+
/** True if a given ImplicitInfo that isValid is not ineligible and matches the expected type.
10921094
*/
10931095
@nowarn("cat=lint-inaccessible")
10941096
def survives(info: ImplicitInfo, shadower: Shadower) = (
@@ -1100,7 +1102,7 @@ trait Implicits extends splain.SplainData {
11001102
/** The implicits that are not valid because they come later in the source and
11011103
* lack an explicit result type. Used for error diagnostics only.
11021104
*/
1103-
val invalidImplicits = new ListBuffer[Symbol]
1105+
val invalidImplicits = ListBuffer.empty[Symbol]
11041106

11051107
/** Tests for validity and updates invalidImplicits by side effect when false.
11061108
*/
@@ -1145,6 +1147,7 @@ trait Implicits extends splain.SplainData {
11451147
}
11461148
}
11471149

1150+
/*
11481151
/** Sorted list of eligible implicits.
11491152
*/
11501153
private def eligibleOld = Shadower.using(isLocalToCallsite) { shadower =>
@@ -1220,13 +1223,26 @@ trait Implicits extends splain.SplainData {
12201223
}
12211224
if (removed) matches.removeIf(_ == null) // remove for real now.
12221225
}
1223-
val result = new ListBuffer[ImplicitInfo]
1226+
val result = ListBuffer.empty[ImplicitInfo]
12241227
matches.forEach(x => result += x.info)
12251228
result.toList
12261229
}
12271230
}
1231+
*/
12281232

1229-
val eligible: List[ImplicitInfo] = if (shadowerUseOldImplementation) eligibleOld else eligibleNew
1233+
val eligibleValidMatching: List[ImplicitInfo] =
1234+
iss.flatMap(_.filter(info => checkValid(info.sym) && survives(info, NoShadower)))
1235+
1236+
val eligible: List[ImplicitInfo] = eligibleValidMatching
1237+
/*
1238+
val eligible: List[ImplicitInfo] = {
1239+
val prev = eligibleNew
1240+
if (!eligibleValidMatching.corresponds(prev)((a, b) => a.sym == b.sym))
1241+
debuglog(s"Eligible diff: ${eligibleValidMatching.diff(prev)}")
1242+
eligibleValidMatching
1243+
}
1244+
//val eligible: List[ImplicitInfo] = if (shadowerUseOldImplementation) eligibleOld else eligibleNew
1245+
*/
12301246
if (eligible.nonEmpty)
12311247
printTyping(tree, s"${eligible.size} eligible for pt=$pt at ${fullSiteString(context)}")
12321248

@@ -1270,8 +1286,9 @@ trait Implicits extends splain.SplainData {
12701286
foreach2(undetParams, savedInfos){ (up, si) => up.setInfo(si) }
12711287
}
12721288
}
1289+
// Don't accumulate constraints from typechecking or type error message creation for failed candidates
12731290
if (typedFirstPending.isFailure)
1274-
undoLog.undoTo(mark) // Don't accumulate constraints from typechecking or type error message creation for failed candidates
1291+
undoLog.undoTo(mark)
12751292

12761293
// Pass the errors to `DivergentImplicitRecovery` so that it can note
12771294
// the first `DivergentImplicitTypeError` that is being propagated
@@ -1302,13 +1319,13 @@ trait Implicits extends splain.SplainData {
13021319
// earlier elems may improve on later ones, but not the other way.
13031320
// So if there is any element not improved upon by the first it is an error.
13041321
rankImplicits(eligible, Nil) match {
1305-
case Nil => ()
1322+
case Nil =>
13061323
case (chosenResult, chosenInfo) :: rest =>
1307-
rest find { case (_, alt) => !improves(chosenInfo, alt) } match {
1308-
case Some((competingResult, competingInfo)) =>
1324+
rest.find { case (_, alt) => !improves(chosenInfo, alt) } match {
1325+
case Some((competingResult, competingInfo)) =>
13091326
AmbiguousImplicitError(chosenInfo, chosenResult.tree, competingInfo, competingResult.tree, "both", "and", "")(isView, pt, tree)(context)
13101327
return AmbiguousSearchFailure // Stop the search once ambiguity is encountered, see t4457_2.scala
1311-
case _ =>
1328+
case _ =>
13121329
if (isView) chosenInfo.useCountView += 1
13131330
else chosenInfo.useCountArg += 1
13141331
}
@@ -1757,15 +1774,41 @@ trait Implicits extends splain.SplainData {
17571774
val failstart = if (stats) statistics.startTimer(inscopeFailNanos) else null
17581775
val succstart = if (stats) statistics.startTimer(inscopeSucceedNanos) else null
17591776

1760-
var result = searchImplicit(context.implicitss, isLocalToCallsite = true)
1777+
// looking up a "candidate" implicit from this context must yield the candidate if not shadowed
1778+
// note that isAccessible is checked by isQualifyingImplicit //&& isAccessible(ii.sym, ii.pre)
1779+
// if lookup fails, it's a stale symbol problem, not our problem (pos/t5639)
1780+
def shadowed(ii: ImplicitInfo): Boolean = {
1781+
val shadowed = context.lookupSymbol(ii.name, _ => true) match {
1782+
case LookupSucceeded(qualifier, symbol) =>
1783+
// ii.pre or skipPackageObject
1784+
val pre = if (ii.pre.typeSymbol.isPackageObjectClass) ii.pre.typeSymbol.owner.module.info else ii.pre
1785+
val ok = (
1786+
symbol.alternatives.exists(_ == ii.sym)
1787+
&& (qualifier.isEmpty || qualifier.tpe =:= pre)
1788+
)
1789+
if (!ok && settings.isDebug) {
1790+
val pretext = if (ii.pre != NoType) s" in ${ii.pre}" else ""
1791+
val qualtext = if (!qualifier.isEmpty) s" in ${qualifier.tpe}" else ""
1792+
debuglog(s"Drop shadowed implicit ${ii.sym.fullLocationString}${pretext} for ${
1793+
symbol.fullLocationString}${qualtext}")
1794+
}
1795+
!ok
1796+
case failure =>
1797+
debuglog(s"Not dropping implicit $ii or ${ii.sym.fullLocationString} on bad lookup $failure in ${
1798+
context.owner}")
1799+
false
1800+
}
1801+
shadowed
1802+
}
1803+
val implicitss = context.implicitss.map(_.filterNot(shadowed)).filter(!_.isEmpty)
1804+
var result = searchImplicit(implicitss, isLocalToCallsite = true)
17611805

1762-
if (stats) {
1806+
if (stats)
17631807
if (result.isFailure) statistics.stopTimer(inscopeFailNanos, failstart)
17641808
else {
17651809
statistics.stopTimer(inscopeSucceedNanos, succstart)
17661810
statistics.incCounter(inscopeImplicitHits)
17671811
}
1768-
}
17691812

17701813
if (result.isFailure) {
17711814
val failstart = if (stats) statistics.startTimer(oftypeFailNanos) else null
@@ -1980,16 +2023,19 @@ trait Implicits extends splain.SplainData {
19802023
def isShadowed(name: Name): Boolean
19812024
}
19822025
object Shadower {
1983-
private[this] val localShadowerCache = ReusableInstance[LocalShadower](new LocalShadower, enabled = isCompilerUniverse)
2026+
/*
2027+
private val localShadowerCache = ReusableInstance[LocalShadower](new LocalShadower, enabled = isCompilerUniverse)
19842028
19852029
def using[T](local: Boolean)(f: Shadower => T): T =
19862030
if (local) localShadowerCache.using { shadower =>
19872031
shadower.clear()
19882032
f(shadower)
19892033
}
19902034
else f(NoShadower)
2035+
*/
19912036
}
19922037

2038+
/*
19932039
/** Used for exclude implicits from outer scopes that are shadowed by same-named implicits */
19942040
private final class LocalShadower extends Shadower {
19952041
// OPT: using j.l.HashSet as that retains the internal array on clear(), which makes it worth caching.
@@ -2000,6 +2046,7 @@ trait Implicits extends splain.SplainData {
20002046
def isShadowed(name: Name) = shadowed.contains(name)
20012047
def clear(): Unit = shadowed.clear()
20022048
}
2049+
*/
20032050
/** Used for the implicits of expected type, when no shadowing checks are needed. */
20042051
private object NoShadower extends Shadower {
20052052
def addInfos(infos: Infos): Unit = {}

0 commit comments

Comments
 (0)
0