Skip to content

Commit

Permalink
Preserve the intersection of disjoint numbers in match analysis
Browse files Browse the repository at this point in the history
The use of provablyDisjoint is a way of simplifying an intersection of
two types into the Empty space, using type comparing logic.  However,
for types like (42L: Long) and (it: Int) (a constant type and a term
ref, of two different number types) the two types are provably disjoint,
but that doesn't mean that a "case `it` =>" won't match a "42L"
scrutinee.  So we extend the logic to keep the intersection as it is
when numeric value types are in play.
  • Loading branch information
dwijnand committed Feb 23, 2022
1 parent 48c65c4 commit 89c107c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
31 changes: 19 additions & 12 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package patmat

import core._
import Types._
import TypeUtils._
import Contexts._
import Flags._
import ast._
Expand Down Expand Up @@ -333,9 +334,11 @@ class SpaceEngine(using Context) extends SpaceLogic {
// Since projections of types don't include null, intersection with null is empty.
Empty
else
val res = TypeComparer.provablyDisjoint(tp1, tp2)
if res then Empty
else Typ(AndType(tp1, tp2), decomposed = true)
val intersection = Typ(AndType(tp1, tp2), decomposed = true)
// unrelated numeric value classes can equal each other, so let's not consider type space interection empty
if tp1.classSymbol.isNumericValueClass && tp2.classSymbol.isNumericValueClass then intersection
else if TypeComparer.provablyDisjoint(tp1, tp2) then Empty
else intersection
}

/** Return the space that represents the pattern `pat` */
Expand Down Expand Up @@ -407,7 +410,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
case tp => Typ(tp, decomposed = true)
}

private def unapplySeqInfo(resTp: Type, pos: SrcPos)(using Context): (Int, Type, Type) = {
private def unapplySeqInfo(resTp: Type, pos: SrcPos): (Int, Type, Type) = {
var resultTp = resTp
var elemTp = unapplySeqTypeElemTp(resultTp)
var arity = productArity(resultTp, pos)
Expand Down Expand Up @@ -501,19 +504,20 @@ class SpaceEngine(using Context) extends SpaceLogic {
}

/** Numeric literals, while being constant values of unrelated types (e.g. Char and Int),
* when used in a case may end up matching at runtime, because their equals may returns true.
* Because these are universally available, general purpose types, it would be good to avoid
* returning false positive warnings, such as in `(c: Char) match { case 67 => ... }` emitting a
* when used in a case may end up matching at runtime as their equals may returns true.
* Because these are universally available, general purpose types, it would be good to avoid,
* for example in `(c: Char) match { case 67 => ... }`, emitting a false positive
* reachability warning on the case. So the type `ConstantType(Constant(67, IntTag))` is
* converted to `ConstantType(Constant(67, CharTag))`. #12805 */
def convertConstantType(tp: Type, pt: Type): Type = tp match
def convertConstantType(tp: Type, pt: Type): Type = trace(i"convertConstantType($tp, $pt)", show = true)(tp match
case tp @ ConstantType(const) =>
val converted = const.convertTo(pt)
if converted == null then tp else ConstantType(converted)
case _ => tp
)

def isPrimToBox(tp: Type, pt: Type) =
tp.classSymbol.isPrimitiveValueClass && (defn.boxedType(tp).classSymbol eq pt.classSymbol)
def isPrimToBox(tp: Type, pt: Type): Boolean =
tp.isPrimitiveValueType && (defn.boxedType(tp).classSymbol eq pt.classSymbol)

/** Adapt types by performing primitive value unboxing or boxing, or numeric constant conversion. #12805
*
Expand All @@ -539,7 +543,10 @@ class SpaceEngine(using Context) extends SpaceLogic {
def isSubType(tp1: Type, tp2: Type): Boolean = trace(i"$tp1 <:< $tp2", debug, show = true) {
if tp1 == constantNullType && !ctx.mode.is(Mode.SafeNulls)
then tp2 == constantNullType
else adaptType(tp1, tp2) <:< tp2
else
val tp1a = adaptType(tp1, tp2)
if tp1a eq tp1 then tp1 <:< tp2
else trace(i"$tp1a <:< $tp2 (adapted)", debug, show = true)(tp1a <:< tp2)
}

def isSameUnapply(tp1: TermRef, tp2: TermRef): Boolean =
Expand Down Expand Up @@ -872,7 +879,7 @@ class SpaceEngine(using Context) extends SpaceLogic {
/** Return the underlying type of non-module, non-constant, non-enum case singleton types.
* Also widen ExprType to its result type, and rewrap any annotation wrappers.
* For example, with `val opt = None`, widen `opt.type` to `None.type`. */
def toUnderlying(tp: Type)(using Context): Type = trace(i"toUnderlying($tp)", show = true)(tp match {
def toUnderlying(tp: Type): Type = trace(i"toUnderlying($tp)", show = true)(tp match {
case _: ConstantType => tp
case tp: TermRef if tp.symbol.is(Module) => tp
case tp: TermRef if tp.symbol.isAllOf(EnumCase) => tp
Expand Down
6 changes: 6 additions & 0 deletions tests/pos/i14407.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// scalac: -Werror
@main def Test =
val it: Int = 42
42L match
case `it` => println("pass")
case _ => println("fail")

0 comments on commit 89c107c

Please sign in to comment.