From 1a235c6719f56e1597241dc38eeda49087b323e8 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Wed, 10 Apr 2024 14:13:27 +0200 Subject: [PATCH 1/3] Implement match type amendment: extractors follow aliases and singletons This implements the change proposed in https://github.com/scala/improvement-proposals/pull/84. The added pos test case presents motivating examples, the added neg test cases demonstrate that errors are correctly reported when cycles are present. The potential for cycle is no worse than with the existing extraction logic as demonstrated by the existing test in `tests/neg/mt-deskolemize.scala`. --- .../dotty/tools/dotc/core/TypeComparer.scala | 65 +++++++++++++++++-- tests/neg/mt-deskolemize.scala | 42 ++++++++++++ tests/pos/mt-deskolemize.scala | 55 ++++++++++++++++ 3 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 tests/pos/mt-deskolemize.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index a849d28c81d6..cc7eaecfd9bd 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -3518,20 +3518,75 @@ class MatchReducer(initctx: Context) extends TypeComparer(initctx) { false case MatchTypeCasePattern.TypeMemberExtractor(typeMemberName, capture) => + /** Try to remove references to `skolem` from a type in accordance with the spec. + * + * If any reference to `skolem` remains in the result type, + * `refersToSkolem` is set to true. + */ + class DropSkolemMap(skolem: SkolemType) extends TypeMap: + var refersToSkolem = false + def apply(tp: Type): Type = + tp match + case `skolem` => + refersToSkolem = true + tp + case tp: NamedType => + var savedRefersToSkolem = refersToSkolem + refersToSkolem = false + try + val pre1 = apply(tp.prefix) + if refersToSkolem then + tp match + case tp: TermRef => tp.info.widenExpr.dealias match + case info: SingletonType => + refersToSkolem = false + apply(info) + case _ => + tp.derivedSelect(pre1) + case tp: TypeRef => tp.info match + case info: AliasingBounds => + refersToSkolem = false + apply(info.alias) + case _ => + tp.derivedSelect(pre1) + else + tp.derivedSelect(pre1) + finally + refersToSkolem |= savedRefersToSkolem + case tp: LazyRef => + // By default, TypeMap maps LazyRefs lazily. We need to + // force it for `refersToSkolem` to be correctly set. + apply(tp.ref) + case _ => + mapOver(tp) + end DropSkolemMap + /** Try to remove references to `skolem` from `u` in accordance with the spec. + * + * If any reference to `skolem` remains in the result type, return + * NoType instead. + */ + def dropSkolem(u: Type, skolem: SkolemType): Type = + val dmap = DropSkolemMap(skolem) + val res = dmap(u) + if dmap.refersToSkolem then NoType else res + val stableScrut: SingletonType = scrut match case scrut: SingletonType => scrut case _ => SkolemType(scrut) + stableScrut.member(typeMemberName) match case denot: SingleDenotation if denot.exists => val info = denot.info match case alias: AliasingBounds => alias.alias // Extract the alias case ClassInfo(prefix, cls, _, _, _) => prefix.select(cls) // Re-select the class from the prefix case info => info // Notably, RealTypeBounds, which will eventually give a MatchResult.NoInstances - val infoRefersToSkolem = stableScrut.isInstanceOf[SkolemType] && stableScrut.occursIn(info) - val info1 = info match - case info: TypeBounds => info // Will already trigger a MatchResult.NoInstances - case _ if infoRefersToSkolem => RealTypeBounds(info, info) // Explicitly trigger a MatchResult.NoInstances - case _ => info // We have a match + val info1 = stableScrut match + case skolem: SkolemType => + dropSkolem(info, skolem).orElse: + info match + case info: TypeBounds => info // Will already trigger a MatchResult.NoInstances + case _ => RealTypeBounds(info, info) // Explicitly trigger a MatchResult.NoInstances + case _ => info rec(capture, info1, variance = 0, scrutIsWidenedAbstract) case _ => false diff --git a/tests/neg/mt-deskolemize.scala b/tests/neg/mt-deskolemize.scala index 0a58d5db7bc4..505e47637ac4 100644 --- a/tests/neg/mt-deskolemize.scala +++ b/tests/neg/mt-deskolemize.scala @@ -14,3 +14,45 @@ class SimpleLoop2 extends Expr: object Test1: val x: ExtractValue[SimpleLoop1] = 1 // error + +trait Description: + type Elem <: Tuple + +class PrimBroken extends Expr: + type Value = Alias + type Alias = Value // error + +class Prim extends Expr: + type Value = BigInt + +class VecExpr[E <: Expr] extends Expr: + type Value = Vector[ExtractValue[E]] + +trait ProdExpr extends Expr: + val description: Description + type Value = Tuple.Map[description.Elem, [X] =>> ExtractValue[X & Expr]] + + +class MyExpr1 extends ProdExpr: + final val description = new Description: + type Elem = (VecExpr[Prim], MyExpr2) + +class MyExpr2 extends ProdExpr: + final val description = new Description: + type Elem = (VecExpr[VecExpr[MyExpr1]], Prim) + +trait Constable[E <: Expr]: + def lit(v: ExtractValue[E]): E +object Constable: + given [E <: Expr]: Constable[E] = ??? + +object Test2: + def fromLiteral[E <: Expr : Constable](v: ExtractValue[E]): E = + summon[Constable[E]].lit(v) + val x0: ExtractValue[Prim] = "" // error + val x1: ExtractValue[PrimBroken] = 1 // error + + val foo: MyExpr2 = new MyExpr2 + val v: foo.Value = (Vector(Vector()), 1) // error: Recursion limit exceeded + val c: MyExpr2 = fromLiteral: + (Vector(Vector()), 1) // error: Recursion limit exceeded diff --git a/tests/pos/mt-deskolemize.scala b/tests/pos/mt-deskolemize.scala new file mode 100644 index 000000000000..34f38289b24d --- /dev/null +++ b/tests/pos/mt-deskolemize.scala @@ -0,0 +1,55 @@ +trait Expr: + type Value + +object Expr: + type Of[V] = Expr { type Value = V } + type ExtractValue[F <: Expr] = F match + case Expr.Of[v] => v +import Expr.ExtractValue + +class Prim extends Expr: + type Value = Alias + type Alias = BigInt + +class VecExpr[E <: Expr] extends Expr: + type Value = Vector[ExtractValue[E]] + +trait Description: + type Elem <: Tuple + +trait ProdExpr extends Expr: + val description: Description + type Value = Tuple.Map[description.Elem, [X] =>> ExtractValue[X & Expr]] + +class MyExpr1 extends ProdExpr: + final val description = new Description: + type Elem = (VecExpr[Prim], Prim) + +class MyExpr2 extends ProdExpr: + final val description = new Description: + type Elem = (VecExpr[VecExpr[MyExpr1]], Prim) + +trait ProdExprAlt[T <: Tuple] extends Expr: + type Value = Tuple.Map[T, [X] =>> ExtractValue[X & Expr]] + +class MyExpr3 extends ProdExprAlt[(Prim, VecExpr[Prim], Prim)] + +trait Constable[E <: Expr]: + def lit(v: ExtractValue[E]): E +object Constable: + given [E <: Expr]: Constable[E] = ??? + +object Test: + def fromLiteral[E <: Expr : Constable](v: ExtractValue[E]): E = + summon[Constable[E]].lit(v) + val a: Prim = fromLiteral(1) + val b: VecExpr[Prim] = fromLiteral(Vector(1)) + val c: MyExpr1 = fromLiteral((Vector(1), 1)) + val d: MyExpr2 = fromLiteral(Vector(Vector((Vector(1), 1))), 2) + val e: MyExpr3 = fromLiteral((1, Vector(1), 1)) + val f: ProdExprAlt[(MyExpr1, VecExpr[MyExpr3])] = fromLiteral: + ( + (Vector(1), 1), + Vector((1, Vector(1), 1), (2, Vector(1), 2)) + ) + val g: Expr { type Alias = Int; type Value = Alias } = fromLiteral(1) From 61b5a7b6a52f32c68a4f3aa8842f6c4850349b87 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sun, 5 May 2024 18:54:30 +0200 Subject: [PATCH 2/3] Move logic under feature.experimental.betterMatchTypesExtractors This way we can merge this PR without waiting for the SIP committee to approve it. --- .../src/dotty/tools/dotc/config/Feature.scala | 3 + .../dotty/tools/dotc/core/TypeComparer.scala | 11 +++- .../runtime/stdLibPatches/language.scala | 7 +++ tests/neg/mt-deskolemize-2.scala | 60 +++++++++++++++++++ tests/neg/mt-deskolemize.scala | 42 ------------- tests/pos/mt-deskolemize.scala | 2 + 6 files changed, 80 insertions(+), 45 deletions(-) create mode 100644 tests/neg/mt-deskolemize-2.scala diff --git a/compiler/src/dotty/tools/dotc/config/Feature.scala b/compiler/src/dotty/tools/dotc/config/Feature.scala index d2bfdcb550dc..0d551094da4d 100644 --- a/compiler/src/dotty/tools/dotc/config/Feature.scala +++ b/compiler/src/dotty/tools/dotc/config/Feature.scala @@ -35,6 +35,7 @@ object Feature: val into = experimental("into") val namedTuples = experimental("namedTuples") val modularity = experimental("modularity") + val betterMatchTypeExtractors = experimental("betterMatchTypeExtractors") def experimentalAutoEnableFeatures(using Context): List[TermName] = defn.languageExperimentalFeatures @@ -89,6 +90,8 @@ object Feature: def scala2ExperimentalMacroEnabled(using Context) = enabled(scala2macros) + def betterMatchTypeExtractorsEnabled(using Context) = enabled(betterMatchTypeExtractors) + /** Is pureFunctions enabled for this compilation unit? */ def pureFunsEnabled(using Context) = enabledBySetting(pureFunctions) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index cc7eaecfd9bd..2481f17ffdad 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -10,7 +10,7 @@ import TypeOps.refineUsingParent import collection.mutable import util.{Stats, NoSourcePosition, EqHashMap} import config.Config -import config.Feature.{migrateTo3, sourceVersion} +import config.Feature.{betterMatchTypeExtractorsEnabled, migrateTo3, sourceVersion} import config.Printers.{subtyping, gadts, matchTypes, noPrinter} import config.SourceVersion import TypeErasure.{erasedLub, erasedGlb} @@ -3519,6 +3519,11 @@ class MatchReducer(initctx: Context) extends TypeComparer(initctx) { case MatchTypeCasePattern.TypeMemberExtractor(typeMemberName, capture) => /** Try to remove references to `skolem` from a type in accordance with the spec. + * + * If `betterMatchTypeExtractorsEnabled` is enabled then references + * to `skolem` occuring are avoided by following aliases and + * singletons, otherwise no attempt made to avoid references to + * `skolem`. * * If any reference to `skolem` remains in the result type, * `refersToSkolem` is set to true. @@ -3530,7 +3535,7 @@ class MatchReducer(initctx: Context) extends TypeComparer(initctx) { case `skolem` => refersToSkolem = true tp - case tp: NamedType => + case tp: NamedType if betterMatchTypeExtractorsEnabled => var savedRefersToSkolem = refersToSkolem refersToSkolem = false try @@ -3553,7 +3558,7 @@ class MatchReducer(initctx: Context) extends TypeComparer(initctx) { tp.derivedSelect(pre1) finally refersToSkolem |= savedRefersToSkolem - case tp: LazyRef => + case tp: LazyRef if betterMatchTypeExtractorsEnabled => // By default, TypeMap maps LazyRefs lazily. We need to // force it for `refersToSkolem` to be correctly set. apply(tp.ref) diff --git a/library/src/scala/runtime/stdLibPatches/language.scala b/library/src/scala/runtime/stdLibPatches/language.scala index 02c4a99bbbcf..1171c62602fb 100644 --- a/library/src/scala/runtime/stdLibPatches/language.scala +++ b/library/src/scala/runtime/stdLibPatches/language.scala @@ -117,6 +117,13 @@ object language: @compileTimeOnly("`relaxedExtensionImports` can only be used at compile time in import statements") @deprecated("The experimental.relaxedExtensionImports language import is no longer needed since the feature is now standard", since = "3.4") object relaxedExtensionImports + + /** Enhance match type extractors to follow aliases and singletons. + * + * @see [[https://github.com/scala/improvement-proposals/pull/84]] + */ + @compileTimeOnly("`betterMatchTypeExtractors` can only be used at compile time in import statements") + object betterMatchTypeExtractors end experimental /** The deprecated object contains features that are no longer officially suypported in Scala. diff --git a/tests/neg/mt-deskolemize-2.scala b/tests/neg/mt-deskolemize-2.scala new file mode 100644 index 000000000000..90d506a42e6f --- /dev/null +++ b/tests/neg/mt-deskolemize-2.scala @@ -0,0 +1,60 @@ +//> using options -language:experimental.betterMatchTypeExtractors + +trait Expr: + type Value +object Expr: + type Of[V] = Expr { type Value = V } + type ExtractValue[F <: Expr] = F match + case Expr.Of[v] => v +import Expr.ExtractValue + +class SimpleLoop1 extends Expr: + type Value = ExtractValue[SimpleLoop2] + +class SimpleLoop2 extends Expr: + type Value = ExtractValue[SimpleLoop1] + +object Test1: + val x: ExtractValue[SimpleLoop1] = 1 // error + +trait Description: + type Elem <: Tuple + +class PrimBroken extends Expr: + type Value = Alias + type Alias = Value // error + +class Prim extends Expr: + type Value = BigInt + +class VecExpr[E <: Expr] extends Expr: + type Value = Vector[ExtractValue[E]] + +trait ProdExpr extends Expr: + val description: Description + type Value = Tuple.Map[description.Elem, [X] =>> ExtractValue[X & Expr]] + + +class MyExpr1 extends ProdExpr: + final val description = new Description: + type Elem = (VecExpr[Prim], MyExpr2) + +class MyExpr2 extends ProdExpr: + final val description = new Description: + type Elem = (VecExpr[VecExpr[MyExpr1]], Prim) + +trait Constable[E <: Expr]: + def lit(v: ExtractValue[E]): E +object Constable: + given [E <: Expr]: Constable[E] = ??? + +object Test2: + def fromLiteral[E <: Expr : Constable](v: ExtractValue[E]): E = + summon[Constable[E]].lit(v) + val x0: ExtractValue[Prim] = "" // error + val x1: ExtractValue[PrimBroken] = 1 // error + + val foo: MyExpr2 = new MyExpr2 + val v: foo.Value = (Vector(Vector()), 1) // error: Recursion limit exceeded + val c: MyExpr2 = fromLiteral: + (Vector(Vector()), 1) // error: Recursion limit exceeded diff --git a/tests/neg/mt-deskolemize.scala b/tests/neg/mt-deskolemize.scala index 505e47637ac4..0a58d5db7bc4 100644 --- a/tests/neg/mt-deskolemize.scala +++ b/tests/neg/mt-deskolemize.scala @@ -14,45 +14,3 @@ class SimpleLoop2 extends Expr: object Test1: val x: ExtractValue[SimpleLoop1] = 1 // error - -trait Description: - type Elem <: Tuple - -class PrimBroken extends Expr: - type Value = Alias - type Alias = Value // error - -class Prim extends Expr: - type Value = BigInt - -class VecExpr[E <: Expr] extends Expr: - type Value = Vector[ExtractValue[E]] - -trait ProdExpr extends Expr: - val description: Description - type Value = Tuple.Map[description.Elem, [X] =>> ExtractValue[X & Expr]] - - -class MyExpr1 extends ProdExpr: - final val description = new Description: - type Elem = (VecExpr[Prim], MyExpr2) - -class MyExpr2 extends ProdExpr: - final val description = new Description: - type Elem = (VecExpr[VecExpr[MyExpr1]], Prim) - -trait Constable[E <: Expr]: - def lit(v: ExtractValue[E]): E -object Constable: - given [E <: Expr]: Constable[E] = ??? - -object Test2: - def fromLiteral[E <: Expr : Constable](v: ExtractValue[E]): E = - summon[Constable[E]].lit(v) - val x0: ExtractValue[Prim] = "" // error - val x1: ExtractValue[PrimBroken] = 1 // error - - val foo: MyExpr2 = new MyExpr2 - val v: foo.Value = (Vector(Vector()), 1) // error: Recursion limit exceeded - val c: MyExpr2 = fromLiteral: - (Vector(Vector()), 1) // error: Recursion limit exceeded diff --git a/tests/pos/mt-deskolemize.scala b/tests/pos/mt-deskolemize.scala index 34f38289b24d..abd61d9d55e6 100644 --- a/tests/pos/mt-deskolemize.scala +++ b/tests/pos/mt-deskolemize.scala @@ -1,3 +1,5 @@ +//> using options -language:experimental.betterMatchTypeExtractors + trait Expr: type Value From a1930c4ca38673885a4ebc2ce95689e9e65d08be Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 7 May 2024 12:34:30 +0200 Subject: [PATCH 3/3] DropSkolemMap: simplify logic No need to save the value of `refersToSkolem`: if it's true before we enter `NamedType` it will be true after and `dropSkolem` will return `NoType`. The previous logic could still be useful if we want to give more easily actionable error messages in the future by only keeping in the type the skolems we couldn't remove. --- .../dotty/tools/dotc/core/TypeComparer.scala | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 2481f17ffdad..c2c502a984c4 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -3531,33 +3531,30 @@ class MatchReducer(initctx: Context) extends TypeComparer(initctx) { class DropSkolemMap(skolem: SkolemType) extends TypeMap: var refersToSkolem = false def apply(tp: Type): Type = + if refersToSkolem then + return tp tp match case `skolem` => refersToSkolem = true tp case tp: NamedType if betterMatchTypeExtractorsEnabled => - var savedRefersToSkolem = refersToSkolem - refersToSkolem = false - try - val pre1 = apply(tp.prefix) - if refersToSkolem then - tp match - case tp: TermRef => tp.info.widenExpr.dealias match - case info: SingletonType => - refersToSkolem = false - apply(info) - case _ => - tp.derivedSelect(pre1) - case tp: TypeRef => tp.info match - case info: AliasingBounds => - refersToSkolem = false - apply(info.alias) - case _ => - tp.derivedSelect(pre1) - else - tp.derivedSelect(pre1) - finally - refersToSkolem |= savedRefersToSkolem + val pre1 = apply(tp.prefix) + if refersToSkolem then + tp match + case tp: TermRef => tp.info.widenExpr.dealias match + case info: SingletonType => + refersToSkolem = false + apply(info) + case _ => + tp.derivedSelect(pre1) + case tp: TypeRef => tp.info match + case info: AliasingBounds => + refersToSkolem = false + apply(info.alias) + case _ => + tp.derivedSelect(pre1) + else + tp.derivedSelect(pre1) case tp: LazyRef if betterMatchTypeExtractorsEnabled => // By default, TypeMap maps LazyRefs lazily. We need to // force it for `refersToSkolem` to be correctly set.