Skip to content
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

Make Scala-3-style implicit resolution explicitly opt-in rather than bundled in -Xsource:3 #10012

Merged
merged 1 commit into from Jul 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/compiler/scala/tools/nsc/Global.scala
Expand Up @@ -1168,6 +1168,7 @@ class Global(var currentSettings: Settings, reporter0: Reporter)

// We hit these checks regularly. They shouldn't change inside the same run, so cache the comparisons here.
val isScala3 = settings.isScala3
val isScala3ImplicitResolution = settings.Yscala3ImplicitResolution
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a BooleanSetting, not a Boolean.


// used in sbt
def uncheckedWarnings: List[(Position, String)] = reporting.uncheckedWarnings
Expand Down
1 change: 1 addition & 0 deletions src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
Expand Up @@ -275,6 +275,7 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
val YpickleWrite = StringSetting("-Ypickle-write", "directory|jar", "destination for generated .sig files containing type signatures.", "", None).internalOnly()
val YpickleWriteApiOnly = BooleanSetting("-Ypickle-write-api-only", "Exclude private members (other than those material to subclass compilation, such as private trait vals) from generated .sig files containing type signatures.").internalOnly()
val YtrackDependencies = BooleanSetting("-Ytrack-dependencies", "Record references to in unit.depends. Deprecated feature that supports SBT 0.13 with incOptions.withNameHashing(false) only.", default = true)
val Yscala3ImplicitResolution = BooleanSetting("-Yscala3-implicit-resolution", "Use Scala-3-style downwards comparisons for implicit search and overloading resolution (see https://github.com/scala/bug/issues/12437).", default = false)

sealed abstract class CachePolicy(val name: String, val help: String)
object CachePolicy {
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/scala/tools/nsc/typechecker/Infer.scala
Expand Up @@ -903,9 +903,9 @@ trait Infer extends Checkable {
isAsSpecificValueType(rtpe1, tpe2, undef1 ::: tparams1, undef2)
case _ =>
tpe2 match {
case PolyType(tparams2, rtpe2) => isAsSpecificValueType(tpe1, rtpe2, undef1, undef2 ::: tparams2)
case _ if !currentRun.isScala3 => existentialAbstraction(undef1, tpe1) <:< existentialAbstraction(undef2, tpe2)
case _ =>
case PolyType(tparams2, rtpe2) => isAsSpecificValueType(tpe1, rtpe2, undef1, undef2 ::: tparams2)
case _ if !currentRun.isScala3ImplicitResolution => existentialAbstraction(undef1, tpe1) <:< existentialAbstraction(undef2, tpe2)
Copy link
Contributor

@som-snytt som-snytt Jul 20, 2022

Choose a reason for hiding this comment

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

This incurs the implicit conversion. I wonder why I don't see a warning currently. Edit: I catch this in that PR.

Copy link
Contributor Author

@povder povder Jul 20, 2022

Choose a reason for hiding this comment

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

I saw your comment and I was going to quickly make a change to avoid the conversion but upon checking out the most recent code I realized the conversion doesn't happen anymore with your change 0d252f6#diff-7b7600384a160cb93e5eceff814f6aed9f5c7b72b752a18c3f61d895482e75fcR907

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for checking my check. (That's what I meant by I caught it in my PR, which was about the same time as yours.) I might change them to vars, except there were a lot of isScala3 calls, so removing the .value is not appealing. But that is why sed is the "scala editor".

case _ =>
// Backport of fix for https://github.com/scala/bug/issues/2509
// from Dotty https://github.com/lampepfl/dotty/commit/89540268e6c49fb92b9ca61249e46bb59981bf5a
//
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t2509-2.scala
@@ -1,4 +1,4 @@
// scalac: -Xsource:3.0
// scalac: -Yscala3-implicit-resolution
class A
class B extends A
class C extends B
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t2509-3.scala
@@ -1,4 +1,4 @@
// scalac: -Xsource:3.0
// scalac: -Yscala3-implicit-resolution
class A
class B extends A

Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t2509-7b.scala
@@ -1,4 +1,4 @@
// scalac: -Xsource:3.0
// scalac: -Yscala3-implicit-resolution
class Both[-A, +B]

trait Factory[A] {
Expand Down
2 changes: 1 addition & 1 deletion test/files/pos/t2509-5.scala
@@ -1,5 +1,5 @@
// See https://github.com/lampepfl/dotty/issues/2974
// scalac: -Xsource:3.0
// scalac: -Yscala3-implicit-resolution

trait Foo[-T]

Expand Down
2 changes: 1 addition & 1 deletion test/files/pos/t2509-6.scala
@@ -1,4 +1,4 @@
// scalac: -Xsource:3.0
// scalac: -Yscala3-implicit-resolution
class A
class B extends A

Expand Down
2 changes: 1 addition & 1 deletion test/files/pos/t2509-7a.scala
@@ -1,4 +1,4 @@
// scalac: -Xsource:3.0
// scalac: -Yscala3-implicit-resolution

class Both[-A, +B]

Expand Down
2 changes: 1 addition & 1 deletion test/files/run/t2509-1.scala
@@ -1,4 +1,4 @@
// scalac: -Xsource:3.0
// scalac: -Yscala3-implicit-resolution
import scala.language.implicitConversions

class A
Expand Down
2 changes: 1 addition & 1 deletion test/files/run/t2509-4.scala
@@ -1,4 +1,4 @@
// scalac: -Xsource:3.0
// scalac: -Yscala3-implicit-resolution
import scala.language.implicitConversions

class A
Expand Down
2 changes: 1 addition & 1 deletion test/files/run/t7768.scala
@@ -1,4 +1,4 @@
// scalac: -Xsource:3.0
// scalac: -Yscala3-implicit-resolution
class A
class B extends A
class C extends B
Expand Down
14 changes: 7 additions & 7 deletions test/junit/scala/tools/nsc/typechecker/InferencerTest.scala
Expand Up @@ -6,7 +6,6 @@ import org.junit.{After, Before, Test}
import org.junit.runner.RunWith
import org.junit.runners.JUnit4

import scala.tools.nsc.settings.ScalaVersion
import scala.tools.testkit.BytecodeTesting

class A
Expand All @@ -26,18 +25,19 @@ trait ZwC[-T] extends Z[T] with C
class InferencerTests extends BytecodeTesting {
import compiler.global._, analyzer._

var storedXsource: ScalaVersion = null
var storedYscala3ImplicitResolution: Boolean = false
@Before
def storeXsource(): Unit = {
storedXsource = settings.source.value
def storeYscala3ImplicitResolution(): Unit = {
storedYscala3ImplicitResolution = settings.Yscala3ImplicitResolution.value
}
@After
def restoreXsource(): Unit = {
settings.source.value = storedXsource
def restoreYscala3ImplicitResolution(): Unit = {
settings.Yscala3ImplicitResolution.value = storedYscala3ImplicitResolution
}

@Test
def isAsSpecificScala2(): Unit = {
settings.Yscala3ImplicitResolution.value = false
val run = new global.Run

enteringPhase(run.typerPhase) {
Expand Down Expand Up @@ -103,7 +103,7 @@ class InferencerTests extends BytecodeTesting {

@Test
def isAsSpecificScala3(): Unit = {
settings.source.value = ScalaVersion("3.0")
settings.Yscala3ImplicitResolution.value = true

val run = new global.Run

Expand Down