From 7930293ee1f2d169956d472631487a637b744eb2 Mon Sep 17 00:00:00 2001 From: Felix Mulder Date: Tue, 24 May 2016 00:07:49 +0200 Subject: [PATCH 1/7] Add initial partial evaluation of isInstanceOf calls --- src/dotty/tools/dotc/Compiler.scala | 1 + .../transform/IsInstanceOfEvaluator.scala | 89 +++++++++++++++++++ .../tools/dotc/transform/PatternMatcher.scala | 4 +- 3 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala diff --git a/src/dotty/tools/dotc/Compiler.scala b/src/dotty/tools/dotc/Compiler.scala index b63e0236df00..049906ef0e1a 100644 --- a/src/dotty/tools/dotc/Compiler.scala +++ b/src/dotty/tools/dotc/Compiler.scala @@ -61,6 +61,7 @@ class Compiler { new CrossCastAnd, // Normalize selections involving intersection types. new Splitter), // Expand selections involving union types into conditionals List(new VCInlineMethods, // Inlines calls to value class methods + new IsInstanceOfEvaluator, // Issues warnings when unreachable statements are present in match/if expressions new SeqLiterals, // Express vararg arguments as arrays new InterceptedMethods, // Special handling of `==`, `|=`, `getClass` methods new Getters, // Replace non-private vals and vars with getter defs (fields are added later) diff --git a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala new file mode 100644 index 000000000000..d4483bb18a34 --- /dev/null +++ b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala @@ -0,0 +1,89 @@ +package dotty.tools.dotc +package transform + +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.core.Types._ +import dotty.tools.dotc.core.Decorators._ +import dotty.tools.dotc.core.Flags +import dotty.tools.dotc.core.Symbols._ +import dotty.tools.dotc.util.Positions._ +import dotty.tools.dotc.core.TypeErasure._ +import TreeTransforms.{MiniPhaseTransform, TransformerInfo} + +class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => + + import dotty.tools.dotc.ast.tpd._ + + def phaseName = "reachabilityChecker" + + override def transformTypeApply(tree: TypeApply)(implicit ctx: Context, info: TransformerInfo): Tree = { + val defn = ctx.definitions + + def handleStaticallyKnown(tree: TypeApply, scrutinee: Type, selector: Type, inMatch: Boolean, pos: Position): Tree = + if (!(scrutinee <:< selector) && inMatch) { + ctx.error(s"this case is unreachable due to `${selector.show}` not being a subclass of `${scrutinee.show}`", pos) + tree + } else if (!(scrutinee <:< selector) && !inMatch) { + ctx.warning(s"this will always yield false since `${scrutinee.show}` is not a subclass of `${selector.show}` (will be optimized away)", pos) + rewrite(tree, to = false) + } else if (scrutinee <:< selector && !inMatch) { + ctx.warning(s"this will always yield true since `${scrutinee.show}` is a subclass of `${selector.show}` (will be optimized away)", pos) + rewrite(tree, to = true) + } else /* if (scrutinee <:< selector && inMatch) */ rewrite(tree, to = true) + + def rewrite(tree: TypeApply, to: Boolean): Tree = tree + + def evalTypeApply(tree: TypeApply): Tree = + if (tree.symbol != defn.Any_isInstanceOf) tree + else tree.fun match { + case s: Select => { + val scrutinee = erasure(s.qualifier.tpe.widen) + val selector = erasure(tree.args.head.tpe.widen) + + val scTrait = scrutinee.typeSymbol is Flags.Trait + val scClass = + scrutinee.typeSymbol.isClass && + !(scrutinee.typeSymbol is Flags.Trait) && + !(scrutinee.typeSymbol is Flags.Module) + + val scClassNonFinal = scClass && !scrutinee.typeSymbol.is(Flags.Final) + val scFinalClass = scClass && (scrutinee.typeSymbol is Flags.Final) + + val selTrait = selector.typeSymbol is Flags.Trait + val selClass = + selector.typeSymbol.isClass && + !(selector.typeSymbol is Flags.Trait) && + !(selector.typeSymbol is Flags.Module) + + val selClassNonFinal = scClass && !(selector.typeSymbol is Flags.Final) + val selFinalClass = scClass && (selector.typeSymbol is Flags.Final) + + // Cases --------------------------------- + val knownStatically = scFinalClass + + val falseIfUnrelated = + (scClassNonFinal && selClassNonFinal) || + (scClassNonFinal && selFinalClass) || + (scTrait && selFinalClass) + + // Doesn't need to be calculated (since true if others are false) + //val happens = + // (scClassNonFinal && selClassNonFinal) || + // (scTrait && selClassNonFinal) || + // (scTrait && selTrait) + + val inMatch = s.qualifier.symbol is Flags.Case + + if (knownStatically) + handleStaticallyKnown(tree, scrutinee, selector, inMatch, tree.pos) + else if (falseIfUnrelated && !(selector <:< scrutinee)) + rewrite(tree, to = false) + else /*if (happens)*/ tree + } + + case _ => tree + } + + evalTypeApply(tree) + } +} diff --git a/src/dotty/tools/dotc/transform/PatternMatcher.scala b/src/dotty/tools/dotc/transform/PatternMatcher.scala index 740fd546053e..64b85a51fb3d 100644 --- a/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -56,7 +56,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans override def transformTry(tree: tpd.Try)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { val selector = - ctx.newSymbol(ctx.owner, ctx.freshName("ex").toTermName, Flags.Synthetic, defn.ThrowableType, coord = tree.pos) + ctx.newSymbol(ctx.owner, ctx.freshName("ex").toTermName, Flags.Synthetic | Flags.Case, defn.ThrowableType, coord = tree.pos) val sel = Ident(selector.termRef).withPos(tree.pos) val rethrow = tpd.CaseDef(EmptyTree, EmptyTree, Throw(ref(selector))) val newCases = tpd.CaseDef( @@ -80,7 +80,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans // assert(owner ne null); assert(owner ne NoSymbol) def freshSym(pos: Position, tp: Type = NoType, prefix: String = "x", owner: Symbol = ctx.owner) = { ctr += 1 - ctx.newSymbol(owner, ctx.freshName(prefix + ctr).toTermName, Flags.Synthetic, tp, coord = pos) + ctx.newSymbol(owner, ctx.freshName(prefix + ctr).toTermName, Flags.Synthetic | Flags.Case, tp, coord = pos) } def newSynthCaseLabel(name: String, tpe:Type, owner: Symbol = ctx.owner) = From efee4faad746736c41d60ab0b488b615e7b54ed4 Mon Sep 17 00:00:00 2001 From: Felix Mulder Date: Tue, 24 May 2016 13:38:28 +0200 Subject: [PATCH 2/7] Rewrite `TypeApply` to null-check on rewrite to true, add docstrings --- .../transform/IsInstanceOfEvaluator.scala | 105 ++++++++++++++---- 1 file changed, 82 insertions(+), 23 deletions(-) diff --git a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala index d4483bb18a34..a8d5b7685ecd 100644 --- a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala +++ b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala @@ -1,38 +1,95 @@ package dotty.tools.dotc package transform -import dotty.tools.dotc.core.Contexts.Context -import dotty.tools.dotc.core.Types._ -import dotty.tools.dotc.core.Decorators._ -import dotty.tools.dotc.core.Flags -import dotty.tools.dotc.core.Symbols._ import dotty.tools.dotc.util.Positions._ -import dotty.tools.dotc.core.TypeErasure._ import TreeTransforms.{MiniPhaseTransform, TransformerInfo} - +import core._ +import Contexts.Context, Types._, Constants._, Decorators._, Symbols._ +import TypeUtils._, TypeErasure._ + + +/** Implements partial `isInstanceOf` evaluation according to the matrix on: + * https://github.com/lampepfl/dotty/issues/1255 + * + * This is a generalized solution to raising an error on unreachable match + * cases and warnings on other statically known results of `isInstanceOf`. + * + * Steps taken: + * + * 1. evalTypeApply will establish the matrix and choose the appropriate + * handling for the case: + * 2. a) handleStaticallyKnown + * b) falseIfUnrelated with `scrutinee <:< selector` + * c) handleFalseUnrelated + * d) leave as is (aka `happens`) + * 3. Rewrite according to step taken in `2` + */ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => import dotty.tools.dotc.ast.tpd._ - def phaseName = "reachabilityChecker" - + def phaseName = "isInstanceOfEvaluator" override def transformTypeApply(tree: TypeApply)(implicit ctx: Context, info: TransformerInfo): Tree = { val defn = ctx.definitions - def handleStaticallyKnown(tree: TypeApply, scrutinee: Type, selector: Type, inMatch: Boolean, pos: Position): Tree = + /** Handles the four cases of statically known `isInstanceOf`s and gives + * the correct warnings, or an error if statically known to be false in + * match + */ + def handleStaticallyKnown(tree: Select, scrutinee: Type, selector: Type, inMatch: Boolean, pos: Position): Tree = if (!(scrutinee <:< selector) && inMatch) { - ctx.error(s"this case is unreachable due to `${selector.show}` not being a subclass of `${scrutinee.show}`", pos) - tree + ctx.error( + s"this case is unreachable due to `${selector.show}` not being a subclass of `${scrutinee.show}`", + Position(pos.start - 5, pos.end - 5) + ) + rewrite(tree, to = false) } else if (!(scrutinee <:< selector) && !inMatch) { - ctx.warning(s"this will always yield false since `${scrutinee.show}` is not a subclass of `${selector.show}` (will be optimized away)", pos) + ctx.warning( + s"this will always yield false since `${scrutinee.show}` is not a subclass of `${selector.show}` (will be optimized away)", + pos + ) rewrite(tree, to = false) } else if (scrutinee <:< selector && !inMatch) { - ctx.warning(s"this will always yield true since `${scrutinee.show}` is a subclass of `${selector.show}` (will be optimized away)", pos) + ctx.warning( + s"this will always yield true since `${scrutinee.show}` is a subclass of `${selector.show}` (will be optimized away)", + pos + ) rewrite(tree, to = true) } else /* if (scrutinee <:< selector && inMatch) */ rewrite(tree, to = true) - def rewrite(tree: TypeApply, to: Boolean): Tree = tree + /** Rewrites cases with unrelated types */ + def handleFalseUnrelated(tree: Select, scrutinee: Type, selector: Type, inMatch: Boolean) = + if (inMatch) { + ctx.error( + s"will never match since `${selector.show}` is not a subclass of `${scrutinee.show}`", + Position(tree.pos.start - 5, tree.pos.end - 5) + ) + rewrite(tree, to = false) + } else { + ctx.warning( + s"will always yield false since `${scrutinee.show}` is not a subclass of `${selector.show}`", + tree.pos + ) + rewrite(tree, to = false) + } + /** Rewrites the select to a boolean if `to` is false or if the qualifier + * is a primitive. + * + * If `to` is set to true and the qualifier is not a primitive, the + * instanceOf is replaced by a null check, since: + * + * `srutinee.isInstanceOf[Selector]` if `scrutinee eq null` + */ + def rewrite(tree: Select, to: Boolean): Tree = + if (!to || tree.qualifier.tpe.widen.isPrimitiveValueType) + Literal(Constant(to)) + else + Apply(tree.qualifier.select(defn.Object_ne), List(Literal(Constant(null)))) + + /** Attempts to rewrite TypeApply to either `scrutinee ne null` or a + * constant + */ def evalTypeApply(tree: TypeApply): Tree = if (tree.symbol != defn.Any_isInstanceOf) tree else tree.fun match { @@ -66,19 +123,21 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => (scClassNonFinal && selFinalClass) || (scTrait && selFinalClass) - // Doesn't need to be calculated (since true if others are false) - //val happens = - // (scClassNonFinal && selClassNonFinal) || - // (scTrait && selClassNonFinal) || - // (scTrait && selTrait) + val happens = + (scClassNonFinal && selClassNonFinal) || + (scTrait && selClassNonFinal) || + (scTrait && selTrait) val inMatch = s.qualifier.symbol is Flags.Case if (knownStatically) - handleStaticallyKnown(tree, scrutinee, selector, inMatch, tree.pos) + handleStaticallyKnown(s, scrutinee, selector, inMatch, tree.pos) + else if (falseIfUnrelated && scrutinee <:< selector) + rewrite(s, to = true) else if (falseIfUnrelated && !(selector <:< scrutinee)) - rewrite(tree, to = false) - else /*if (happens)*/ tree + handleFalseUnrelated(s, scrutinee, selector, inMatch) + else if (happens) tree + else tree } case _ => tree From a06d2ab776ecfd5c1b48773032e071e871531760 Mon Sep 17 00:00:00 2001 From: Felix Mulder Date: Tue, 24 May 2016 14:44:54 +0200 Subject: [PATCH 3/7] Fix primitive rewriting --- src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala index a8d5b7685ecd..03ebb9515888 100644 --- a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala +++ b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala @@ -82,7 +82,7 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => * `srutinee.isInstanceOf[Selector]` if `scrutinee eq null` */ def rewrite(tree: Select, to: Boolean): Tree = - if (!to || tree.qualifier.tpe.widen.isPrimitiveValueType) + if (!to || !tree.qualifier.tpe.widen.derivesFrom(defn.AnyRefAlias)) Literal(Constant(to)) else Apply(tree.qualifier.select(defn.Object_ne), List(Literal(Constant(null)))) From 6b48e990d7e7d59cc5a67942a455eadcd9f4570f Mon Sep 17 00:00:00 2001 From: Felix Mulder Date: Tue, 24 May 2016 21:20:43 +0200 Subject: [PATCH 4/7] Address reviewer feedback --- .../transform/IsInstanceOfEvaluator.scala | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala index 03ebb9515888..8b1baa11a645 100644 --- a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala +++ b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala @@ -8,8 +8,15 @@ import Contexts.Context, Types._, Constants._, Decorators._, Symbols._ import TypeUtils._, TypeErasure._ -/** Implements partial `isInstanceOf` evaluation according to the matrix on: - * https://github.com/lampepfl/dotty/issues/1255 +/** Implements partial evaluation of `sc.isInstanceOf[Sel]` according to: + * + * +-------------+----------------------------+----------------------------+------------------+ + * | Sel\sc | trait | class | final class | + * +-------------+----------------------------+----------------------------+------------------+ + * | trait | ? | ? | statically known | + * | class | ? | false if classes unrelated | statically known | + * | final class | false if classes unrelated | false if classes unrelated | statically known | + * +-------------+----------------------------+----------------------------+------------------+ * * This is a generalized solution to raising an error on unreachable match * cases and warnings on other statically known results of `isInstanceOf`. @@ -36,26 +43,28 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => * the correct warnings, or an error if statically known to be false in * match */ - def handleStaticallyKnown(tree: Select, scrutinee: Type, selector: Type, inMatch: Boolean, pos: Position): Tree = - if (!(scrutinee <:< selector) && inMatch) { + def handleStaticallyKnown(tree: Select, scrutinee: Type, selector: Type, inMatch: Boolean, pos: Position): Tree = { + val scrutineeSubSelector = scrutinee <:< selector + if (!scrutineeSubSelector && inMatch) { ctx.error( s"this case is unreachable due to `${selector.show}` not being a subclass of `${scrutinee.show}`", Position(pos.start - 5, pos.end - 5) ) rewrite(tree, to = false) - } else if (!(scrutinee <:< selector) && !inMatch) { + } else if (!scrutineeSubSelector && !inMatch) { ctx.warning( s"this will always yield false since `${scrutinee.show}` is not a subclass of `${selector.show}` (will be optimized away)", pos ) rewrite(tree, to = false) - } else if (scrutinee <:< selector && !inMatch) { + } else if (scrutineeSubSelector && !inMatch) { ctx.warning( - s"this will always yield true since `${scrutinee.show}` is a subclass of `${selector.show}` (will be optimized away)", + 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)", pos ) rewrite(tree, to = true) - } else /* if (scrutinee <:< selector && inMatch) */ rewrite(tree, to = true) + } else /* if (scrutineeSubSelector && inMatch) */ rewrite(tree, to = true) + } /** Rewrites cases with unrelated types */ def handleFalseUnrelated(tree: Select, scrutinee: Type, selector: Type, inMatch: Boolean) = @@ -74,12 +83,12 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => } /** Rewrites the select to a boolean if `to` is false or if the qualifier - * is a primitive. + * is a value class. * * If `to` is set to true and the qualifier is not a primitive, the * instanceOf is replaced by a null check, since: * - * `srutinee.isInstanceOf[Selector]` if `scrutinee eq null` + * `scrutinee.isInstanceOf[Selector]` if `scrutinee eq null` */ def rewrite(tree: Select, to: Boolean): Tree = if (!to || !tree.qualifier.tpe.widen.derivesFrom(defn.AnyRefAlias)) From fc419f55e29b7e77e9062d40a0864086f8d51bfa Mon Sep 17 00:00:00 2001 From: Felix Mulder Date: Wed, 25 May 2016 12:50:36 +0200 Subject: [PATCH 5/7] 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 --- .../transform/IsInstanceOfEvaluator.scala | 63 +++++++++++-------- tests/{ => disabled}/pos-scala2/i1059.scala | 0 tests/{ => disabled}/pos/t3480.scala | 0 tests/neg/i1255.scala | 6 ++ tests/pos/t1168.scala | 2 +- tests/run/t6637.scala | 3 +- 6 files changed, 44 insertions(+), 30 deletions(-) rename tests/{ => disabled}/pos-scala2/i1059.scala (100%) rename tests/{ => disabled}/pos/t3480.scala (100%) create mode 100644 tests/neg/i1255.scala diff --git a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala index 8b1baa11a645..14c3dea71e56 100644 --- a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala +++ b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala @@ -5,7 +5,7 @@ import dotty.tools.dotc.util.Positions._ import TreeTransforms.{MiniPhaseTransform, TransformerInfo} import core._ import Contexts.Context, Types._, Constants._, Decorators._, Symbols._ -import TypeUtils._, TypeErasure._ +import TypeUtils._, TypeErasure._, Flags._ /** Implements partial evaluation of `sc.isInstanceOf[Sel]` according to: @@ -25,10 +25,11 @@ import TypeUtils._, TypeErasure._ * * 1. evalTypeApply will establish the matrix and choose the appropriate * handling for the case: - * 2. a) handleStaticallyKnown - * b) falseIfUnrelated with `scrutinee <:< selector` - * c) handleFalseUnrelated - * d) leave as is (aka `happens`) + * 2. a) Sel/sc is a value class or scrutinee is `Any` + * b) handleStaticallyKnown + * c) falseIfUnrelated with `scrutinee <:< selector` + * d) handleFalseUnrelated + * e) leave as is (aka `happens`) * 3. Rewrite according to step taken in `2` */ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => @@ -43,43 +44,43 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => * the correct warnings, or an error if statically known to be false in * match */ - def handleStaticallyKnown(tree: Select, scrutinee: Type, selector: Type, inMatch: Boolean, pos: Position): Tree = { + def handleStaticallyKnown(select: Select, scrutinee: Type, selector: Type, inMatch: Boolean, pos: Position): Tree = { val scrutineeSubSelector = scrutinee <:< selector if (!scrutineeSubSelector && inMatch) { ctx.error( s"this case is unreachable due to `${selector.show}` not being a subclass of `${scrutinee.show}`", Position(pos.start - 5, pos.end - 5) ) - rewrite(tree, to = false) + rewrite(select, to = false) } else if (!scrutineeSubSelector && !inMatch) { ctx.warning( s"this will always yield false since `${scrutinee.show}` is not a subclass of `${selector.show}` (will be optimized away)", pos ) - rewrite(tree, to = false) + rewrite(select, to = false) } else if (scrutineeSubSelector && !inMatch) { ctx.warning( 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)", pos ) - rewrite(tree, to = true) - } else /* if (scrutineeSubSelector && inMatch) */ rewrite(tree, to = true) + rewrite(select, to = true) + } else /* if (scrutineeSubSelector && inMatch) */ rewrite(select, to = true) } /** Rewrites cases with unrelated types */ - def handleFalseUnrelated(tree: Select, scrutinee: Type, selector: Type, inMatch: Boolean) = + def handleFalseUnrelated(select: Select, scrutinee: Type, selector: Type, inMatch: Boolean) = if (inMatch) { ctx.error( s"will never match since `${selector.show}` is not a subclass of `${scrutinee.show}`", - Position(tree.pos.start - 5, tree.pos.end - 5) + Position(select.pos.start - 5, select.pos.end - 5) ) - rewrite(tree, to = false) + rewrite(select, to = false) } else { ctx.warning( s"will always yield false since `${scrutinee.show}` is not a subclass of `${selector.show}`", - tree.pos + select.pos ) - rewrite(tree, to = false) + rewrite(select, to = false) } /** Rewrites the select to a boolean if `to` is false or if the qualifier @@ -106,25 +107,30 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => val scrutinee = erasure(s.qualifier.tpe.widen) val selector = erasure(tree.args.head.tpe.widen) - val scTrait = scrutinee.typeSymbol is Flags.Trait + val scTrait = scrutinee.typeSymbol is Trait val scClass = scrutinee.typeSymbol.isClass && - !(scrutinee.typeSymbol is Flags.Trait) && - !(scrutinee.typeSymbol is Flags.Module) + !(scrutinee.typeSymbol is Trait) && + !(scrutinee.typeSymbol is Module) - val scClassNonFinal = scClass && !scrutinee.typeSymbol.is(Flags.Final) - val scFinalClass = scClass && (scrutinee.typeSymbol is Flags.Final) + val scClassNonFinal = scClass && !(scrutinee.typeSymbol is Final) + val scFinalClass = scClass && (scrutinee.typeSymbol is Final) - val selTrait = selector.typeSymbol is Flags.Trait + val selTrait = selector.typeSymbol is Trait val selClass = selector.typeSymbol.isClass && - !(selector.typeSymbol is Flags.Trait) && - !(selector.typeSymbol is Flags.Module) + !(selector.typeSymbol is Trait) && + !(selector.typeSymbol is Module) - val selClassNonFinal = scClass && !(selector.typeSymbol is Flags.Final) - val selFinalClass = scClass && (selector.typeSymbol is Flags.Final) + val selClassNonFinal = scClass && !(selector.typeSymbol is Final) + val selFinalClass = scClass && (selector.typeSymbol is Final) // Cases --------------------------------- + val valueClassesOrAny = + ValueClasses.isDerivedValueClass(scrutinee.typeSymbol) || + ValueClasses.isDerivedValueClass(selector.typeSymbol) || + scrutinee == defn.ObjectType + val knownStatically = scFinalClass val falseIfUnrelated = @@ -137,13 +143,16 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => (scTrait && selClassNonFinal) || (scTrait && selTrait) - val inMatch = s.qualifier.symbol is Flags.Case + val inMatch = s.qualifier.symbol is Case - if (knownStatically) + if (valueClassesOrAny) tree + else if (knownStatically) handleStaticallyKnown(s, scrutinee, selector, inMatch, tree.pos) else if (falseIfUnrelated && scrutinee <:< selector) + // scrutinee is a subtype of the selector, safe to rewrite rewrite(s, to = true) else if (falseIfUnrelated && !(selector <:< scrutinee)) + // selector and scrutinee are unrelated handleFalseUnrelated(s, scrutinee, selector, inMatch) else if (happens) tree else tree diff --git a/tests/pos-scala2/i1059.scala b/tests/disabled/pos-scala2/i1059.scala similarity index 100% rename from tests/pos-scala2/i1059.scala rename to tests/disabled/pos-scala2/i1059.scala diff --git a/tests/pos/t3480.scala b/tests/disabled/pos/t3480.scala similarity index 100% rename from tests/pos/t3480.scala rename to tests/disabled/pos/t3480.scala diff --git a/tests/neg/i1255.scala b/tests/neg/i1255.scala new file mode 100644 index 000000000000..3bb7e8f25d35 --- /dev/null +++ b/tests/neg/i1255.scala @@ -0,0 +1,6 @@ +object Test { + def foo(x: Option[Int]) = x match { + case Some(_: Double) => true // error + case None => true + } +} diff --git a/tests/pos/t1168.scala b/tests/pos/t1168.scala index f43436812f09..08f1b5cd90e7 100644 --- a/tests/pos/t1168.scala +++ b/tests/pos/t1168.scala @@ -1,6 +1,6 @@ object Test extends App { - trait SpecialException {} + trait SpecialException extends Throwable {} try { throw new Exception diff --git a/tests/run/t6637.scala b/tests/run/t6637.scala index 7f9c3cd61c6e..052e7f5d6405 100644 --- a/tests/run/t6637.scala +++ b/tests/run/t6637.scala @@ -1,7 +1,6 @@ - object Test extends dotty.runtime.LegacyApp { try { - class A ; class B ; List().head.isInstanceOf[A with B] + List().head } catch { case _ :java.util.NoSuchElementException => println("ok") } From fa81e54db9ebb36c7f76cbedfa98115e50a8f88c Mon Sep 17 00:00:00 2001 From: Felix Mulder Date: Thu, 26 May 2016 11:41:19 +0200 Subject: [PATCH 6/7] Take side-effects into account during rewriting --- .../dotc/transform/IsInstanceOfEvaluator.scala | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala index 14c3dea71e56..584628acd22b 100644 --- a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala +++ b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala @@ -91,11 +91,16 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => * * `scrutinee.isInstanceOf[Selector]` if `scrutinee eq null` */ - def rewrite(tree: Select, to: Boolean): Tree = - if (!to || !tree.qualifier.tpe.widen.derivesFrom(defn.AnyRefAlias)) - Literal(Constant(to)) - else - Apply(tree.qualifier.select(defn.Object_ne), List(Literal(Constant(null)))) + def rewrite(tree: Select, to: Boolean): Tree = { + val expr = + if (!to || !tree.qualifier.tpe.widen.derivesFrom(defn.AnyRefAlias)) + Literal(Constant(to)) + else + Apply(tree.qualifier.select(defn.Object_ne), List(Literal(Constant(null)))) + + if (!isPureExpr(tree.qualifier)) Block(List(tree.qualifier), expr) + else expr + } /** Attempts to rewrite TypeApply to either `scrutinee ne null` or a * constant From 73f099ba506f6e877b676022a12cfdd31ab915b7 Mon Sep 17 00:00:00 2001 From: Felix Mulder Date: Thu, 26 May 2016 12:06:56 +0200 Subject: [PATCH 7/7] Fix double evaluation of scrutinee with side-effects, add test --- .../dotc/transform/IsInstanceOfEvaluator.scala | 17 +++++++---------- tests/run/isInstanceOf-eval.check | 2 ++ tests/run/isInstanceOf-eval.scala | 15 +++++++++++++++ 3 files changed, 24 insertions(+), 10 deletions(-) create mode 100644 tests/run/isInstanceOf-eval.check create mode 100644 tests/run/isInstanceOf-eval.scala diff --git a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala index 584628acd22b..6765604c8bf3 100644 --- a/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala +++ b/src/dotty/tools/dotc/transform/IsInstanceOfEvaluator.scala @@ -91,16 +91,13 @@ class IsInstanceOfEvaluator extends MiniPhaseTransform { thisTransformer => * * `scrutinee.isInstanceOf[Selector]` if `scrutinee eq null` */ - def rewrite(tree: Select, to: Boolean): Tree = { - val expr = - if (!to || !tree.qualifier.tpe.widen.derivesFrom(defn.AnyRefAlias)) - Literal(Constant(to)) - else - Apply(tree.qualifier.select(defn.Object_ne), List(Literal(Constant(null)))) - - if (!isPureExpr(tree.qualifier)) Block(List(tree.qualifier), expr) - else expr - } + def rewrite(tree: Select, to: Boolean): Tree = + if (!to || !tree.qualifier.tpe.widen.derivesFrom(defn.AnyRefAlias)) { + val literal = Literal(Constant(to)) + if (!isPureExpr(tree.qualifier)) Block(List(tree.qualifier), literal) + else literal + } else + Apply(tree.qualifier.select(defn.Object_ne), List(Literal(Constant(null)))) /** Attempts to rewrite TypeApply to either `scrutinee ne null` or a * constant diff --git a/tests/run/isInstanceOf-eval.check b/tests/run/isInstanceOf-eval.check new file mode 100644 index 000000000000..1191247b6d9a --- /dev/null +++ b/tests/run/isInstanceOf-eval.check @@ -0,0 +1,2 @@ +1 +2 diff --git a/tests/run/isInstanceOf-eval.scala b/tests/run/isInstanceOf-eval.scala new file mode 100644 index 000000000000..aa907b6972fa --- /dev/null +++ b/tests/run/isInstanceOf-eval.scala @@ -0,0 +1,15 @@ +object Test extends dotty.runtime.LegacyApp { + lazy val any = { + println(1) + 1: Any + } + + any.isInstanceOf[Int] + + lazy val int = { + println(2) + 2 + } + + int.isInstanceOf[Int] +}