Skip to content

Commit b632e62

Browse files
committed
Don't evaluate isInstanceOf for value classes, disable bugged tests
The tests `i1059.scala` and `t3480.scala` are failing due to a bug in pattern matcher that evaluates the `x` in `List(x: _*)` incorrectly. Concerned issue: #1276
1 parent 5a3ab17 commit b632e62

File tree

7 files changed

+43
-37
lines changed

7 files changed

+43
-37
lines changed

src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import dotty.tools.dotc.util.Positions._
55
import TreeTransforms.{MiniPhaseTransform, TransformerInfo}
66
import core._
77
import Contexts.Context, Types._, Constants._, Decorators._, Symbols._
8-
import TypeUtils._, TypeErasure._
8+
import TypeUtils._, TypeErasure._, Flags._
99

1010

1111
/** Implements partial evaluation of `sc.isInstanceOf[Sel]` according to:
@@ -25,10 +25,11 @@ import TypeUtils._, TypeErasure._
2525
*
2626
* 1. evalTypeApply will establish the matrix and choose the appropriate
2727
* handling for the case:
28-
* 2. a) handleStaticallyKnown
29-
* b) falseIfUnrelated with `scrutinee <:< selector`
30-
* c) handleFalseUnrelated
31-
* d) leave as is (aka `happens`)
28+
* 2. a) Sel/sc is a value class or scrutinee is `Any`
29+
* b) handleStaticallyKnown
30+
* c) falseIfUnrelated with `scrutinee <:< selector`
31+
* d) handleFalseUnrelated
32+
* e) leave as is (aka `happens`)
3233
* 3. Rewrite according to step taken in `2`
3334
*/
3435
class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer =>
@@ -43,43 +44,43 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer =>
4344
* the correct warnings, or an error if statically known to be false in
4445
* match
4546
*/
46-
def handleStaticallyKnown(tree: Select, scrutinee: Type, selector: Type, inMatch: Boolean, pos: Position): Tree = {
47+
def handleStaticallyKnown(select: Select, scrutinee: Type, selector: Type, inMatch: Boolean, pos: Position): Tree = {
4748
val scrutineeSubSelector = scrutinee <:< selector
4849
if (!scrutineeSubSelector && inMatch) {
4950
ctx.error(
5051
s"this case is unreachable due to `${selector.show}` not being a subclass of `${scrutinee.show}`",
5152
Position(pos.start - 5, pos.end - 5)
5253
)
53-
rewrite(tree, to = false)
54+
rewrite(select, to = false)
5455
} else if (!scrutineeSubSelector && !inMatch) {
5556
ctx.warning(
5657
s"this will always yield false since `${scrutinee.show}` is not a subclass of `${selector.show}` (will be optimized away)",
5758
pos
5859
)
59-
rewrite(tree, to = false)
60+
rewrite(select, to = false)
6061
} else if (scrutineeSubSelector && !inMatch) {
6162
ctx.warning(
6263
s"this will always yield true if the scrutinee is non-null, since `${scrutinee.show}` is a subclass of `${selector.show}` (will be optimized away)",
6364
pos
6465
)
65-
rewrite(tree, to = true)
66-
} else /* if (scrutineeSubSelector && inMatch) */ rewrite(tree, to = true)
66+
rewrite(select, to = true)
67+
} else /* if (scrutineeSubSelector && inMatch) */ rewrite(select, to = true)
6768
}
6869

6970
/** Rewrites cases with unrelated types */
70-
def handleFalseUnrelated(tree: Select, scrutinee: Type, selector: Type, inMatch: Boolean) =
71+
def handleFalseUnrelated(select: Select, scrutinee: Type, selector: Type, inMatch: Boolean) =
7172
if (inMatch) {
7273
ctx.error(
7374
s"will never match since `${selector.show}` is not a subclass of `${scrutinee.show}`",
74-
Position(tree.pos.start - 5, tree.pos.end - 5)
75+
Position(select.pos.start - 5, select.pos.end - 5)
7576
)
76-
rewrite(tree, to = false)
77+
rewrite(select, to = false)
7778
} else {
7879
ctx.warning(
7980
s"will always yield false since `${scrutinee.show}` is not a subclass of `${selector.show}`",
80-
tree.pos
81+
select.pos
8182
)
82-
rewrite(tree, to = false)
83+
rewrite(select, to = false)
8384
}
8485

