Permalink
Browse files

no type test if static type <:< primitive value class

  • Loading branch information...
1 parent 494ba94 commit b47bb0fe1a0eb2f60f280ef0fa62a6f0be65580b @adriaanm adriaanm committed Jan 29, 2013
View
44 src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala
@@ -411,7 +411,17 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
// TODO: extractor.paramType may contain unbound type params (run/t2800, run/t3530)
// `patBinderOrCasted` is assigned the result of casting `patBinder` to `extractor.paramType`
val (typeTestTreeMaker, patBinderOrCasted, binderKnownNonNull) =
- if (needsTypeTest(patBinder.info.widen, extractor.paramType)) {
+ if (patBinder.info.widen <:< extractor.paramType) {
+ // no type test needed, but the tree maker relies on `patBinderOrCasted` having type `extractor.paramType` (and not just some type compatible with it)
+ // SI-6624 shows this is necessary because apparently patBinder may have an unfortunate type (.decls don't have the case field accessors)
+ // TODO: get to the bottom of this -- I assume it happens when type checking infers a weird type for an unapply call
+ // by going back to the parameterType for the extractor call we get a saner type, so let's just do that for now
+ /* TODO: uncomment when `settings.developer` and `devWarning` become available
+ if (settings.developer.value && !(patBinder.info =:= extractor.paramType))
+ devWarning(s"resetting info of $patBinder: ${patBinder.info} to ${extractor.paramType}")
+ */
+ (Nil, patBinder setInfo extractor.paramType, false)
+ } else {
// chain a type-testing extractor before the actual extractor call
// it tests the type, checks the outer pointer and casts to the expected type
// TODO: the outer check is mandated by the spec for case classes, but we do it for user-defined unapplies as well [SPEC]
@@ -422,16 +432,6 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
// even though the eventual null check will be on patBinderOrCasted
// it'll be equal to patBinder casted to extractor.paramType anyway (and the type test is on patBinder)
(List(treeMaker), treeMaker.nextBinder, treeMaker.impliesBinderNonNull(patBinder))
- } else {
- // no type test needed, but the tree maker relies on `patBinderOrCasted` having type `extractor.paramType` (and not just some type compatible with it)
- // SI-6624 shows this is necessary because apparently patBinder may have an unfortunate type (.decls don't have the case field accessors)
- // TODO: get to the bottom of this -- I assume it happens when type checking infers a weird type for an unapply call
- // by going back to the parameterType for the extractor call we get a saner type, so let's just do that for now
- /* TODO: uncomment when `settings.developer` and `devWarning` become available
- if (settings.developer.value && !(patBinder.info =:= extractor.paramType))
- devWarning(s"resetting info of $patBinder: ${patBinder.info} to ${extractor.paramType}")
- */
- (Nil, patBinder setInfo extractor.paramType, false)
}
withSubPats(typeTestTreeMaker :+ extractor.treeMaker(patBinderOrCasted, binderKnownNonNull, pos), extractor.subBindersAndPatterns: _*)
@@ -1176,9 +1176,6 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
override def toString = "P"+(prevBinder.name, extraCond getOrElse "", localSubstitution)
}
- // typetag-based tests are inserted by the type checker
- def needsTypeTest(tp: Type, pt: Type): Boolean = !(tp <:< pt)
-
object TypeTestTreeMaker {
// factored out so that we can consistently generate other representations of the tree that implements the test
// (e.g. propositions for exhaustivity and friends, boolean for isPureTypeTest)
@@ -1192,12 +1189,14 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
def equalsTest(pat: Tree, testedBinder: Symbol): Result
def eqTest(pat: Tree, testedBinder: Symbol): Result
def and(a: Result, b: Result): Result
+ def tru: Result
}
object treeCondStrategy extends TypeTestCondStrategy { import CODE._
type Result = Tree
def and(a: Result, b: Result): Result = a AND b
+ def tru = TRUE_typed
def typeTest(testedBinder: Symbol, expectedTp: Type) = codegen._isInstanceOf(testedBinder, expectedTp)
def nonNullTest(testedBinder: Symbol) = REF(testedBinder) OBJ_NE NULL
def equalsTest(pat: Tree, testedBinder: Symbol) = codegen._equals(pat, testedBinder)
@@ -1228,6 +1227,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
def equalsTest(pat: Tree, testedBinder: Symbol): Result = false
def eqTest(pat: Tree, testedBinder: Symbol): Result = false
def and(a: Result, b: Result): Result = false // we don't and type tests, so the conjunction must include at least one false
+ def tru = true
}
def nonNullImpliedByTestChecker(binder: Symbol) = new TypeTestCondStrategy {
@@ -1239,6 +1239,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
def equalsTest(pat: Tree, testedBinder: Symbol): Result = false // could in principle analyse pat and see if it's statically known to be non-null
def eqTest(pat: Tree, testedBinder: Symbol): Result = false // could in principle analyse pat and see if it's statically known to be non-null
def and(a: Result, b: Result): Result = a || b
+ def tru = false
}
}
@@ -1308,10 +1309,16 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
// I think it's okay:
// - the isInstanceOf test includes a test for the element type
// - Scala's arrays are invariant (so we don't drop type tests unsoundly)
- case _ if (expectedTp <:< AnyRefClass.tpe) && !needsTypeTest(testedBinder.info.widen, expectedTp) =>
- // do non-null check first to ensure we won't select outer on null
- if (outerTestNeeded) and(nonNullTest(testedBinder), outerTest(testedBinder, expectedTp))
- else nonNullTest(testedBinder)
+ case _ if testedBinder.info.widen <:< expectedTp =>
+ // if the expected type is a primitive value type, it cannot be null and it cannot have an outer pointer
+ // since the types conform, no further checking is required
+ if (expectedTp.typeSymbol.isPrimitiveValueClass) tru
+ // have to test outer and non-null only when it's a reference type
+ else if (expectedTp <:< AnyRefClass.tpe) {
+ // do non-null check first to ensure we won't select outer on null
+ if (outerTestNeeded) and(nonNullTest(testedBinder), outerTest(testedBinder, expectedTp))
+ else nonNullTest(testedBinder)
+ } else default
case _ => default
}
@@ -1823,6 +1830,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
def nonNullTest(testedBinder: Symbol) = NonNullCond(binderToUniqueTree(testedBinder))
def equalsTest(pat: Tree, testedBinder: Symbol) = EqualityCond(binderToUniqueTree(testedBinder), unique(pat))
def eqTest(pat: Tree, testedBinder: Symbol) = EqualityCond(binderToUniqueTree(testedBinder), unique(pat)) // TODO: eq, not ==
+ def tru = TrueCond
}
ttm.renderCondition(condStrategy)
case EqualityTestTreeMaker(prevBinder, patTree, _) => EqualityCond(binderToUniqueTree(prevBinder), unique(patTree))
View
1 test/files/jvm/patmat_opt_primitive_typetest.check
@@ -0,0 +1 @@
+bytecode identical
View
1 test/files/jvm/patmat_opt_primitive_typetest.flags
@@ -0,0 +1 @@
+-optimize
View
25 test/files/jvm/patmat_opt_primitive_typetest/Analyzed_1.scala
@@ -0,0 +1,25 @@
+// this class's bytecode, compiled under -optimize is analyzed by the test
+// method a's bytecode should be identical to method b's bytecode
+class SameBytecode {
+ case class Foo(x: Int, y: String)
+
+ def a =
+ Foo(1, "a") match {
+ case Foo(_: Int, y) => y
+ }
+
+ // this method's body holds the tree that should be generated by the pattern matcher for method a (-Xprint:patmat)
+ // the test checks that bytecode for a and b is identical (modulo line numbers)
+ // we can't diff trees as they are quite different (patmat uses jumps to labels that cannot be expressed in source, for example)
+ // note that the actual tree is quite bad: we do an unnecessary null check, and local val (x3)
+ // some of these will be fixed soon (the initial null check is for the scrutinee, which is harder to fix in patmat)
+ def b: String = {
+ val x1 = Foo(1, "a")
+ if (x1.ne(null)) {
+ val x3 = x1.x
+ return x1.y
+ }
+
+ throw new MatchError(x1)
+ }
+}
View
8 test/files/jvm/patmat_opt_primitive_typetest/test.scala
@@ -0,0 +1,8 @@
+import scala.tools.partest.BytecodeTest
+
+object Test extends BytecodeTest {
+ def show: Unit = {
+ val classNode = loadClassNode("SameBytecode")
+ sameBytecode(getMethod(classNode, "a"), getMethod(classNode, "b"))
+ }
+}

2 comments on commit b47bb0f

@scala-jenkins

Job pr-rangepos-per-commit failed for b47bb0f (results):


Took 4 min.
sad kitty
to rebuild, comment "PLS REBUILD/pr-rangepos-per-commit@b47bb0f"on PR #2112

@scala-jenkins

Job pr-checkin-per-commit failed for b47bb0f (results):


Took 5 min.
sad kitty
to rebuild, comment "PLS REBUILD/pr-checkin-per-commit@b47bb0f"on PR #2112

Please sign in to comment.