From 0908c3a72c4183534fd241d474ae1c04b1b16f8e Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 1 Aug 2023 12:16:23 +0100 Subject: [PATCH 1/2] Fix how implicit candidates are combined --- .../tools/dotc/printing/PlainPrinter.scala | 3 +++ .../dotty/tools/dotc/printing/Printer.scala | 5 +++- .../dotty/tools/dotc/typer/Implicits.scala | 22 +++++++++++++++-- tests/pos/i18316.orig.scala | 24 +++++++++++++++++++ tests/pos/i18316.scala | 13 ++++++++++ 5 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 tests/pos/i18316.orig.scala create mode 100644 tests/pos/i18316.scala diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 6cba2f78776b..489aeb1b52b5 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -641,6 +641,9 @@ class PlainPrinter(_ctx: Context) extends Printer { else if (pos.source.exists) s"${pos.source.file.name}:${pos.line + 1}" else s"(no source file, offset = ${pos.span.point})" + def toText(cand: Candidate): Text = + "Candidate(" ~ toText(cand.ref) ~ ", " ~ Str("kind=" + cand.kind) ~ ", " ~ Str("lvl=" + cand.level) ~ ")" + def toText(result: SearchResult): Text = result match { case result: SearchSuccess => "SearchSuccess: " ~ toText(result.ref) ~ " via " ~ toText(result.tree) diff --git a/compiler/src/dotty/tools/dotc/printing/Printer.scala b/compiler/src/dotty/tools/dotc/printing/Printer.scala index b3e48ab2d843..04cea9fb9702 100644 --- a/compiler/src/dotty/tools/dotc/printing/Printer.scala +++ b/compiler/src/dotty/tools/dotc/printing/Printer.scala @@ -7,7 +7,7 @@ import Texts._, ast.Trees._ import Types.{Type, SingletonType, LambdaParam, NamedType}, Symbols.Symbol, Scopes.Scope, Constants.Constant, Names.Name, Denotations._, Annotations.Annotation, Contexts.Context -import typer.Implicits.SearchResult +import typer.Implicits.* import util.SourcePosition import typer.ImportInfo @@ -153,6 +153,9 @@ abstract class Printer { /** Textual representation of source position */ def toText(pos: SourcePosition): Text + /** Textual representation of implicit candidates. */ + def toText(cand: Candidate): Text + /** Textual representation of implicit search result */ def toText(result: SearchResult): Text diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index e576c6363e39..fce8f711b05e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -55,11 +55,13 @@ object Implicits: } /** An eligible implicit candidate, consisting of an implicit reference and a nesting level */ - case class Candidate(implicitRef: ImplicitRef, kind: Candidate.Kind, level: Int) extends RefAndLevel { + case class Candidate(implicitRef: ImplicitRef, kind: Candidate.Kind, level: Int) extends RefAndLevel with Showable { def ref: TermRef = implicitRef.underlyingRef def isExtension = (kind & Candidate.Extension) != 0 def isConversion = (kind & Candidate.Conversion) != 0 + + def toText(printer: Printer): Text = printer.toText(this) } object Candidate { type Kind = Int @@ -331,6 +333,7 @@ object Implicits: else if outerEligible.isEmpty then ownEligible else def filter(xs: List[Candidate], remove: List[Candidate]) = + // Drop candidates that are shadowed by candidates in "remove" val shadowed = remove.map(_.ref.implicitName).toSet xs.filterConserve(cand => !shadowed.contains(cand.ref.implicitName)) @@ -342,7 +345,22 @@ object Implicits: if !migrateTo3(using irefCtx) && level == outer.level && (preferDefinitions || preferNamedImport) then // special cases: definitions beat imports, and named imports beat // wildcard imports, provided both are in contexts with same scope - filter(ownEligible, outerEligible) ::: outerEligible + + // Using only the outer candidates at the same level as us, + // remove from our own eligibles any shadowed candidate. + // This removes locally imported candidates from shadowing local definitions, (foo's in i18316) + // but without a remotely imported candidate removing a more locally imported candidates (mkFoo's in i18183) + val ownEligible1 = filter(ownEligible, outerEligible.filter(_.level == level)) + + // Remove, from the outer eligibles, any candidate shadowed by one of our own candidates, + // provided that the outer eligibles aren't at the same level (so actually shadows). + // This complements the filtering of our own eligible candidates, by removing candidates in the outer candidates + // that are low-level priority and shadowed by our candidates. E.g. the outer import Imp.mkFoo in i18183. + val shadowed = ownEligible.map(_.ref.implicitName).toSet + val outerEligible1 = + outerEligible.filterConserve(cand => cand.level == level || !shadowed.contains(cand.ref.implicitName)) + + ownEligible1 ::: outerEligible1 else ownEligible ::: filter(outerEligible, ownEligible) diff --git a/tests/pos/i18316.orig.scala b/tests/pos/i18316.orig.scala new file mode 100644 index 000000000000..1d7d6ce8043b --- /dev/null +++ b/tests/pos/i18316.orig.scala @@ -0,0 +1,24 @@ +import scala.language.implicitConversions +object squerel { + trait EqualityExpression + object PrimitiveTypeMode: + implicit def intToTE(f: Int): TypedExpression[Int] = ??? + + trait TypedExpression[A1]: + def ===[A2](b: TypedExpression[A2]): EqualityExpression = ??? +} + +object scalactic { + trait TripleEqualsSupport: + class Equalizer[L](val leftSide: L): + def ===(rightSide: Any): Boolean = ??? + + trait TripleEquals extends TripleEqualsSupport: + implicit def convertToEqualizer[T](left: T): Equalizer[T] = ??? +} + +import squerel.PrimitiveTypeMode._ // remove to make code compile +object Test extends scalactic.TripleEquals { + import squerel.PrimitiveTypeMode._ + val fails: squerel.EqualityExpression = 1 === 1 +} diff --git a/tests/pos/i18316.scala b/tests/pos/i18316.scala new file mode 100644 index 000000000000..82f56a33468a --- /dev/null +++ b/tests/pos/i18316.scala @@ -0,0 +1,13 @@ +class R1 +class R2 + +class Foo { def meth(x: Int): R1 = null } +class Bar { def meth(x: Int): R2 = null } + +object Impl { implicit def mkFoo(i: Int): Foo = null } +trait Trait { implicit def mkBar(i: Int): Bar = null } + +import Impl.mkFoo // remove to make code compile +object Test extends Trait: + import Impl.mkFoo + val fails: R1 = 1.meth(1) From aeb6a9f2a78f64c991641a7b846592ffe56edc2d Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 8 Aug 2023 10:32:33 +0100 Subject: [PATCH 2/2] Simplify combineEligibles logic --- .../tools/dotc/printing/PlainPrinter.scala | 6 ++- .../dotty/tools/dotc/typer/Implicits.scala | 48 +++++++------------ .../src/dotty/tools/dotc/typer/Typer.scala | 9 ++-- 3 files changed, 29 insertions(+), 34 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala index 489aeb1b52b5..4d87d6406567 100644 --- a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala @@ -642,7 +642,11 @@ class PlainPrinter(_ctx: Context) extends Printer { else s"(no source file, offset = ${pos.span.point})" def toText(cand: Candidate): Text = - "Candidate(" ~ toText(cand.ref) ~ ", " ~ Str("kind=" + cand.kind) ~ ", " ~ Str("lvl=" + cand.level) ~ ")" + "Cand(" + ~ toTextRef(cand.ref) + ~ (if cand.isConversion then " conv" else "") + ~ (if cand.isExtension then " ext" else "") + ~ Str(" L" + cand.level) ~ ")" def toText(result: SearchResult): Text = result match { case result: SearchSuccess => diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index fce8f711b05e..b0f476444754 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -23,6 +23,7 @@ import ProtoTypes._ import ErrorReporting._ import Inferencing.{fullyDefinedType, isFullyDefined} import Scopes.newScope +import Typer.BindingPrec, BindingPrec.* import transform.TypeUtils._ import Hashable._ import util.{EqHashMap, Stats} @@ -49,7 +50,7 @@ object Implicits: } /** Both search candidates and successes are references with a specific nesting level. */ - sealed trait RefAndLevel { + sealed trait RefAndLevel extends Showable { def ref: TermRef def level: Int } @@ -328,41 +329,28 @@ object Implicits: (this eq finalImplicits) || (outerImplicits eqn finalImplicits) } + def bindingPrec: BindingPrec = + if isImport then if ctx.importInfo.uncheckedNN.isWildcardImport then WildImport else NamedImport else Definition + private def combineEligibles(ownEligible: List[Candidate], outerEligible: List[Candidate]): List[Candidate] = if ownEligible.isEmpty then outerEligible else if outerEligible.isEmpty then ownEligible else - def filter(xs: List[Candidate], remove: List[Candidate]) = - // Drop candidates that are shadowed by candidates in "remove" - val shadowed = remove.map(_.ref.implicitName).toSet - xs.filterConserve(cand => !shadowed.contains(cand.ref.implicitName)) - + val ownNames = mutable.Set(ownEligible.map(_.ref.implicitName)*) val outer = outerImplicits.uncheckedNN - def isWildcardImport(using Context) = ctx.importInfo.nn.isWildcardImport - def preferDefinitions = isImport && !outer.isImport - def preferNamedImport = isWildcardImport && !isWildcardImport(using outer.irefCtx) - - if !migrateTo3(using irefCtx) && level == outer.level && (preferDefinitions || preferNamedImport) then - // special cases: definitions beat imports, and named imports beat - // wildcard imports, provided both are in contexts with same scope - - // Using only the outer candidates at the same level as us, - // remove from our own eligibles any shadowed candidate. - // This removes locally imported candidates from shadowing local definitions, (foo's in i18316) - // but without a remotely imported candidate removing a more locally imported candidates (mkFoo's in i18183) - val ownEligible1 = filter(ownEligible, outerEligible.filter(_.level == level)) - - // Remove, from the outer eligibles, any candidate shadowed by one of our own candidates, - // provided that the outer eligibles aren't at the same level (so actually shadows). - // This complements the filtering of our own eligible candidates, by removing candidates in the outer candidates - // that are low-level priority and shadowed by our candidates. E.g. the outer import Imp.mkFoo in i18183. - val shadowed = ownEligible.map(_.ref.implicitName).toSet - val outerEligible1 = - outerEligible.filterConserve(cand => cand.level == level || !shadowed.contains(cand.ref.implicitName)) - - ownEligible1 ::: outerEligible1 + if !migrateTo3(using irefCtx) && level == outer.level && outer.bindingPrec.beats(bindingPrec) then + val keptOuters = outerEligible.filterConserve: cand => + if ownNames.contains(cand.ref.implicitName) then + val keepOuter = cand.level == level + if keepOuter then ownNames -= cand.ref.implicitName + keepOuter + else true + val keptOwn = ownEligible.filterConserve: cand => + ownNames.contains(cand.ref.implicitName) + keptOwn ::: keptOuters else - ownEligible ::: filter(outerEligible, ownEligible) + ownEligible ::: outerEligible.filterConserve: cand => + !ownNames.contains(cand.ref.implicitName) def uncachedEligible(tp: Type)(using Context): List[Candidate] = Stats.record("uncached eligible") diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 3aaf4fec59d6..0082805f9ba4 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -65,6 +65,11 @@ object Typer { case NothingBound, PackageClause, WildImport, NamedImport, Inheritance, Definition def isImportPrec = this == NamedImport || this == WildImport + + /** special cases: definitions beat imports, and named imports beat + * wildcard imports, provided both are in contexts with same scope */ + def beats(prevPrec: BindingPrec): Boolean = + this == Definition || this == NamedImport && prevPrec == WildImport } /** Assert tree has a position, unless it is empty or a typed splice */ @@ -226,9 +231,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer def checkNewOrShadowed(found: Type, newPrec: BindingPrec, scala2pkg: Boolean = false)(using Context): Type = if !previous.exists || TypeComparer.isSameRef(previous, found) then found - else if (prevCtx.scope eq ctx.scope) - && (newPrec == Definition || newPrec == NamedImport && prevPrec == WildImport) - then + else if (prevCtx.scope eq ctx.scope) && newPrec.beats(prevPrec) then // special cases: definitions beat imports, and named imports beat // wildcard imports, provided both are in contexts with same scope found