Skip to content

Fix #9928: Don't treat import contexts as properly nested #9950

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 5 commits into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ object Contexts {
else
outer.implicits
if (implicitRefs.isEmpty) outerImplicits
else new ContextualImplicits(implicitRefs, outerImplicits)(this)
else new ContextualImplicits(implicitRefs, outerImplicits, isImportContext)(this)
}
implicitsCache
}
Expand Down Expand Up @@ -777,7 +777,7 @@ object Contexts {

@sharable object NoContext extends Context(null) {
source = NoSource
override val implicits: ContextualImplicits = new ContextualImplicits(Nil, null)(this)
override val implicits: ContextualImplicits = new ContextualImplicits(Nil, null, false)(this)
}

/** A context base defines state and associated methods that exist once per
Expand Down
10 changes: 7 additions & 3 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,10 @@ object Implicits:
* name, b, whereas the name of the symbol is the original name, a.
* @param outerCtx the next outer context that makes visible further implicits
*/
class ContextualImplicits(val refs: List[ImplicitRef], val outerImplicits: ContextualImplicits)(initctx: Context) extends ImplicitRefs(initctx) {
class ContextualImplicits(
val refs: List[ImplicitRef],
val outerImplicits: ContextualImplicits,
isImport: Boolean)(initctx: Context) extends ImplicitRefs(initctx) {
private val eligibleCache = EqHashMap[Type, List[Candidate]]()

/** The level increases if current context has a different owner or scope than
Expand All @@ -304,7 +307,8 @@ object Implicits:
*/
override val level: Int =
if outerImplicits == null then 1
else if migrateTo3(using irefCtx)
else if isImport && (irefCtx.owner eq outerImplicits.irefCtx.owner)
Copy link
Contributor

Choose a reason for hiding this comment

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

Bikeshed: If desired, readability could be slightly improved by refactoring this boolean expression to:

      inline def isSameOwner = irefCtx.owner eq outerImplicits.irefCtx.owner
      inline def isSameScope = irefCtx.scope eq outerImplicits.irefCtx.scope
      inline def isLazyImplicit = refs.head.implicitName.is(LazyImplicitName)

      if outerImplicits == null then 1
      else if migrateTo3(using irefCtx)
              || isSameOwner && (isImport || isSameScope && !isLazyImplicit)
      then outerImplicits.level
      else outerImplicits.level + 1

This way imports also might look a little bit less special than now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true. And I don't think we need the inline; the JVM is good at inlining these anyway.

|| migrateTo3(using irefCtx)
|| (irefCtx.owner eq outerImplicits.irefCtx.owner)
&& (irefCtx.scope eq outerImplicits.irefCtx.scope)
&& !refs.head.implicitName.is(LazyImplicitName)
Expand Down Expand Up @@ -370,7 +374,7 @@ object Implicits:
val outerExcluded = outerImplicits exclude root
if (irefCtx.importInfo.site.termSymbol == root) outerExcluded
else if (outerExcluded eq outerImplicits) this
else new ContextualImplicits(refs, outerExcluded)(irefCtx)
else new ContextualImplicits(refs, outerExcluded, isImport)(irefCtx)
}
}

Expand Down
22 changes: 12 additions & 10 deletions compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import ast.{untpd, tpd}
import Implicits.{hasExtMethod, Candidate}
import java.util.{Timer, TimerTask}
import collection.mutable
import scala.util.control.NonFatal

/** This trait defines the method `importSuggestionAddendum` that adds an addendum
* to error messages suggesting additional imports.
Expand Down Expand Up @@ -58,10 +59,12 @@ trait ImportSuggestions:
val seen = mutable.Set[TermRef]()

def lookInside(root: Symbol)(using Context): Boolean =
if root.is(Package) then root.isTerm && root.isCompleted
else !root.name.is(FlatName)
&& !root.name.lastPart.contains('$')
&& root.is(ModuleVal, butNot = JavaDefined)
explore {
if root.is(Package) then root.isTerm && root.isCompleted
else !root.name.is(FlatName)
&& !root.name.lastPart.contains('$')
&& root.is(ModuleVal, butNot = JavaDefined)
}

def nestedRoots(site: Type)(using Context): List[Symbol] =
val seenNames = mutable.Set[Name]()
Expand Down Expand Up @@ -245,12 +248,11 @@ trait ImportSuggestions:
match
case (Nil, partials) => (extensionImports, partials)
case givenImports => givenImports
catch
case ex: Throwable =>
if ctx.settings.Ydebug.value then
println("caught exception when searching for suggestions")
ex.printStackTrace()
(Nil, Nil)
catch case NonFatal(ex) =>
if ctx.settings.Ydebug.value then
println("caught exception when searching for suggestions")
ex.printStackTrace()
(Nil, Nil)
finally
timer.cancel()
reduceTimeBudget(((System.currentTimeMillis() - start) min Int.MaxValue).toInt)
Expand Down
26 changes: 26 additions & 0 deletions tests/neg/i9928.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
trait Magic[F]:
extension (x: Int) def read: F

object Magic:
given Magic[String]:
extension(x: Int) def read: String =
println("In string")
s"$x"

opaque type Foo = String
object Foo:
import Magic.{given _}
def apply(s: String): Foo = s

given Magic[Foo]:
extension (x: Int) def read: Foo =
println("In foo")
Foo(s"$x")

def test: Unit =
(3.read: Foo) // error: ambiguous


@main def Test = {
Foo.test
}
2 changes: 2 additions & 0 deletions tests/run/i9928.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
In foo
In foo
44 changes: 44 additions & 0 deletions tests/run/i9928.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
trait Magic[F]:
extension (x: Int) def read: F

trait LowPrio:
given Magic[String]:
extension(x: Int) def read: String =
println("In string")
s"$x"

object test1:
object Magic extends LowPrio

opaque type Foo = String
object Foo extends LowPrio:
import Magic.{given _}
def apply(s: String): Foo = s

given Magic[Foo]:
extension (x: Int) def read: Foo =
println("In foo")
Foo(s"$x")

def test: Unit =
(3.read: Foo)

object test2:
object Magic extends LowPrio:
given Magic[Foo]:
extension (x: Int) def read: Foo =
println("In foo")
Foo(s"$x")

opaque type Foo = String
object Foo extends LowPrio:
import Magic.{given _}
def apply(s: String): Foo = s

def test: Unit =
(3.read: Foo)


@main def Test =
test1.Foo.test
test2.Foo.test