Skip to content

Preserve the intersection of disjoint numbers in match analysis #14550

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.

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 2 commits into from
Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
47 changes: 12 additions & 35 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,12 @@ 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 = false)
// unrelated numeric value classes can equal each other, so let's not consider type space intersection empty
if tp1.classSymbol.isNumericValueClass && tp2.classSymbol.isNumericValueClass then intersection
else if isPrimToBox(tp1, tp2) || isPrimToBox(tp2, tp1) 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 +411,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 @@ -500,35 +504,8 @@ 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
* 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
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)

/** Adapt types by performing primitive value unboxing or boxing, or numeric constant conversion. #12805
*
* This makes these isSubType cases work like this:
* {{{
* 1 <:< Integer => (<skolem> : Integer) <:< Integer = true
* ONE <:< Int => (<skolem> : Int) <:< Int = true
* Integer <:< (1: Int) => (<skolem> : Int) <:< (1: Int) = false
* }}}
*/
def adaptType(tp1: Type, tp2: Type): Type = trace(i"adaptType($tp1, $tp2)", show = true) {
if isPrimToBox(tp1, tp2) then defn.boxedType(tp1).narrow
else if isPrimToBox(tp2, tp1) then defn.unboxedType(tp1).narrow
else convertConstantType(tp1, tp2)
}
def isPrimToBox(tp: Type, pt: Type): Boolean =
tp.isPrimitiveValueType && (defn.boxedType(tp).classSymbol eq pt.classSymbol)

private val isSubspaceCache = mutable.HashMap.empty[(Space, Space, Context), Boolean]

Expand All @@ -539,7 +516,7 @@ 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 tp1 <:< tp2
}

def isSameUnapply(tp1: TermRef, tp2: TermRef): Boolean =
Expand Down Expand Up @@ -872,7 +849,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
25 changes: 0 additions & 25 deletions compiler/test/dotty/tools/dotc/transform/SpaceEngineTest.scala

This file was deleted.

1 change: 1 addition & 0 deletions tests/patmat/i14407.dupe.check
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
6: Match case Unreachable
7 changes: 7 additions & 0 deletions tests/patmat/i14407.dupe.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// scalac: -Werror
@main def Test =
val it: Int = 42
42L match
case `it` => println("pass")
case `it` => println("dupe") // error
case _ => println("fail")
6 changes: 6 additions & 0 deletions tests/patmat/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")