diff options
author | Adriaan Moors <adriaan.moors@typesafe.com> | 2016-03-14 19:17:52 -0700 |
---|---|---|
committer | Adriaan Moors <adriaan.moors@typesafe.com> | 2016-03-14 19:17:52 -0700 |
commit | d209a377bd73b71cb791f1b262acfe4b20b993c1 (patch) | |
tree | 55f100a96e152615502f2885066fd19e1c10017a /src | |
parent | 798462e739f23bb498b72675a57e4cc331404945 (diff) | |
parent | e7e1c77092ec0d64a6ecbb8b920d1b4cca74837f (diff) | |
download | scala-d209a377bd73b71cb791f1b262acfe4b20b993c1.tar.gz scala-d209a377bd73b71cb791f1b262acfe4b20b993c1.tar.bz2 scala-d209a377bd73b71cb791f1b262acfe4b20b993c1.zip |
Merge pull request #4974 from szeiger/wip/patmat-outertest
More conservative optimization for unnecessary outer ref checks
Diffstat (limited to 'src')
5 files changed, 55 insertions, 64 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala index 1a0b4cdec0..4abcc774da 100644 --- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala +++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala @@ -486,7 +486,8 @@ abstract class ExplicitOuter extends InfoTransform // since we can't fix SI-4440 properly (we must drop the outer accessors of final classes when there's no immediate reference to them in sight) // at least don't crash... this duplicates maybeOmittable from constructors (acc.owner.isEffectivelyFinal && !acc.isOverridingSymbol)) { - currentRun.reporting.uncheckedWarning(tree.pos, "The outer reference in this type test cannot be checked at run time.") + if (!base.tpe.hasAnnotation(UncheckedClass)) + currentRun.reporting.uncheckedWarning(tree.pos, "The outer reference in this type test cannot be checked at run time.") transform(TRUE) // urgh... drop condition if there's no accessor (or if it may disappear after constructors) } else { // println("(base, acc)= "+(base, acc)) diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala index ab3b25e76b..2b16995f0b 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala @@ -351,7 +351,7 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT object condStrategy extends TypeTestTreeMaker.TypeTestCondStrategy { type Result = Prop def and(a: Result, b: Result) = And(a, b) - def outerTest(testedBinder: Symbol, expectedTp: Type) = True // TODO OuterEqProp(testedBinder, expectedType) + def withOuterTest(testedBinder: Symbol, expectedTp: Type) = True // TODO OuterEqProp(testedBinder, expectedType) def typeTest(b: Symbol, pt: Type) = { // a type test implies the tested path is non-null (null.isInstanceOf[T] is false for all T) val p = binderToUniqueTree(b); And(uniqueNonNullProp(p), uniqueTypeProp(p, uniqueTp(pt))) } diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala index c6e7f8fcda..8c59ced28f 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala @@ -316,7 +316,7 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging { trait TypeTestCondStrategy { type Result - def outerTest(testedBinder: Symbol, expectedTp: Type): Result + def withOuterTest(orig: Result)(testedBinder: Symbol, expectedTp: Type): Result = orig // TODO: can probably always widen def typeTest(testedBinder: Symbol, expectedTp: Type): Result def nonNullTest(testedBinder: Symbol): Result @@ -336,18 +336,34 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging { def equalsTest(pat: Tree, testedBinder: Symbol) = codegen._equals(pat, testedBinder) def eqTest(pat: Tree, testedBinder: Symbol) = REF(testedBinder) OBJ_EQ pat - def outerTest(testedBinder: Symbol, expectedTp: Type): Tree = { - val expectedOuter = expectedTp.prefix match { - case ThisType(clazz) => This(clazz) - case NoType => mkTRUE // fallback for SI-6183 - case pre => REF(pre.prefix, pre.termSymbol) + override def withOuterTest(orig: Tree)(testedBinder: Symbol, expectedTp: Type): Tree = { + val expectedPrefix = expectedTp.prefix + val testedPrefix = testedBinder.info.prefix + + // Check if a type is defined in a static location. Unlike `tp.isStatic` before `flatten`, + // this also includes methods and (possibly nested) objects inside of methods. + def definedInStaticLocation(tp: Type): Boolean = { + def isStatic(tp: Type): Boolean = + if (tp == NoType || tp.typeSymbol.isPackageClass || tp == NoPrefix) true + else if (tp.typeSymbol.isModuleClass) isStatic(tp.prefix) + else false + tp.typeSymbol.owner == tp.prefix.typeSymbol && isStatic(tp.prefix) } - // ExplicitOuter replaces `Select(q, outerSym) OBJ_EQ expectedPrefix` by `Select(q, outerAccessor(outerSym.owner)) OBJ_EQ expectedPrefix` - // if there's an outer accessor, otherwise the condition becomes `true` -- TODO: can we improve needsOuterTest so there's always an outerAccessor? - val outer = expectedTp.typeSymbol.newMethod(vpmName.outer, newFlags = SYNTHETIC | ARTIFACT) setInfo expectedTp.prefix - - (Select(codegen._asInstanceOf(testedBinder, expectedTp), outer)) OBJ_EQ expectedOuter + if ((expectedPrefix eq NoPrefix) + || definedInStaticLocation(expectedTp) + || testedPrefix =:= expectedPrefix) orig + else gen.mkAttributedQualifierIfPossible(expectedPrefix) match { + case None => orig + case Some(expectedOuterRef) => + // ExplicitOuter replaces `Select(q, outerSym) OBJ_EQ expectedPrefix` + // by `Select(q, outerAccessor(outerSym.owner)) OBJ_EQ expectedPrefix` + // if there's an outer accessor, otherwise the condition becomes `true` + // TODO: centralize logic whether there's an outer accessor and use here? + val synthOuterGetter = expectedTp.typeSymbol.newMethod(vpmName.outer, newFlags = SYNTHETIC | ARTIFACT) setInfo expectedPrefix + val outerTest = (Select(codegen._asInstanceOf(testedBinder, expectedTp), synthOuterGetter)) OBJ_EQ expectedOuterRef + and(orig, outerTest) + } } } @@ -356,7 +372,6 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging { def typeTest(testedBinder: Symbol, expectedTp: Type): Result = true - def outerTest(testedBinder: Symbol, expectedTp: Type): Result = false def nonNullTest(testedBinder: Symbol): Result = false def equalsTest(pat: Tree, testedBinder: Symbol): Result = false def eqTest(pat: Tree, testedBinder: Symbol): Result = false @@ -368,7 +383,6 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging { type Result = Boolean def typeTest(testedBinder: Symbol, expectedTp: Type): Result = testedBinder eq binder - def outerTest(testedBinder: Symbol, expectedTp: Type): Result = false def nonNullTest(testedBinder: Symbol): Result = testedBinder eq binder 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 @@ -405,12 +419,6 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging { import TypeTestTreeMaker._ debug.patmat("TTTM"+((prevBinder, extractorArgTypeTest, testedBinder, expectedTp, nextBinderTp))) - lazy val outerTestNeeded = ( - (expectedTp.prefix ne NoPrefix) - && !expectedTp.prefix.typeSymbol.isPackageClass - && needsOuterTest(expectedTp, testedBinder.info, matchOwner) - ) - // the logic to generate the run-time test that follows from the fact that // a `prevBinder` is expected to have type `expectedTp` // the actual tree-generation logic is factored out, since the analyses generate Cond(ition)s rather than Trees @@ -429,12 +437,11 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging { def isExpectedPrimitiveType = isAsExpected && isPrimitiveValueType(expectedTp) def isExpectedReferenceType = isAsExpected && (expectedTp <:< AnyRefTpe) def mkNullTest = nonNullTest(testedBinder) - def mkOuterTest = outerTest(testedBinder, expectedTp) def mkTypeTest = typeTest(testedBinder, expectedWide) def mkEqualsTest(lhs: Tree): cs.Result = equalsTest(lhs, testedBinder) def mkEqTest(lhs: Tree): cs.Result = eqTest(lhs, testedBinder) - def addOuterTest(res: cs.Result): cs.Result = if (outerTestNeeded) and(res, mkOuterTest) else res + def addOuterTest(res: cs.Result): cs.Result = withOuterTest(res)(testedBinder, expectedTp) // If we conform to expected primitive type: // it cannot be null and cannot have an outer pointer. No further checking. diff --git a/src/reflect/scala/reflect/internal/TreeGen.scala b/src/reflect/scala/reflect/internal/TreeGen.scala index 2933aa3892..ec6426558c 100644 --- a/src/reflect/scala/reflect/internal/TreeGen.scala +++ b/src/reflect/scala/reflect/internal/TreeGen.scala @@ -117,6 +117,30 @@ abstract class TreeGen { case _ => qual } + + + // val selType = testedBinder.info + // + // // See the test for SI-7214 for motivation for dealias. Later `treeCondStrategy#outerTest` + // // generates an outer test based on `patType.prefix` with automatically dealises. + // // Prefixes can have all kinds of shapes SI-9110 + // val patPre = expectedTp.dealiasWiden.prefix + // val selPre = selType.dealiasWiden.prefix + // + // // Optimization: which prefixes can we disqualify from the need for an outer reference check? + // // - classes in static owners do not get outer pointers + // // - if the prefixes are statically known to be equal, the type system ensures an outer test is redundant + // !((patPre eq NoPrefix) || (selPre eq NoPrefix) + // || patPre.typeSymbol.isPackageClass + // || selPre =:= patPre) + + def mkAttributedQualifierIfPossible(prefix: Type): Option[Tree] = prefix match { + case NoType | NoPrefix | ErrorType => None + case TypeRef(_, sym, _) if sym.isModule || sym.isClass || sym.isType => None + case pre => Some(mkAttributedQualifier(prefix)) + } + + /** Builds a reference to given symbol with given stable prefix. */ def mkAttributedRef(pre: Type, sym: Symbol): RefTree = { val qual = mkAttributedQualifier(pre) diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index 685c9f7476..24709cf379 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -3946,47 +3946,6 @@ trait Types check(tp1, tp2) && check(tp2, tp1) } - /** Does a pattern of type `patType` need an outer test when executed against - * selector type `selType` in context defined by `currentOwner`? - */ - def needsOuterTest(patType: Type, selType: Type, currentOwner: Symbol) = { - def createDummyClone(pre: Type): Type = { - val dummy = currentOwner.enclClass.newValue(nme.ANYname).setInfo(pre.widen) - singleType(ThisType(currentOwner.enclClass), dummy) - } - def maybeCreateDummyClone(pre: Type, sym: Symbol): Type = pre match { - case SingleType(pre1, sym1) => - if (sym1.isModule && sym1.isStatic) { - if (sym.owner == sym1 || sym.isJavaDefined || sym.owner.sourceModule.isStaticModule) NoType - else pre - } else if (sym1.isModule && sym.owner == sym1.moduleClass) { - val pre2 = maybeCreateDummyClone(pre1, sym1) - if (pre2 eq NoType) pre2 - else singleType(pre2, sym1) - } else { - createDummyClone(pre) - } - case ThisType(clazz) => - if (clazz.isModuleClass) - maybeCreateDummyClone(clazz.typeOfThis, sym) - else if (sym.owner == clazz && (sym.hasFlag(PRIVATE) || sym.privateWithin == clazz)) - NoType - else - createDummyClone(pre) - case _ => - NoType - } - // See the test for SI-7214 for motivation for dealias. Later `treeCondStrategy#outerTest` - // generates an outer test based on `patType.prefix` with automatically dealises. - patType.dealias match { - case TypeRef(pre, sym, args) => - val pre1 = maybeCreateDummyClone(pre, sym) - (pre1 ne NoType) && isPopulated(copyTypeRef(patType, pre1, sym, args), selType) - case _ => - false - } - } - def normalizePlus(tp: Type): Type = { if (isRawType(tp)) rawToExistential(tp) else tp.normalize match { |