Skip to content

Backport "Fix #21841: Check more that an unapplySeq on a NonEmptyTuple is valid." to 3.3 LTS #184

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 18, 2025
Merged

Backport "Fix #21841: Check more that an unapplySeq on a NonEmptyTuple is valid." to 3.3 LTS #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
403eaaa
Fix #21841: Check more that an `unapplySeq` on a `NonEmptyTuple` is v…
sjrd Jan 14, 2025
ad82bfc
Fix #21841: Check more that an `unapplySeq` on a `NonEmptyTuple` is v…
tgodzik Mar 14, 2025
8b20967
bugfix: Add mising utility method
tgodzik Mar 14, 2025
d552034
bugfix: Revert changes to coverage tests
tgodzik Mar 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down
34 changes: 27 additions & 7 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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 =
Expand All @@ -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
}

Expand Down
43 changes: 30 additions & 13 deletions tests/coverage/run/i16940/test.scoverage.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,23 @@ Test
Object
<empty>.Test
<init>
387
390
20
Seq
Ident
false
0
false
Seq

5
i16940/i16940.scala
<empty>
Test
Object
<empty>.Test
<init>
391
421
20
Expand All @@ -103,7 +120,7 @@ false
false
brokenSynchronizedBlock(false)

5
6
i16940/i16940.scala
<empty>
Test
Expand All @@ -120,7 +137,7 @@ false
false
brokenSynchronizedBlock(true)

6
7
i16940/i16940.scala
<empty>
Test
Expand All @@ -137,7 +154,7 @@ false
false
println(test)

7
8
i16940/i16940.scala
<empty>
Test
Expand All @@ -154,7 +171,7 @@ false
false
assert(test == 2)

8
9
i16940/i16940.scala
<empty>
Test
Expand All @@ -171,7 +188,7 @@ true
false
assert(test == 2)

9
10
i16940/i16940.scala
<empty>
Test
Expand All @@ -188,7 +205,7 @@ true
false
assert(test == 2)

10
11
i16940/i16940.scala
<empty>
Test
Expand All @@ -205,7 +222,7 @@ false
false
3.seconds

11
12
i16940/i16940.scala
<empty>
i16940$package
Expand All @@ -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
<empty>
i16940$package
Expand All @@ -239,7 +256,7 @@ false
false
Thread.sleep(500)

13
14
i16940/i16940.scala
<empty>
i16940$package
Expand All @@ -256,7 +273,7 @@ true
false
{\n Thread.sleep(500)\n }

14
15
i16940/i16940.scala
<empty>
i16940$package
Expand All @@ -273,7 +290,7 @@ true
false


15
16
i16940/i16940.scala
<empty>
i16940$package
Expand All @@ -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
<empty>
i16940$package
Expand All @@ -307,7 +324,7 @@ false
false
Thread.sleep(1000)

17
18
i16940/i16940.scala
<empty>
i16940$package
Expand Down
46 changes: 40 additions & 6 deletions tests/coverage/run/i18233-min/test.scoverage.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,23 @@ i18233-min$package
Object
<empty>.i18233-min$package
aList
14
18
2
List
Ident
false
0
false
List

6
i18233-min/i18233-min.scala
<empty>
i18233-min$package
Object
<empty>.i18233-min$package
aList
19
34
2
Expand All @@ -120,7 +137,7 @@ false
false
Array[String]()

6
7
i18233-min/i18233-min.scala
<empty>
i18233-min$package
Expand All @@ -137,7 +154,7 @@ false
false
def aList

7
8
i18233-min/i18233-min.scala
<empty>
i18233-min$package
Expand All @@ -154,7 +171,7 @@ false
false
Array("abc", "def")

8
9
i18233-min/i18233-min.scala
<empty>
i18233-min$package
Expand All @@ -171,7 +188,7 @@ false
false
def arr

9
10
i18233-min/i18233-min.scala
<empty>
i18233-min$package
Expand All @@ -188,7 +205,24 @@ false
false
List(arr*)

10
11
i18233-min/i18233-min.scala
<empty>
i18233-min$package
Object
<empty>.i18233-min$package
anotherList
91
95
8
List
Ident
false
0
false
List

12
i18233-min/i18233-min.scala
<empty>
i18233-min$package
Expand All @@ -205,7 +239,7 @@ false
false
arr

11
13
i18233-min/i18233-min.scala
<empty>
i18233-min$package
Expand Down
27 changes: 22 additions & 5 deletions tests/coverage/run/i18233/test.scoverage.check
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,23 @@ Foo
Object
<empty>.Foo
render
54
58
5
List
Ident
false
0
false
List

2
i18233/i18233.scala
<empty>
Foo
Object
<empty>.Foo
render
59
65
5
Expand All @@ -52,7 +69,7 @@ false
false
values

2
3
i18233/i18233.scala
<empty>
Foo
Expand All @@ -69,7 +86,7 @@ false
false
values.tail

3
4
i18233/i18233.scala
<empty>
Foo
Expand All @@ -86,7 +103,7 @@ false
false
List(values.tail*).mkString

4
5
i18233/i18233.scala
<empty>
Foo
Expand All @@ -103,7 +120,7 @@ false
false
def render

5
6
i18233/i18233.scala
<empty>
Test
Expand All @@ -120,7 +137,7 @@ false
false
println(Foo.render)

6
7
i18233/i18233.scala
<empty>
Test
Expand Down
6 changes: 6 additions & 0 deletions tests/neg/i21841.check
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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`
Loading
Loading