Skip to content

Commit

Permalink
A tweak of a tweak about owner scope disambiguation
Browse files Browse the repository at this point in the history
  • Loading branch information
odersky committed Aug 2, 2024
1 parent 3d9ce8e commit 62acaf1
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 14 deletions.
21 changes: 16 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1963,8 +1963,11 @@ trait Applications extends Compatibility {
else if winsPrefix1 then 1
else -1

val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner)
val implicitPair = alt1.symbol.is(Implicit) && alt2.symbol.is(Implicit)
val newGivenRules = preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution)

def compareWithTypes(tp1: Type, tp2: Type) =
val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner)
val winsType1 = isAsGood(alt1, tp1, alt2, tp2)
val winsType2 = isAsGood(alt2, tp2, alt1, tp1)

Expand All @@ -1976,13 +1979,14 @@ trait Applications extends Compatibility {
// (prefer the one that is not a method, but that's arbitrary).
if alt1.widenExpr =:= alt2 then -1 else 1
else
// For implicit resolution, take ownerscore as more significant than type resolution
// For new implicit resolution, take ownerscore as more significant than type resolution
// Reason: People use owner hierarchies to explicitly prioritize, we should not
// break that by changing implicit priority of types.
// But don't do that for implicit/implicit pairs. It's better to leave
// them ambiguous so that the logic in Implicits/compareAlternatives kicks in
// which resolves them with the old rules.
def drawOrOwner =
if preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution)
then ownerScore
else 0
if newGivenRules && !implicitPair then ownerScore else 0
ownerScore match
case 1 => if winsType1 || !winsType2 then 1 else drawOrOwner
case -1 => if winsType2 || !winsType1 then -1 else drawOrOwner
Expand All @@ -2002,6 +2006,13 @@ trait Applications extends Compatibility {

var result = compareWithTypes(strippedType1, strippedType2)
if result != 0 then result
else if ownerScore != 0 && newGivenRules && implicitPair then
0 // for implicit/implicit pairs fail fast
// so that we retry in compareAlternatives with old rules.
// Note: This is problematic since it fails the transitity
// requirement, i.e compareAlternatives is not a partial order
// anymore. But it might be needed to keep the number of failing
// projects smaller
else if strippedType1 eq fullType1 then
if strippedType2 eq fullType2 then 0 // no implicits either side: its' a draw
else 1 // prefer 1st alternative with no implicits
Expand Down
4 changes: 2 additions & 2 deletions tests/neg/scala-uri.check
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
|
| QueryKeyValue.tuple2QueryKeyValue[String, None.type](QueryKey.stringQueryKey,
| QueryValue.optionQueryValue[A](
| /* ambiguous: both value stringQueryValue in trait QueryValueInstances1 and value noneQueryValue in trait QueryValueInstances1 match type QueryValue[A] */
| /* ambiguous: both given instance stringQueryValue in trait QueryValueInstances1 and given instance noneQueryValue in trait QueryValueInstances1 match type QueryValue[A] */
| summon[QueryValue[A]]
| )
| )
|
|But both value stringQueryValue in trait QueryValueInstances1 and value noneQueryValue in trait QueryValueInstances1 match type QueryValue[A].
|But both given instance stringQueryValue in trait QueryValueInstances1 and given instance noneQueryValue in trait QueryValueInstances1 match type QueryValue[A].
10 changes: 5 additions & 5 deletions tests/neg/scala-uri.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import scala.language.implicitConversions
trait QueryKey[A]
object QueryKey extends QueryKeyInstances
sealed trait QueryKeyInstances:
implicit val stringQueryKey: QueryKey[String] = ???
given stringQueryKey: QueryKey[String] = ???

trait QueryValue[-A]
object QueryValue extends QueryValueInstances
sealed trait QueryValueInstances1:
implicit final val stringQueryValue: QueryValue[String] = ???
implicit final val noneQueryValue: QueryValue[None.type] = ???
given stringQueryValue: QueryValue[String] = ???
given noneQueryValue: QueryValue[None.type] = ???
// The noneQueryValue makes no sense at this priority. Since QueryValue
// is contravariant, QueryValue[None.type] is always better than QueryValue[Option[A]]
// no matter whether it's old or new resolution. So taking both owner and type
Expand All @@ -20,11 +20,11 @@ sealed trait QueryValueInstances1:
// same trait as QueryValue[Option[A]], as is shown in pos/scala-uri.scala.

sealed trait QueryValueInstances extends QueryValueInstances1:
implicit final def optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ???
given optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ???

trait QueryKeyValue[A]
object QueryKeyValue:
implicit def tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ???
given tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ???


@main def Test = summon[QueryKeyValue[(String, None.type)]] // error
73 changes: 73 additions & 0 deletions tests/pos/i21320.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import scala.deriving.*
import scala.compiletime.*

trait ConfigMonoid[T]:
def zero: T
def orElse(main: T, defaults: T): T

object ConfigMonoid:
given option[T]: ConfigMonoid[Option[T]] = ???

inline def zeroTuple[C <: Tuple]: Tuple =
inline erasedValue[C] match
case _: EmptyTuple => EmptyTuple
case _: (t *: ts) =>
summonInline[ConfigMonoid[t]].zero *: zeroTuple[ts]

inline def valueTuple[C <: Tuple, T](index: Int, main: T, defaults: T): Tuple =
inline erasedValue[C] match
case _: EmptyTuple => EmptyTuple
case _: (t *: ts) =>
def get(v: T) = v.asInstanceOf[Product].productElement(index).asInstanceOf[t]
summonInline[ConfigMonoid[t]].orElse(get(main), get(defaults)) *: valueTuple[ts, T](
index + 1,
main,
defaults
)

inline given derive[T](using m: Mirror.ProductOf[T]): ConfigMonoid[T] =
new ConfigMonoid[T]:
def zero: T = m.fromProduct(zeroTuple[m.MirroredElemTypes])
def orElse(main: T, defaults: T): T = m.fromProduct(valueTuple[m.MirroredElemTypes, T](0, main, defaults))



final case class PublishOptions(
v1: Option[String] = None,
v2: Option[String] = None,
v3: Option[String] = None,
v4: Option[String] = None,
v5: Option[String] = None,
v6: Option[String] = None,
v7: Option[String] = None,
v8: Option[String] = None,
v9: Option[String] = None,
ci: PublishContextualOptions = PublishContextualOptions(),
)
object PublishOptions:
implicit val monoid: ConfigMonoid[PublishOptions] = ConfigMonoid.derive

final case class PublishContextualOptions(
v1: Option[String] = None,
v2: Option[String] = None,
v3: Option[String] = None,
v4: Option[String] = None,
v5: Option[String] = None,
v6: Option[String] = None,
v7: Option[String] = None,
v8: Option[String] = None,
v9: Option[String] = None,
v10: Option[String] = None,
v11: Option[String] = None,
v12: Option[String] = None,
v13: Option[String] = None,
v14: Option[String] = None,
v15: Option[String] = None,
v16: Option[String] = None,
v17: Option[String] = None,
v18: Option[String] = None,
v19: Option[String] = None,
v20: Option[String] = None
)
object PublishContextualOptions:
implicit val monoid: ConfigMonoid[PublishContextualOptions] = ConfigMonoid.derive
4 changes: 2 additions & 2 deletions tests/pos/scala-uri.scala
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// This works for implicit/implicit pairs but not for givens, see neg version.
import scala.language.implicitConversions

trait QueryKey[A]
Expand All @@ -9,14 +10,13 @@ trait QueryValue[-A]
object QueryValue extends QueryValueInstances
sealed trait QueryValueInstances1:
implicit final val stringQueryValue: QueryValue[String] = ???
implicit final val noneQueryValue: QueryValue[None.type] = ???

sealed trait QueryValueInstances extends QueryValueInstances1:
implicit final def optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ???
implicit final val noneQueryValue: QueryValue[None.type] = ???

trait QueryKeyValue[A]
object QueryKeyValue:
implicit def tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ???


@main def Test = summon[QueryKeyValue[(String, None.type)]]

0 comments on commit 62acaf1

Please sign in to comment.