From 164eea51df8bc593423389e345532ed37a61d445 Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 9 May 2023 11:56:32 +0200 Subject: [PATCH 1/2] Make SIP 54 a standard feature - 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. --- compiler/src/dotty/tools/dotc/config/Feature.scala | 1 - compiler/src/dotty/tools/dotc/typer/Typer.scala | 2 +- docs/_docs/reference/contextual/extension-methods.md | 3 ++- library/src/scala/runtime/stdLibPatches/language.scala | 8 -------- 4 files changed, 3 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Feature.scala b/compiler/src/dotty/tools/dotc/config/Feature.scala index e5ab8f65f55b..419ed5868cbf 100644 --- 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/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 7eb8519739c6..da9b1d1a9d80 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -260,7 +260,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 index d98d80caafc5..8b9a3df5b84c 100644 --- 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 index 091e75fa06e1..d92495c6f5aa 100644 --- 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]] From 81b4e5ce0c82b4e953864fd3e7ac5c28c3dc586b Mon Sep 17 00:00:00 2001 From: odersky Date: Tue, 9 May 2023 15:39:53 +0200 Subject: [PATCH 2/2] Add error message hint when extension imports are used as normal methods --- .../dotty/tools/dotc/reporting/messages.scala | 12 +++++-- .../src/dotty/tools/dotc/typer/Typer.scala | 3 +- tests/neg/i13558.scala | 31 ------------------- tests/neg/i16920.check | 28 ++++++++--------- tests/neg/i16920.scala | 1 - tests/neg/sip54.check | 17 ++++++++++ tests/neg/sip54.scala | 12 +++++++ tests/pos/i13558.scala | 1 - tests/pos/i16920.scala | 1 - 9 files changed, 55 insertions(+), 51 deletions(-) delete mode 100644 tests/neg/i13558.scala create mode 100644 tests/neg/sip54.check create mode 100644 tests/neg/sip54.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 423c1cdef264..138e51ecbc6d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -1336,7 +1336,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 @@ -1358,10 +1359,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 index da9b1d1a9d80..b1b57d70ef53 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -233,7 +233,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: diff --git a/tests/neg/i13558.scala b/tests/neg/i13558.scala deleted file mode 100644 index 1d4e1c506e43..000000000000 --- a/tests/neg/i13558.scala +++ /dev/null @@ -1,31 +0,0 @@ -package testcode - -class A - -class B - -object ExtensionA { - extension (self: A) { - def id = "A" - } -} -object ExtensionB { - extension (self: B) { - def id = "B" - } -} - -object Main { - def main1(args: Array[String]): Unit = { - import ExtensionB._ - import ExtensionA._ - val a = A() - println(a.id) // error - } - def main2(args: Array[String]): Unit = { - import ExtensionA._ - import ExtensionB._ - val a = A() - println(a.id) // error - } -} \ No newline at end of file diff --git a/tests/neg/i16920.check b/tests/neg/i16920.check index 131ba4c6265e..8f8172b5538e 100644 --- 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 index 38345e811c1f..c4a54046e027 100644 --- 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 new file mode 100644 index 000000000000..d53687f8ba79 --- /dev/null +++ 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 new file mode 100644 index 000000000000..3e092af65d32 --- /dev/null +++ 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 index 0c8be379f6a9..6f18b770f467 100644 --- 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 index dd4f5804a4fd..d52e7e453e7e 100644 --- a/tests/pos/i16920.scala +++ b/tests/pos/i16920.scala @@ -1,4 +1,3 @@ -import language.experimental.relaxedExtensionImports object One: extension (s: String)