Support extension methods imported from different objects

[odersky]

Add a special case to name resolution so that when expanding an extension method from e.m to m(e) and m is imported by several imports on the same level, we try to typecheck under every such import and pick the successful alternative if it exists and is unambiguous.

Fixes #16920

[odersky]

test performance please

[odersky]

test performance please

[odersky]

@mbovel it seems the benchmark server is down?

[mbovel]

test performance please

[mbovel]

test performance please

[dottybot]

performance test scheduled: 1 job(s) in queue, 0 running.

[mbovel]

it seems the benchmark server is down?

Fixed.

[dottybot]

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/17050/ to see the changes.

Benchmarks is based on merging with main (424da9f)

[odersky]

test performance please

[dottybot]

performance test scheduled: 1 job(s) in queue, 0 running.

[dottybot]

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/17050/ to see the changes.

Benchmarks is based on merging with main (424da9f)

[Ichoran]

This looks great! I just wonder if it's necessary to add a test where an import is re-imported at a deeper level to make sure the search for alternatives is maintained. Something like

object ReimportSucceeds:
  import One._
  def test: Unit =
    import Two._
    import One._
    "five".wow
    5.wow

The reason this is important is that people may want only some extensions in a broader context, but when they switch to a narrower context with more extensions, they may still wish to have the broader extensions available.

For instance, one might want + to add Durations everywhere in a file, but to import a file-handling library that uses + on Path only in one method. But one might want to do temporal math in that same method.

A bulk reimport is probably a reasonable balance between actual accidental collisions and requiring a bunch of extra boilerplate in order to get the desired functionality. Does it already work this way by default? Because I don't know how duplicate but more deeply-nested import statements are handled, I don't know whether this is actually a separate case that needs to be tested.

[odersky]

@Ichoran Nested imports take precedence over outer ones, so the example you show should not be a problem.

[bishabosha]

doesnt this require a SIP? or does this come under "inference", I notice this changes the reference "Translation of Calls to Extension Methods", so it is touching spec

[bishabosha]

this should be under experimental import until SIP approval

[sjrd]

Yes, indeed. As much as I think this is a clear win, there is a reason we have a process: to allow eyes with other kinds of expertise than compiler engineering to see it first, to catch things we might miss.

[odersky]

I agree that this could be a SIP, but I won't have the time to submit and defend it. Would one of you want to take that over?

[sjrd]

I can write the SIP, yes.

[odersky]

For now this requires a language import

import language.experimental.relaxedExtensionImports

We will drop that requirement once the SIP committee has approved the change.

[Ichoran]

@sjrd - I'm glad you're able to write the SIP! If it would help to have more motivating examples, I have plenty!

[bishabosha]

One of the other problems from the Contributor's thread is that this example still does not work:

import language.experimental.relaxedExtensionImports
import scala.annotation.targetName

trait Foo[+T]

object Ops:
  extension (foo : Foo[Int])
    @targetName("bazInt")
    def baz : Unit = {}

import Ops.*

extension (foo : Foo[Double])
  @targetName("bazDouble")
  def baz : Unit = {}

@main def Test =
  val f = new Foo[Int] {}
  f.baz //error

In this case the bazDouble is always chosen because it is not imported, but again surely from the user's point of view it makes sense to also try the valid Ops.bazInt that was imported. To compare with the working Implicit Classes version:

import scala.annotation.targetName

trait Foo[+T]

object Ops:
  implicit class FooOpsInt(foo : Foo[Int]):
    @targetName("bazInt")
    def baz : Unit = {}

import Ops.*

implicit class FooOpsDouble(foo : Foo[Double]):
  @targetName("bazDouble")
  def baz : Unit = {}

@main def Test =
  val f = new Foo[Int] {}
  f.baz //ok

[odersky]

In this case the bazDouble is always chosen because it is not imported, but again surely from the user's point of view it makes sense to also try the valid Ops.bazInt that was imported.

Yes, but that's just wishful thinking. And catering to that would simply produce a huge ball of mud. The current change is already highly problematic for it non-orthogonality. But the argument was that this is an important use case and there was no way to re-arrange things to make it work without the change. By comparison, the latest example is a fringe use case, not worth complicating the rules even further.

it won't be worked on

[bishabosha]

this is a great improvement over the status quo - and the local scope version always being picked is something that can be overcome by application developers (move the extension to a separate object)

[sjrd]

SIP: scala/improvement-proposals#60

[odersky]

Since SIP 54 just got accepted, I'll remove the experimental language import.

[sjrd]

It's been accepted to be shipped as experimental, at this stage. ;)

[odersky]

Ah indeed, it was labelled stage:design, not stage:implementation.