8586
/** Rewrites the select to a boolean if `to` is false or if the qualifier
@@ -106,25 +107,30 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer =>
106107
val scrutinee = erasure(s.qualifier.tpe.widen)
107108
val selector = erasure(tree.args.head.tpe.widen)
108109

109-
val scTrait = scrutinee.typeSymbol is Flags.Trait
110+
val scTrait = scrutinee.typeSymbol is Trait
110111
val scClass =
111112
scrutinee.typeSymbol.isClass &&
112-
!(scrutinee.typeSymbol is Flags.Trait) &&
113-
!(scrutinee.typeSymbol is Flags.Module)
113+
!(scrutinee.typeSymbol is Trait) &&
114+
!(scrutinee.typeSymbol is Module)
114115

115-
val scClassNonFinal = scClass && !scrutinee.typeSymbol.is(Flags.Final)
116-
val scFinalClass = scClass && (scrutinee.typeSymbol is Flags.Final)
116+
val scClassNonFinal = scClass && !(scrutinee.typeSymbol is Final)
117+
val scFinalClass = scClass && (scrutinee.typeSymbol is Final)
117118

118-
val selTrait = selector.typeSymbol is Flags.Trait
119+
val selTrait = selector.typeSymbol is Trait
119120
val selClass =
120121
selector.typeSymbol.isClass &&
121-
!(selector.typeSymbol is Flags.Trait) &&
122-
!(selector.typeSymbol is Flags.Module)
122+
!(selector.typeSymbol is Trait) &&
123+
!(selector.typeSymbol is Module)
123124

124-
val selClassNonFinal = scClass && !(selector.typeSymbol is Flags.Final)
125-
val selFinalClass = scClass && (selector.typeSymbol is Flags.Final)
125+
val selClassNonFinal = scClass && !(selector.typeSymbol is Final)
126+
val selFinalClass = scClass && (selector.typeSymbol is Final)
126127

127128
// Cases ---------------------------------
129+
val valueClassesOrAny =
130+
ValueClasses.isDerivedValueClass(scrutinee.typeSymbol) ||
131+
ValueClasses.isDerivedValueClass(selector.typeSymbol) ||
132+
scrutinee == defn.ObjectType
133+
128134
val knownStatically = scFinalClass
129135

130136
val falseIfUnrelated =
@@ -137,13 +143,16 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer =>
137143
(scTrait && selClassNonFinal) ||
138144
(scTrait && selTrait)
139145

140-
val inMatch = s.qualifier.symbol is Flags.Case
146+
val inMatch = s.qualifier.symbol is Case
141147

142-
if (knownStatically)
148+
if (valueClassesOrAny) tree
149+
else if (knownStatically)
143150
handleStaticallyKnown(s, scrutinee, selector, inMatch, tree.pos)
144151
else if (falseIfUnrelated && scrutinee <:< selector)
152+
// scrutinee is a subtype of the selector, safe to rewrite
145153
rewrite(s, to = true)
146154
else if (falseIfUnrelated && !(selector <:< scrutinee))
155+
// selector and scrutinee are unrelated
147156
handleFalseUnrelated(s, scrutinee, selector, inMatch)
148157
else if (happens) tree
149158
else tree
File renamed without changes.
File renamed without changes.

tests/neg/i1255.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
object Test {
2+
def foo(x: Option[Int]) = x match {
3+
case Some(_: Double) => true // error
4+
case None => true
5+
}
6+
}

tests/pos/t1168.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
object Test extends App {
22

3-
trait SpecialException {}
3+
trait SpecialException extends Throwable {}
44

55
try {
66
throw new Exception

tests/run/t6637.check

Lines changed: 0 additions & 1 deletion
This file was deleted.

tests/run/t6637.scala

Lines changed: 0 additions & 8 deletions
This file was deleted.

0 commit comments

Comments
 (0)