From 403eaaaab05221c2be92eec0a530b8d5a5a816dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Tue, 14 Jan 2025 11:57:26 +0100 Subject: [PATCH 1/4] Fix #21841: Check more that an `unapplySeq` on a `NonEmptyTuple` is valid. --- .../dotty/tools/dotc/typer/Applications.scala | 34 +++++++++++++++---- tests/neg/i21841.check | 6 ++++ tests/neg/i21841.scala | 22 ++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 tests/neg/i21841.check create mode 100644 tests/neg/i21841.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index a109a2f3f621..1e6fbd7bed17 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -67,6 +67,21 @@ object Applications { unapplySeqTypeElemTp(productSelectorTypes(tp, errorPos).last).exists } + /** Does `tp` fit the "product-seq match" conditions for a `NonEmptyTuple` as + * an unapply result type for a pattern with `numArgs` subpatterns? + * This is the case if (1) `tp` derives from `NonEmptyTuple`. + * (2) `tp.tupleElementTypes` exists. + * (3) `tp.tupleElementTypes.last` conforms to Seq match + */ + def isNonEmptyTupleSeqMatch(tp: Type, numArgs: Int, errorPos: SrcPos = NoSourcePosition)(using Context): Boolean = { + tp.derivesFrom(defn.NonEmptyTupleClass) + && tp.tupleElementTypes.exists { elemTypes => + val arity = elemTypes.size + arity > 0 && arity <= numArgs + 1 && + unapplySeqTypeElemTp(elemTypes.last).exists + } + } + /** Does `tp` fit the "get match" conditions as an unapply result type? * This is the case of `tp` has a `get` member as well as a * parameterless `isEmpty` member of result type `Boolean`. @@ -143,12 +158,17 @@ object Applications { } else tp :: Nil - def productSeqSelectors(tp: Type, argsNum: Int, pos: SrcPos)(using Context): List[Type] = { - val selTps = productSelectorTypes(tp, pos) - val arity = selTps.length - val elemTp = unapplySeqTypeElemTp(selTps.last) - (0 until argsNum).map(i => if (i < arity - 1) selTps(i) else elemTp).toList - } + def productSeqSelectors(tp: Type, argsNum: Int, pos: SrcPos)(using Context): List[Type] = + seqSelectors(productSelectorTypes(tp, pos), argsNum) + + def nonEmptyTupleSeqSelectors(tp: Type, argsNum: Int, pos: SrcPos)(using Context): List[Type] = + seqSelectors(tp.tupleElementTypes.get, argsNum) + + private def seqSelectors(selectorTypes: List[Type], argsNum: Int)(using Context): List[Type] = + val arity = selectorTypes.length + val elemTp = unapplySeqTypeElemTp(selectorTypes.last) + (0 until argsNum).map(i => if (i < arity - 1) selectorTypes(i) else elemTp).toList + end seqSelectors def unapplyArgs(unapplyResult: Type, unapplyFn: Tree, args: List[untpd.Tree], pos: SrcPos)(using Context): List[Type] = { def getName(fn: Tree): Name = @@ -169,7 +189,7 @@ object Applications { val elemTp = unapplySeqTypeElemTp(tp) if (elemTp.exists) args.map(Function.const(elemTp)) else if (isProductSeqMatch(tp, args.length, pos)) productSeqSelectors(tp, args.length, pos) - else if tp.derivesFrom(defn.NonEmptyTupleClass) then foldApplyTupleType(tp) + else if isNonEmptyTupleSeqMatch(tp, args.length, pos) then nonEmptyTupleSeqSelectors(tp, args.length, pos) else fallback } diff --git a/tests/neg/i21841.check b/tests/neg/i21841.check new file mode 100644 index 000000000000..e119d650b360 --- /dev/null +++ b/tests/neg/i21841.check @@ -0,0 +1,6 @@ +-- [E108] Declaration Error: tests/neg/i21841.scala:20:13 -------------------------------------------------------------- +20 | case v[T](l, r) => () // error + | ^^^^^^^^^^ + | Option[(Test.Expr[Test.T], Test.Expr[Test.T])] is not a valid result type of an unapplySeq method of an extractor. + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/i21841.scala b/tests/neg/i21841.scala new file mode 100644 index 000000000000..8a280abc2cf8 --- /dev/null +++ b/tests/neg/i21841.scala @@ -0,0 +1,22 @@ +object Test { + + sealed trait T + sealed trait Arrow[A, B] + + type ArgsTo[S1, Target] <: NonEmptyTuple = S1 match { + case Arrow[a, Target] => Tuple1[Expr[a]] + case Arrow[a, b] => Expr[a] *: ArgsTo[b, Target] + } + + sealed trait Expr[S] : + def unapplySeq[Target](e: Expr[Target]): Option[ArgsTo[S, Target]] = ??? + + case class Variable[S](id: String) extends Expr[S] + + val v = Variable[Arrow[T, Arrow[T, T]]]("v") + val e : Expr[T] = ??? + + e match + case v[T](l, r) => () // error + case _ => () +} From ad82bfc714c15b03eba66c3924a41c1f73f02f09 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 14 Mar 2025 19:47:30 +0100 Subject: [PATCH 2/4] Fix #21841: Check more that an `unapplySeq` on a `NonEmptyTuple` is valid. [Cherry-picked d5b5defe8b41bbf249f853dd90dccfac35b36592][modified] From 8b209679e02036f56bea7b41cd972cee1a7e6fa0 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 14 Mar 2025 19:57:07 +0100 Subject: [PATCH 3/4] bugfix: Add mising utility method --- compiler/src/dotty/tools/dotc/core/TypeComparer.scala | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index a8795cd50e54..c69ea177ee96 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -1944,6 +1944,13 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling else op2 end necessaryEither + inline def rollbackConstraintsUnless(inline op: Boolean): Boolean = + val saved = constraint + var result = false + try result = ctx.gadtState.rollbackGadtUnless(op) + finally if !result then constraint = saved + result + /** Decompose into conjunction of types each of which has only a single refinement */ def decomposeRefinements(tp: Type, refines: List[(Name, Type)]): Type = tp match case RefinedType(parent, rname, rinfo) => From d552034b3dd2a4f18d348e61f2ba20809cebdc69 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Mon, 17 Mar 2025 21:09:54 +0100 Subject: [PATCH 4/4] bugfix: Revert changes to coverage tests --- .../coverage/run/i16940/test.scoverage.check | 43 +++++++++++------ .../run/i18233-min/test.scoverage.check | 46 ++++++++++++++++--- .../coverage/run/i18233/test.scoverage.check | 27 +++++++++-- 3 files changed, 92 insertions(+), 24 deletions(-) diff --git a/tests/coverage/run/i16940/test.scoverage.check b/tests/coverage/run/i16940/test.scoverage.check index 357080ba9da8..9354a4e67b29 100644 --- a/tests/coverage/run/i16940/test.scoverage.check +++ b/tests/coverage/run/i16940/test.scoverage.check @@ -93,6 +93,23 @@ Test Object .Test +387 +390 +20 +Seq +Ident +false +0 +false +Seq + +5 +i16940/i16940.scala + +Test +Object +.Test + 391 421 20 @@ -103,7 +120,7 @@ false false brokenSynchronizedBlock(false) -5 +6 i16940/i16940.scala Test @@ -120,7 +137,7 @@ false false brokenSynchronizedBlock(true) -6 +7 i16940/i16940.scala Test @@ -137,7 +154,7 @@ false false println(test) -7 +8 i16940/i16940.scala Test @@ -154,7 +171,7 @@ false false assert(test == 2) -8 +9 i16940/i16940.scala Test @@ -171,7 +188,7 @@ true false assert(test == 2) -9 +10 i16940/i16940.scala Test @@ -188,7 +205,7 @@ true false assert(test == 2) -10 +11 i16940/i16940.scala Test @@ -205,7 +222,7 @@ false false 3.seconds -11 +12 i16940/i16940.scala i16940$package @@ -222,7 +239,7 @@ false false Future {\n if (option) {\n Thread.sleep(500)\n }\n synchronized {\n val tmp = test\n Thread.sleep(1000)\n test = tmp + 1\n }\n} -12 +13 i16940/i16940.scala i16940$package @@ -239,7 +256,7 @@ false false Thread.sleep(500) -13 +14 i16940/i16940.scala i16940$package @@ -256,7 +273,7 @@ true false {\n Thread.sleep(500)\n } -14 +15 i16940/i16940.scala i16940$package @@ -273,7 +290,7 @@ true false -15 +16 i16940/i16940.scala i16940$package @@ -290,7 +307,7 @@ false false synchronized {\n val tmp = test\n Thread.sleep(1000)\n test = tmp + 1\n } -16 +17 i16940/i16940.scala i16940$package @@ -307,7 +324,7 @@ false false Thread.sleep(1000) -17 +18 i16940/i16940.scala i16940$package diff --git a/tests/coverage/run/i18233-min/test.scoverage.check b/tests/coverage/run/i18233-min/test.scoverage.check index 7570ebaaed96..108887701017 100644 --- a/tests/coverage/run/i18233-min/test.scoverage.check +++ b/tests/coverage/run/i18233-min/test.scoverage.check @@ -110,6 +110,23 @@ i18233-min$package Object .i18233-min$package aList +14 +18 +2 +List +Ident +false +0 +false +List + +6 +i18233-min/i18233-min.scala + +i18233-min$package +Object +.i18233-min$package +aList 19 34 2 @@ -120,7 +137,7 @@ false false Array[String]() -6 +7 i18233-min/i18233-min.scala i18233-min$package @@ -137,7 +154,7 @@ false false def aList -7 +8 i18233-min/i18233-min.scala i18233-min$package @@ -154,7 +171,7 @@ false false Array("abc", "def") -8 +9 i18233-min/i18233-min.scala i18233-min$package @@ -171,7 +188,7 @@ false false def arr -9 +10 i18233-min/i18233-min.scala i18233-min$package @@ -188,7 +205,24 @@ false false List(arr*) -10 +11 +i18233-min/i18233-min.scala + +i18233-min$package +Object +.i18233-min$package +anotherList +91 +95 +8 +List +Ident +false +0 +false +List + +12 i18233-min/i18233-min.scala i18233-min$package @@ -205,7 +239,7 @@ false false arr -11 +13 i18233-min/i18233-min.scala i18233-min$package diff --git a/tests/coverage/run/i18233/test.scoverage.check b/tests/coverage/run/i18233/test.scoverage.check index 878ca8a14705..97e16be7839f 100644 --- a/tests/coverage/run/i18233/test.scoverage.check +++ b/tests/coverage/run/i18233/test.scoverage.check @@ -42,6 +42,23 @@ Foo Object .Foo render +54 +58 +5 +List +Ident +false +0 +false +List + +2 +i18233/i18233.scala + +Foo +Object +.Foo +render 59 65 5 @@ -52,7 +69,7 @@ false false values -2 +3 i18233/i18233.scala Foo @@ -69,7 +86,7 @@ false false values.tail -3 +4 i18233/i18233.scala Foo @@ -86,7 +103,7 @@ false false List(values.tail*).mkString -4 +5 i18233/i18233.scala Foo @@ -103,7 +120,7 @@ false false def render -5 +6 i18233/i18233.scala Test @@ -120,7 +137,7 @@ false false println(Foo.render) -6 +7 i18233/i18233.scala Test