Don't propagate imports into inlined code

[odersky]

Imports are supposed to be resolved already, so they need not and should not be copied into inlined code.

Fixes #16156, unfortuntely in the sense that it will not work.

[odersky]

This is not quite baked yet. There are several questions

• Should the inliner copy imports?
• Can the inlining phase make use of copied imports (in implicit search, maybe?)
• When should such imports be eliminated?

@nicolasstucki WDYT?

[deusaquilus]

Whatever you do, please, please don't make inline completely ignore imports. At least right now I have static imports and I can do this:

object StaticContext:
  inline def summonMyEncoder[T]: String =
    ${ SummonEncoder.impl[T] }
  implicit val encoderInstance: MyEncoder[String] =
    new MyEncoder[String] { def encode = "blah" }
end StaticContext

...and then this:

import StaticContext._ // Imports the `encoderInstance`

class Repo[T]:
  inline def summonEncoder = {
    summonMyEncoder[T]
  }

object Use:
  val repo = new Repo[String]
  val v = repo.summonEncoder

...and then it will import the encoderInstance instance and it will actually work.
(See this branch for a full example: https://github.com/deusaquilus/encoder-typing-reproduction/tree/static-context)

Also, if you define Repo as an extension of Context it'll work too. First I do this:

trait TraitContext:
  inline def summonMyEncoder[T]: String =
    ${ SummonEncoder.impl[T] }
  implicit val encoderInstance: MyEncoder[String] =
    new MyEncoder[String] { def encode = "blah" }
end TraitContex

...and then this:

class Repo[T] extends TraitContext:
  inline def summonEncoder = {
    summonMyEncoder[T]
  }

object Use:
  val repo = new Repo[String]
  import repo._  // Imports the `encoderInstance`
  val v = repo.summonEncoder

Right now, every single Quill context uses this latter pattern. Quill couldn't possibly work without it.
(Here's an example of the working code here: https://github.com/deusaquilus/encoder-typing-reproduction/tree/trait-context)

If inline doesn't work together with imports (in this latter way) then Quill won't work at all.

[odersky]

@deusaquilus @nicolasstucki As far as I can see these are two different situations. One is where the inlined method sees an import in its outer scope. That will of course continue to work. All the examples you gave seem to fall in this category.

The other is where an import is part of an inlined method body. Here I had thought that the import would have been resolved at typer and therefore would be redundant for inlining, so it could be removed. But I am not sure. In particular:

1. Can an inlined import influence implicit search at phase inlining?
2. If the answer to (1) is yes, is that what's intended?

[joroKr21]

We rely a lot on inline imports in Kittens 3 and so far it worked without issues:
https://github.com/typelevel/kittens/blob/master/core/src/main/scala-3/cats/derived/DerivedOrder.scala#L18-L20

[nicolasstucki]

1. Can an inlined import influence implicit search at phase inlining?

This could influence inline summoning (such as summonFrom and Expr.summon). I thought that the current implementation is supposed to ignore imports inserted by inlining and we use the implicits available in the inline call site context. There were some use cases where users wanted to influence inline summoning in some issue (don't remember which one). I will have to investigate.

[nicolasstucki]

While performing inline summoning, we do take into account local given definition. For example, the code generated for test will summon the locally inlined Int implicit (also see #16372).

inline def f: Int = {
  given Int = 10
  compiletime.summonInline[Int]
}
def test = f

In theory, to be consistent, we would need to keep local imports.

But if the import is local, then we should be able to use the normal summon instead of inline summoning. Not sure if there are corner cases where we can not.

[odersky]

Looks like there are lots of reasons why this won't work.

[som-snytt]

Footnote: I was recently looking at this PR because of -Wunused:imports. There are the usual issues about whether to lint before or after expansion; a related ticket is #22546. I think inlining imports is useful and correct, but the discussion remains interesting.