From aa132db10df260e9a12090234ea6f96893897f20 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 6 Dec 2023 12:23:52 +0000 Subject: [PATCH 1/3] Handle TupleXXL in match analysis There's a number of problems with the match analysis of TupleXXL. Of course, they manifest as (in)exhaustivity and (un)reachability warnings. Reachability suffered by the problem that a large generic tuple scrutinee type wasn't considered extractable by the TupleXXL extractor that Typer associates the extractor pattern with. That was solved by special handling in SpaceEngine's isSubType. Exhaustivity suffered by a variety of problems, again stemming from the disconnect between the TupleXXL pattern type and the large generic tuple scrutinee (or component) type. That was solved by special handling in exhaustivityCheckable to ignore large generic tuple scrutinees. That then highlighted an irrefutable failure (checkIrrefutable), which also needed to be taught that extra large generic tuples do conform to TupleXXL extractors type, afterwhich SpaceEngine isIrrefutable needed special handling to consider TuplXXL irrefutable. [Cherry-picked e56b75ee5fb10128cbb9f94df3ae82e7315d9ef4] --- .../src/dotty/tools/dotc/core/TypeUtils.scala | 42 +++++++++++++++---- .../tools/dotc/transform/patmat/Space.scala | 9 ++-- .../src/dotty/tools/dotc/typer/Checking.scala | 5 ++- tests/pos/i14588.scala | 20 +++++++++ tests/pos/i16186.scala | 9 ++++ tests/pos/i16657.scala | 13 ++++++ tests/pos/i19084.scala | 14 +++++++ 7 files changed, 97 insertions(+), 15 deletions(-) create mode 100644 tests/pos/i14588.scala create mode 100644 tests/pos/i16186.scala create mode 100644 tests/pos/i16657.scala create mode 100644 tests/pos/i19084.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeUtils.scala b/compiler/src/dotty/tools/dotc/core/TypeUtils.scala index 0f7ab756d362..ec2eb982f205 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeUtils.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeUtils.scala @@ -72,17 +72,41 @@ class TypeUtils { else None recur(self.stripTypeVar, bound) - /** Is this a generic tuple that would fit into the range 1..22, - * but is not already an instance of one of Tuple1..22? - * In this case we need to cast it to make the TupleN/ members accessible. - * This works only for generic tuples of known size up to 22. - */ - def isSmallGenericTuple(using Context): Boolean = + /** Is this a generic tuple but not already an instance of one of Tuple1..22? */ + def isGenericTuple(using Context): Boolean = self.derivesFrom(defn.PairClass) && !defn.isTupleNType(self.widenDealias) - && self.widenTermRefExpr.tupleElementTypesUpTo(Definitions.MaxTupleArity).match - case Some(elems) if elems.length <= Definitions.MaxTupleArity => true - case _ => false + + /** Is this a generic tuple that would fit into the range 1..22? + * In this case we need to cast it to make the TupleN members accessible. + * This works only for generic tuples of known size up to 22. + */ + def isSmallGenericTuple(using Context): Boolean = genericTupleArityCompare < 0 + + /** Is this a generic tuple with an arity above 22? */ + def isLargeGenericTuple(using Context): Boolean = genericTupleArityCompare > 0 + + /** If this is a generic tuple with element types, compare the arity and return: + * * -1, if the generic tuple is small (<= MaxTupleArity) + * * 1, if the generic tuple is large (> MaxTupleArity) + * * 0 if this isn't a generic tuple with element types + */ + def genericTupleArityCompare(using Context): Int = + if self.isGenericTuple then + self.widenTermRefExpr.tupleElementTypesUpTo(Definitions.MaxTupleArity).match + case Some(elems) => if elems.length <= Definitions.MaxTupleArity then -1 else 1 + case _ => 0 + else 0 + + /** Is this a large generic tuple and is `pat` TupleXXL? + * TupleXXL.unapplySeq extracts values of type TupleXXL + * but large scrutinee terms are typed as large generic tuples. + * This allows them to hold on to their precise element types, + * but it means type-wise, the terms don't conform to the + * extractor's parameter type, so this method identifies case. + */ + def isTupleXXLExtract(pat: Type)(using Context): Boolean = + pat.typeSymbol == defn.TupleXXLClass && self.isLargeGenericTuple /** The `*:` equivalent of an instance of a Tuple class */ def toNestedPairs(using Context): Type = diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index fe16a00d7256..335a4c695253 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -286,11 +286,9 @@ object SpaceEngine { || (unapp.symbol.is(Synthetic) && unapp.symbol.owner.linkedClass.is(Case)) // scala2 compatibility || unapplySeqTypeElemTp(unappResult).exists // only for unapplySeq || isProductMatch(unappResult, argLen) - || { - val isEmptyTp = extractorMemberType(unappResult, nme.isEmpty, NoSourcePosition) - isEmptyTp <:< ConstantType(Constant(false)) - } + || extractorMemberType(unappResult, nme.isEmpty, NoSourcePosition) <:< ConstantType(Constant(false)) || unappResult.derivesFrom(defn.NonEmptyTupleClass) + || unapp.symbol == defn.TupleXXL_unapplySeq // Fixes TupleXXL.unapplySeq which returns Some but declares Option } /** Is the unapply or unapplySeq irrefutable? @@ -514,6 +512,7 @@ object SpaceEngine { def isSubType(tp1: Type, tp2: Type)(using Context): Boolean = trace(i"$tp1 <:< $tp2", debug, show = true) { if tp1 == ConstantType(Constant(null)) && !ctx.mode.is(Mode.SafeNulls) then tp2 == ConstantType(Constant(null)) + else if tp1.isTupleXXLExtract(tp2) then true // See isTupleXXLExtract, fixes TupleXXL parameter type else tp1 <:< tp2 } @@ -838,7 +837,7 @@ object SpaceEngine { def isCheckable(tp: Type): Boolean = val tpw = tp.widen.dealias val classSym = tpw.classSymbol - classSym.is(Sealed) || + classSym.is(Sealed) && !tpw.isLargeGenericTuple || // exclude large generic tuples from exhaustivity tpw.isInstanceOf[OrType] || (tpw.isInstanceOf[AndType] && { val and = tpw.asInstanceOf[AndType] diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 7647768a3a34..ef30c4fe7bfb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -911,7 +911,10 @@ trait Checking { false } - def check(pat: Tree, pt: Type): Boolean = (pt <:< pat.tpe) || fail(pat, pt, Reason.NonConforming) + def check(pat: Tree, pt: Type): Boolean = + pt.isTupleXXLExtract(pat.tpe) // See isTupleXXLExtract, fixes TupleXXL parameter type + || pt <:< pat.tpe + || fail(pat, pt, Reason.NonConforming) def recur(pat: Tree, pt: Type): Boolean = !sourceVersion.isAtLeast(`3.2`) diff --git a/tests/pos/i14588.scala b/tests/pos/i14588.scala new file mode 100644 index 000000000000..80483ace34f6 --- /dev/null +++ b/tests/pos/i14588.scala @@ -0,0 +1,20 @@ +//> using options -Werror + +class Test: + def t1: Unit = + (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23) match + case (x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13, x14, x15, x16, x17, x18, x19, x20, x21, x22, x23) => + def t2: Unit = + (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23) match + case (x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13, x14, x15, x16, x17, x18, x19, x20, x21, x22, x23, x24) => + def t3: Unit = + (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24) match + case (x1, x2, x3, x4, x5, x6, x7, x8, x9, x10, x11, x12, x13, x14, x15, x16, x17, x18, x19, x20, x21, x22, x23) => + +object Main: + def main(args: Array[String]): Unit = { + val t = new Test + t.t1 + try { t.t2; ??? } catch case _: MatchError => () + try { t.t3; ??? } catch case _: MatchError => () + } diff --git a/tests/pos/i16186.scala b/tests/pos/i16186.scala new file mode 100644 index 000000000000..9a8948cae529 --- /dev/null +++ b/tests/pos/i16186.scala @@ -0,0 +1,9 @@ +//> using options -Werror + +class Test: + val x = 42 + val tup23 = (x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x, x) + + tup23 match { + case (_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _) => "Tuple Pattern" + } diff --git a/tests/pos/i16657.scala b/tests/pos/i16657.scala new file mode 100644 index 000000000000..5c2c0aa6567d --- /dev/null +++ b/tests/pos/i16657.scala @@ -0,0 +1,13 @@ +//> using options -Werror + +class Test: + val (_, ( + _, _, _, _, _, _, _, _, _, _, // 10 + _, _, _, _, _, _, _, _, _, _, // 20 + _, c22, _ // 23 + )) = // nested pattern has 23 elems + (0, ( + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, + 1, 2, 3, 4, 5, 6, 7, 8, 9, 20, + 1, 2, 3 + )) // ok, exhaustive, reachable, conforming and irrefutable diff --git a/tests/pos/i19084.scala b/tests/pos/i19084.scala new file mode 100644 index 000000000000..ab44f0e48b43 --- /dev/null +++ b/tests/pos/i19084.scala @@ -0,0 +1,14 @@ +//> using options -Werror + +class Test: + def t1(y: ( + Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, + "Bob", Int, 33, Int, + Int, Int, Int, Int, Int, Int, Int, Int, Int, Int) + ): Unit = y match + case b @ (x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, + "Bob", y1, 33, y2, + z0, z1, z2, z3, z4, z5, z6, z7, z8, z9) + => // was: !!! spurious unreachable case warning + () + case _ => () From ad05c50e7be04f824eb2a42112f03d34c3ca2702 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 6 Dec 2023 18:09:49 +0000 Subject: [PATCH 2/3] Add a TupleXXL unreachable test case [Cherry-picked 5bafaaccdcad5a9bd690e8c89715b5b28f04d266] --- tests/warn/i19084.scala | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 tests/warn/i19084.scala diff --git a/tests/warn/i19084.scala b/tests/warn/i19084.scala new file mode 100644 index 000000000000..0ba033d131be --- /dev/null +++ b/tests/warn/i19084.scala @@ -0,0 +1,17 @@ + + +class Test: + def t1(y: ( + Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, + "Bob", Int, 33, Int, + Int, Int, Int, Int, Int, Int, Int, Int, Int, Int) + ): Unit = y match + case b @ (x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, + "Bob", y1, 33, y2, + z0, z1, z2, z3, z4, z5, z6, z7, z8, z9) + => () + case b @ (x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, // warn: unreachable + "Bob", y1, 33, y2, + z0, z1, z2, z3, z4, z5, z6, z7, z8, z9) + => () + case _ => () From 42e8c0c6ad7059cd1905e0ac84776189b20d2dbb Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 7 Dec 2023 12:13:25 +0000 Subject: [PATCH 3/3] Parser simple expression error recovery change from `null` to `???` (#19103) Previously, simpleExpr was recovered as `Literal(Constant(null))` which led to some errors in interactive. Type inference in Scala 3 works on whole chain, thus type vars were inferred as union type of `Null` because of this very reason. Recovering such errors as `unimplementedExpr` which has a type of `Nothing`, solves the issue. --- compiler/src/dotty/tools/dotc/core/Definitions.scala | 1 + compiler/src/dotty/tools/dotc/transform/patmat/Space.scala | 1 + 2 files changed, 2 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 570cec135ac8..1c61df42d8f9 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -947,6 +947,7 @@ class Definitions { def TupleXXLModule(using Context): Symbol = TupleXXLClass.companionModule def TupleXXL_fromIterator(using Context): Symbol = TupleXXLModule.requiredMethod("fromIterator") + def TupleXXL_unapplySeq(using Context): Symbol = TupleXXLModule.requiredMethod(nme.unapplySeq) @tu lazy val RuntimeTupleMirrorTypeRef: TypeRef = requiredClassRef("scala.runtime.TupleMirror") diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 335a4c695253..977fcfb703d8 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -838,6 +838,7 @@ object SpaceEngine { val tpw = tp.widen.dealias val classSym = tpw.classSymbol classSym.is(Sealed) && !tpw.isLargeGenericTuple || // exclude large generic tuples from exhaustivity + // requires an unknown number of changes to make work tpw.isInstanceOf[OrType] || (tpw.isInstanceOf[AndType] && { val and = tpw.asInstanceOf[AndType]