Skip to content

Commit eb34c3c

Browse files
dwijnandSethTisue
andcommitted
Restore unreachable warnings by converting literals
The fix implemented to deal with primitive types auto-converting was suppressing a series of legitimate unreachable warnings. So we removed that fixed and tried to mirror the auto-conversion of the type, such that the space intersection calculation ends up with the correct results. In doing so we triggered reachability warnings being emitted by sealed trait companion object (or enum) `ordinal` synthetic methods, in some weird test cases. So we added an `@unchecked` annotation to suppress those. Co-authored-by: Seth Tisue <[email protected]>
1 parent 38b983c commit eb34c3c

File tree

6 files changed

+63
-9
lines changed

6 files changed

+63
-9
lines changed

compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
525525
val pat = Typed(untpd.Ident(nme.WILDCARD).withType(patType), TypeTree(patType))
526526
CaseDef(pat, EmptyTree, Literal(Constant(idx)))
527527
}
528-
Match(param, cases)
528+
Match(param.annotated(New(defn.UncheckedAnnot.typeRef, Nil)), cases)
529529
}
530530

531531
/** - If `impl` is the companion of a generic sum, add `deriving.Mirror.Sum` parent

compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -503,8 +503,21 @@ class SpaceEngine(using Context) extends SpaceLogic {
503503
}
504504
}
505505

506+
/** Numeric literals, while being constant values of unrelated types (e.g. Char and Int),
507+
* when used in a case may end up matching at runtime, because their equals may returns true.
508+
* Because these are universally available, general purpose types, it would be good to avoid
509+
* returning false positive warnings, such as in `(c: Char) match { case 67 => ... }` emitting a
510+
* reachability warning on the case. So the type `ConstantType(Constant(67, IntTag))` is
511+
* converted to `ConstantType(Constant(67, CharTag))`. #12805 */
512+
def convertConstantType(tp: Type, pt: Type): Type = tp match
513+
case tp @ ConstantType(const) =>
514+
val converted = const.convertTo(pt)
515+
if converted == null then tp else ConstantType(converted)
516+
case _ => tp
517+
506518
/** Is `tp1` a subtype of `tp2`? */
507-
def isSubType(tp1: Type, tp2: Type): Boolean = {
519+
def isSubType(_tp1: Type, tp2: Type): Boolean = {
520+
val tp1 = convertConstantType(_tp1, tp2)
508521
debug.println(TypeComparer.explained(_.isSubType(tp1, tp2)))
509522
val res = if (ctx.explicitNulls) {
510523
tp1 <:< tp2
@@ -750,6 +763,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
750763
}
751764

752765
def show(ss: Seq[Space]): String = ss.map(show).mkString(", ")
766+
753767
/** Display spaces */
754768
def show(s: Space): String = {
755769
def params(tp: Type): List[Type] = tp.classSymbol.primaryConstructor.info.firstParamTypes
@@ -894,13 +908,15 @@ class SpaceEngine(using Context) extends SpaceLogic {
894908
else
895909
project(OrType(selTyp, constantNullType, soft = false))
896910

911+
debug.println(s"targetSpace: ${show(targetSpace)}")
912+
897913
// in redundancy check, take guard as false in order to soundly approximate
898914
val spaces = cases.map { x =>
899915
val res =
900916
if (x.guard.isEmpty) project(x.pat)
901917
else Empty
902918

903-
debug.println(s"${x.pat.show} ====> ${res}")
919+
debug.println(s"${x.pat.show} ====> ${show(res)}")
904920
res
905921
}
906922

@@ -914,12 +930,8 @@ class SpaceEngine(using Context) extends SpaceLogic {
914930
debug.println(s"---------------reachable? ${show(curr)}")
915931
debug.println(s"prev: ${show(prevs)}")
916932

917-
var covered = simplify(intersect(curr, targetSpace))
918-
debug.println(s"covered: $covered")
919-
920-
// `covered == Empty` may happen for primitive types with auto-conversion
921-
// see tests/patmat/reader.scala tests/patmat/byte.scala
922-
if (covered == Empty && !isNullLit(pat)) covered = curr
933+
val covered = simplify(intersect(curr, targetSpace))
934+
debug.println(s"covered: ${show(covered)}")
923935

924936
if (isSubspace(covered, prevs)) {
925937
if i == cases.length - 1

tests/patmat/i12805.check

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
10: Match case Unreachable
2+
16: Match case Unreachable
3+
22: Match case Unreachable

tests/patmat/i12805.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import scala.language.implicitConversions
2+
3+
type Timeframe = "1m" | "2m" | "1H"
4+
type TimeframeN = 1 | 2 | 60
5+
6+
def manualConvertToN(tf: Timeframe): TimeframeN = tf match
7+
case "1m" => 1
8+
case "2m" => 2
9+
case "1H" => 60
10+
case "4H" => ??? // was: no reachability warning
11+
12+
given Conversion[Timeframe, TimeframeN] =
13+
case "1m" => 1
14+
case "2m" => 2
15+
case "1H" => 60
16+
case "4H" => ??? // was: no reachability warning
17+
18+
given Conversion[TimeframeN, Timeframe] =
19+
case 1 => "1m"
20+
case 2 => "2m"
21+
case 60 => "1H"
22+
case 240 => ??? // was: no reachability warning

tests/patmat/i12805b.check

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
4: Match case Unreachable
2+
9: Match case Unreachable
3+
14: Match case Unreachable

tests/patmat/i12805b.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
def test1(a: 1 | 2) = a match
2+
case 1 => true
3+
case 2 => false
4+
case 4 => ??? // unreachable case, was: no warning
5+
6+
def test2(a: 1 | 2) = a match
7+
case 1 => true
8+
case 2 => false
9+
case _ => ??? // unreachable
10+
11+
def test3(a: 1 | 2) = a match
12+
case 1 => true
13+
case 2 => false
14+
case a if a < 0 => ??? // unreachable

0 commit comments

Comments
 (0)