Make SIP 54 (Multi-Source Extension Overloads) a standard feature (#1…

…7441) - Drop experimental language import - Change note in docs to say that this relaxation only applies to extension method calls, not extension methods called as normal methods. I tried to also reflect the second point in error messages but it turned out too hard. At the point where we generate the error message we do not know how the method was called and it would be unsystematic to create that side channel. In fact, information flows the other way: When we resolve an extension method name, we buffer the error messages and fix selected AmbiguityErrors.
scala · Oct 3, 2023 · 9464bf0 · 9464bf0
2 parents c14d2de + 81b4e5c
commit 9464bf0
Show file tree

Hide file tree

Showing 12 changed files with 58 additions and 62 deletions.
diff --git a/compiler/src/dotty/tools/dotc/config/Feature.scala b/compiler/src/dotty/tools/dotc/config/Feature.scala
@@ -29,7 +29,6 @@ object Feature:
   val fewerBraces = experimental("fewerBraces")
   val saferExceptions = experimental("saferExceptions")
   val clauseInterleaving = experimental("clauseInterleaving")
-  val relaxedExtensionImports = experimental("relaxedExtensionImports")
   val pureFunctions = experimental("pureFunctions")
   val captureChecking = experimental("captureChecking")
   val into = experimental("into")

diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala
@@ -1395,7 +1395,8 @@ class ConstrProxyShadows(proxy: TermRef, shadowed: Type, shadowedIsApply: Boolea
        |or use a full prefix for ${shadowed.termSymbol.name} if you mean the latter."""
 end ConstrProxyShadows
 
-class AmbiguousReference(name: Name, newPrec: BindingPrec, prevPrec: BindingPrec, prevCtx: Context)(using Context)
+class AmbiguousReference(
+    name: Name, newPrec: BindingPrec, prevPrec: BindingPrec, prevCtx: Context, isExtension: => Boolean = false)(using Context)
   extends ReferenceMsg(AmbiguousReferenceID), NoDisambiguation {
 
   /** A string which explains how something was bound; Depending on `prec` this is either
@@ -1417,10 +1418,17 @@ class AmbiguousReference(name: Name, newPrec: BindingPrec, prevPrec: BindingPrec
       i"""$howVisible$qualifier in ${whereFound.owner}"""
   }
 
+  def importHint =
+    if (newPrec == BindingPrec.NamedImport || newPrec == BindingPrec.WildImport)
+        && prevPrec == newPrec
+        && isExtension
+    then i"\n\n Hint: This error may arise if extension method `$name` is called as a normal method."
+    else ""
+
   def msg(using Context) =
     i"""|Reference to $name is ambiguous.
         |It is both ${bindingString(newPrec, ctx)}
-        |and ${bindingString(prevPrec, prevCtx, " subsequently")}"""
+        |and ${bindingString(prevPrec, prevCtx, " subsequently")}$importHint"""
 
   def explain(using Context) =
     val precedent =

diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -237,7 +237,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
           found
         else
           if !scala2pkg && !previous.isError && !found.isError then
-            fail(AmbiguousReference(name, newPrec, prevPrec, prevCtx))
+            fail(AmbiguousReference(name, newPrec, prevPrec, prevCtx,
+              isExtension = previous.termSymbol.is(ExtensionMethod) && found.termSymbol.is(ExtensionMethod)))
           previous
 
       /** Assemble and check alternatives to an imported reference. This implies:
@@ -264,7 +265,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
           then
             altImports.uncheckedNN += altImp
 
-        if Feature.enabled(Feature.relaxedExtensionImports) && altImports != null && ctx.isImportContext then
+        if altImports != null && ctx.isImportContext then
           val curImport = ctx.importInfo.uncheckedNN
           namedImportRef(curImport) match
             case altImp: TermRef =>

diff --git a/docs/_docs/reference/contextual/extension-methods.md b/docs/_docs/reference/contextual/extension-methods.md
@@ -254,7 +254,8 @@ The following two rewritings are tried in order:
     not a wildcard import, pick the expansion from that import. Otherwise, report
     an ambiguous reference error.
 
-    **Note**: This relaxation is currently enabled only under the `experimental.relaxedExtensionImports` language import.
+    **Note**: This relaxation of the import rules applies only if the method `m` is used as an extension method. If it is used as a normal method in prefix form, the usual import rules apply, which means that importing `m` from
+    multiple places can lead to an ambiguity error.
 
  2. If the first rewriting does not typecheck with expected type `T`,
     and there is an extension method `m` in some eligible object `o`, the selection is rewritten to `o.m[Ts](e)`. An object `o` is _eligible_ if

diff --git a/library/src/scala/runtime/stdLibPatches/language.scala b/library/src/scala/runtime/stdLibPatches/language.scala
@@ -69,14 +69,6 @@ object language:
     @compileTimeOnly("`clauseInterleaving` can only be used at compile time in import statements")
     object clauseInterleaving
 
-    /** Adds support for relaxed imports of extension methods.
-      * Extension methods with the same name can be imported from several places.
-      *
-      * @see [[http://dotty.epfl.ch/docs/reference/contextual/extension-methods]]
-      */
-    @compileTimeOnly("`relaxedExtensionImports` can only be used at compile time in import statements")
-    object relaxedExtensionImports
-
     /** Experimental support for pure function type syntax
      *
      *  @see [[https://dotty.epfl.ch/docs/reference/experimental/purefuns]]

diff --git a/tests/neg/i13558.scala b/tests/neg/i13558.scala
diff --git a/tests/neg/i16920.check b/tests/neg/i16920.check
@@ -1,5 +1,5 @@
--- [E008] Not Found Error: tests/neg/i16920.scala:20:11 ----------------------------------------------------------------
-20 |    "five".wow // error
+-- [E008] Not Found Error: tests/neg/i16920.scala:19:11 ----------------------------------------------------------------
+19 |    "five".wow // error
    |    ^^^^^^^^^^
    |    value wow is not a member of String.
    |    An extension method was tried, but could not be fully constructed:
@@ -10,8 +10,8 @@
    |
    |            Found:    ("five" : String)
    |            Required: Int
--- [E008] Not Found Error: tests/neg/i16920.scala:28:6 -----------------------------------------------------------------
-28 |    5.wow // error
+-- [E008] Not Found Error: tests/neg/i16920.scala:27:6 -----------------------------------------------------------------
+27 |    5.wow // error
    |    ^^^^^
    |    value wow is not a member of Int.
    |    An extension method was tried, but could not be fully constructed:
@@ -22,8 +22,8 @@
    |
    |            Found:    (5 : Int)
    |            Required: Boolean
--- [E008] Not Found Error: tests/neg/i16920.scala:29:11 ----------------------------------------------------------------
-29 |    "five".wow // error
+-- [E008] Not Found Error: tests/neg/i16920.scala:28:11 ----------------------------------------------------------------
+28 |    "five".wow // error
    |    ^^^^^^^^^^
    |    value wow is not a member of String.
    |    An extension method was tried, but could not be fully constructed:
@@ -34,8 +34,8 @@
    |
    |            Found:    ("five" : String)
    |            Required: Boolean
--- [E008] Not Found Error: tests/neg/i16920.scala:36:6 -----------------------------------------------------------------
-36 |    5.wow // error
+-- [E008] Not Found Error: tests/neg/i16920.scala:35:6 -----------------------------------------------------------------
+35 |    5.wow // error
    |    ^^^^^
    |    value wow is not a member of Int.
    |    An extension method was tried, but could not be fully constructed:
@@ -48,8 +48,8 @@
    |            both Three.wow(5)
    |            and  Two.wow(5)
    |            are possible expansions of 5.wow
--- [E008] Not Found Error: tests/neg/i16920.scala:44:11 ----------------------------------------------------------------
-44 |    "five".wow // error
+-- [E008] Not Found Error: tests/neg/i16920.scala:43:11 ----------------------------------------------------------------
+43 |    "five".wow // error
    |    ^^^^^^^^^^
    |    value wow is not a member of String.
    |    An extension method was tried, but could not be fully constructed:
@@ -60,8 +60,8 @@
    |
    |            Found:    ("five" : String)
    |            Required: Int
--- [E008] Not Found Error: tests/neg/i16920.scala:51:11 ----------------------------------------------------------------
-51 |    "five".wow // error
+-- [E008] Not Found Error: tests/neg/i16920.scala:50:11 ----------------------------------------------------------------
+50 |    "five".wow // error
    |    ^^^^^^^^^^
    |    value wow is not a member of String.
    |    An extension method was tried, but could not be fully constructed:
@@ -72,8 +72,8 @@
    |
    |            Found:    ("five" : String)
    |            Required: Int
--- [E008] Not Found Error: tests/neg/i16920.scala:58:6 -----------------------------------------------------------------
-58 |    5.wow // error
+-- [E008] Not Found Error: tests/neg/i16920.scala:57:6 -----------------------------------------------------------------
+57 |    5.wow // error
    |    ^^^^^
    |    value wow is not a member of Int.
    |    An extension method was tried, but could not be fully constructed:

diff --git a/tests/neg/i16920.scala b/tests/neg/i16920.scala
@@ -1,4 +1,3 @@
-import language.experimental.relaxedExtensionImports
 
 object One:
   extension (s: String)

diff --git a/tests/neg/sip54.check b/tests/neg/sip54.check
@@ -0,0 +1,17 @@
+-- [E049] Reference Error: tests/neg/sip54.scala:12:8 ------------------------------------------------------------------
+12 |val _ = meth(foo)() // error // error
+   |        ^^^^
+   |        Reference to meth is ambiguous.
+   |        It is both imported by import A._
+   |        and imported subsequently by import B._
+   |
+   |         Hint: This error may arise if extension method `meth` is called as a normal method.
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E007] Type Mismatch Error: tests/neg/sip54.scala:12:13 -------------------------------------------------------------
+12 |val _ = meth(foo)() // error // error
+   |             ^^^
+   |             Found:    (foo : Foo)
+   |             Required: Bar
+   |
+   | longer explanation available when compiling with `-explain`
diff --git a/tests/neg/sip54.scala b/tests/neg/sip54.scala
@@ -0,0 +1,12 @@
+class Foo
+class Bar
+object A:
+  extension (foo: Foo) def meth(): Foo = foo
+object B:
+  extension (bar: Bar) def meth(): Bar = bar
+
+import A.*
+import B.*
+
+val foo = new Foo
+val _ = meth(foo)() // error // error
diff --git a/tests/pos/i13558.scala b/tests/pos/i13558.scala
@@ -1,5 +1,4 @@
 package testcode
-import language.experimental.relaxedExtensionImports
 
 class A
 

diff --git a/tests/pos/i16920.scala b/tests/pos/i16920.scala
@@ -1,4 +1,3 @@
-import language.experimental.relaxedExtensionImports
 
 object One:
   extension (s: String)