From eab288442931f01b5bad2dcfa244a6183db0f4b6 Mon Sep 17 00:00:00 2001 From: James Iry Date: Wed, 23 Jan 2013 16:01:59 -0800 Subject: SI-5313 Do not eliminate stores that potentially wipe referenes Storing to local variables of reference or array type is indirectly observable because it potentially allows gc to collect an object. So this commit makes DeadCodeElimination mark a store necessary if it assigns to a local that potentially stored by a previous necessary store. --- test/files/run/t5313.check | 8 ++++++++ test/files/run/t5313.scala | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 test/files/run/t5313.check create mode 100644 test/files/run/t5313.scala (limited to 'test/files/run') diff --git a/test/files/run/t5313.check b/test/files/run/t5313.check new file mode 100644 index 0000000000..dd8791f109 --- /dev/null +++ b/test/files/run/t5313.check @@ -0,0 +1,8 @@ +STORE_LOCAL(variable kept1) +STORE_LOCAL(value result) +STORE_LOCAL(variable kept1) +STORE_LOCAL(variable kept2) +STORE_LOCAL(value kept3) +STORE_LOCAL(variable kept2) +STORE_LOCAL(variable kept4) +STORE_LOCAL(variable kept4) diff --git a/test/files/run/t5313.scala b/test/files/run/t5313.scala new file mode 100644 index 0000000000..b65311622a --- /dev/null +++ b/test/files/run/t5313.scala @@ -0,0 +1,42 @@ +import scala.tools.partest.IcodeTest + +object Test extends IcodeTest { + override def printIcodeAfterPhase = "dce" + + override def extraSettings: String = super.extraSettings + " -optimize" + + override def code = + """class Foo { + def foo = true + def bar = { + var kept1 = new Object + val result = new java.lang.ref.WeakReference(kept1) + kept1 = null // we can't eliminate this assigment because result can observe + // when the object has no more references. See SI-5313 + var erased2 = null // we can eliminate this store because it's never used + val erased3 = erased2 // and this + var erased4 = erased2 // and this + val erased5 = erased4 // and this + var kept2: Object = new Object // ultimately can't be eliminated + while(foo) { + val kept3 = kept2 + kept2 = null // this can't, because it clobbers x, which is ultimately used + erased4 = null // safe to eliminate + println(kept3) + } + var kept4 = new Object // have to keep, it's used + try + println(kept4) + catch { + case _ : Throwable => kept4 = null // have to keep, it clobbers kept4 which is used + } + result + } + }""".stripMargin + + override def show() { + val storeLocal = "STORE_LOCAL" + val lines1 = collectIcode("") filter (_ contains storeLocal) map (x => x.drop(x.indexOf(storeLocal))) + println(lines1 mkString "\n") + } +} -- cgit v1.2.3 From 9b4fa8382fe0ed6fef3ff91e2c153a1840c954b9 Mon Sep 17 00:00:00 2001 From: James Iry Date: Tue, 29 Jan 2013 09:13:30 -0800 Subject: SI-5313 Eliminate more stores by replacing clobbers with null stores When an unused store clobbers a previous store, replace it with storing a null. Don't mark clobbers as "used" so that the original clobber and all following clobbers can still be eliminated. --- .../nsc/backend/opt/DeadCodeElimination.scala | 128 +++++++++++++-------- test/files/run/t5313.check | 2 + test/files/run/t5313.scala | 8 +- 3 files changed, 87 insertions(+), 51 deletions(-) (limited to 'test/files/run') diff --git a/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala b/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala index 90d5686bd0..53270c0651 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala @@ -68,6 +68,9 @@ abstract class DeadCodeElimination extends SubComponent { /** Map from a local and a basic block to the instructions that store to that local in that basic block */ val localStores = mutable.Map[(Local, BasicBlock), mutable.BitSet]() + + /** Stores that clobber previous stores to array or ref locals. See SI-5313 */ + val clobbers = mutable.Set[(BasicBlock, Int)]() /** the current method. */ var method: IMethod = _ @@ -80,6 +83,7 @@ abstract class DeadCodeElimination extends SubComponent { debuglog("dead code elimination on " + m); dropOf.clear() localStores.clear() + clobbers.clear() m.code.blocks.clear() accessedLocals = m.params.reverse m.code.blocks ++= linearizer.linearize(m) @@ -201,58 +205,15 @@ abstract class DeadCodeElimination extends SubComponent { worklist += ((bb1, idx1)) } - // Storing to local variables of reference or array type may be indirectly - // observable because may remove a reference to an object which may allow the object to be - // gc'd. See SI-5313. In this code I call the LOCAL_STORE(s) that immediately follow a - // LOCAL_STORE and that store to the same local "clobbers." If a LOCAL_STORE is marked - // useful then its clobbers must also go into the worklist to be marked useful. case STORE_LOCAL(l1) if l1.kind.isRefOrArrayType => addDefs() - // previously visited blocks tracked to prevent searching forever in a cycle - val inspected = mutable.Set[BasicBlock]() - // our worklist of blocks that still need to be checked - val blocksToBeInspected = mutable.Set[BasicBlock]() - - // Tries to find the next clobber of l1 starting at idx1 in bb1. - // if it finds any it adds them clobber to the worklist. - // If not it adds both bb's exception blocks and direct - // successor blocks to the uninspectedBlocks - def findClobber(idx1: Int, bb1: BasicBlock) { - val key = ((l1, bb1)) - val foundClobber = (localStores contains key) && { - def minIdx(s : mutable.BitSet) = if(s.isEmpty) -1 else s.min - - // find the smallest index greater than or equal to idx1 - val clobberIdx = minIdx(localStores(key) dropWhile (_ < idx1)) - if (clobberIdx == -1) - false - else { - debuglog(s"\t${bb1(clobberIdx)} is a clobber of $instr") - if (!useful(bb1)(clobberIdx)) worklist += ((bb1, clobberIdx)) - true - } - } - - // if we found a clobber in this block then we don't need to add the successors - // because they'll be picked up when the worklist comes around to work on that clobber - if (!foundClobber) { - blocksToBeInspected ++= (bb1.exceptionSuccessors filterNot inspected) - blocksToBeInspected ++= (bb1.directSuccessors filterNot inspected) - } - } - - // first search starting at the current index - // note we don't put bb in the inspected list yet because a loop may later force - // us back around to search from the beginning of bb - findClobber(idx + 1, bb) - // then loop until we've exhausted the set of uninspected blocks - while(!blocksToBeInspected.isEmpty) { - val bb1 = blocksToBeInspected.head - blocksToBeInspected -= bb1 - inspected += bb1 - findClobber(0, bb1) - } - + // see SI-5313 + // search for clobbers of this store if we aren't doing l1 = null + // this doesn't catch the second store in x=null;l1=x; but in practice this catches + // a lot of null stores very cheaply + if (idx == 0 || bb(idx - 1) != CONSTANT(Constant(null))) + findClobbers(l1, bb, idx + 1) + case nw @ NEW(REFERENCE(sym)) => assert(nw.init ne null, "null new.init at: " + bb + ": " + idx + "(" + instr + ")") worklist += findInstruction(bb, nw.init) @@ -277,6 +238,67 @@ abstract class DeadCodeElimination extends SubComponent { } } } + + /** + * Finds and marks all clobbers of the given local starting in the given + * basic block at the given index + * + * Storing to local variables of reference or array type may be indirectly + * observable because it may remove a reference to an object which may allow the object + * to be gc'd. See SI-5313. In this code I call the LOCAL_STORE(s) that immediately follow a + * LOCAL_STORE and that store to the same local "clobbers." If a LOCAL_STORE is marked + * useful then its clobbers must go into the set of clobbers, which will be + * compensated for later + */ + def findClobbers(l: Local, bb: BasicBlock, idx: Int) { + // previously visited blocks tracked to prevent searching forever in a cycle + val inspected = mutable.Set[BasicBlock]() + // our worklist of blocks that still need to be checked + val blocksToBeInspected = mutable.Set[BasicBlock]() + + // Tries to find the next clobber of l1 in bb1 starting at idx1. + // if it finds one it adds the clobber to clobbers set for later + // handling. If not it adds the direct successor blocks to + // the uninspectedBlocks to try to find clobbers there. Either way + // it adds the exception successor blocks for further search + def findClobberInBlock(idx1: Int, bb1: BasicBlock) { + val key = ((l, bb1)) + val foundClobber = (localStores contains key) && { + def minIdx(s : mutable.BitSet) = if(s.isEmpty) -1 else s.min + + // find the smallest index greater than or equal to idx1 + val clobberIdx = minIdx(localStores(key) dropWhile (_ < idx1)) + if (clobberIdx == -1) + false + else { + debuglog(s"\t${bb1(clobberIdx)} is a clobber of ${bb(idx)}") + clobbers += ((bb1, clobberIdx)) + true + } + } + + // always need to look into the exception successors for additional clobbers + // because we don't know when flow might enter an exception handler + blocksToBeInspected ++= (bb1.exceptionSuccessors filterNot inspected) + // If we didn't find a clobber here then we need to look at successor blocks. + // if we found a clobber then we don't need to search in the direct successors + if (!foundClobber) { + blocksToBeInspected ++= (bb1.directSuccessors filterNot inspected) + } + } + + // first search starting at the current index + // note we don't put bb in the inspected list yet because a loop may later force + // us back around to search from the beginning of bb + findClobberInBlock(idx, bb) + // then loop until we've exhausted the set of uninspected blocks + while(!blocksToBeInspected.isEmpty) { + val bb1 = blocksToBeInspected.head + blocksToBeInspected -= bb1 + inspected += bb1 + findClobberInBlock(0, bb1) + } + } def sweep(m: IMethod) { val compensations = computeCompensations(m) @@ -306,6 +328,12 @@ abstract class DeadCodeElimination extends SubComponent { i match { case NEW(REFERENCE(sym)) => log(s"Eliminated instantation of $sym inside $m") + case STORE_LOCAL(l) if clobbers contains ((bb, idx)) => + // if an unused instruction was a clobber of a used store to a reference or array type + // then we'll replace it with the store of a null to make sure the reference is + // eliminated. See SI-5313 + bb emit CONSTANT(Constant(null)) + bb emit STORE_LOCAL(l) case _ => () } debuglog("Skipped: bb_" + bb + ": " + idx + "( " + i + ")") diff --git a/test/files/run/t5313.check b/test/files/run/t5313.check index dd8791f109..aa30c0efa5 100644 --- a/test/files/run/t5313.check +++ b/test/files/run/t5313.check @@ -6,3 +6,5 @@ STORE_LOCAL(value kept3) STORE_LOCAL(variable kept2) STORE_LOCAL(variable kept4) STORE_LOCAL(variable kept4) +STORE_LOCAL(variable kept5) +STORE_LOCAL(variable kept5) diff --git a/test/files/run/t5313.scala b/test/files/run/t5313.scala index b65311622a..64009e29af 100644 --- a/test/files/run/t5313.scala +++ b/test/files/run/t5313.scala @@ -13,6 +13,7 @@ object Test extends IcodeTest { val result = new java.lang.ref.WeakReference(kept1) kept1 = null // we can't eliminate this assigment because result can observe // when the object has no more references. See SI-5313 + kept1 = new Object // but we can eliminate this one because kept1 has already been clobbered var erased2 = null // we can eliminate this store because it's never used val erased3 = erased2 // and this var erased4 = erased2 // and this @@ -20,7 +21,7 @@ object Test extends IcodeTest { var kept2: Object = new Object // ultimately can't be eliminated while(foo) { val kept3 = kept2 - kept2 = null // this can't, because it clobbers x, which is ultimately used + kept2 = null // this can't, because it clobbers kept2, which is used erased4 = null // safe to eliminate println(kept3) } @@ -30,6 +31,11 @@ object Test extends IcodeTest { catch { case _ : Throwable => kept4 = null // have to keep, it clobbers kept4 which is used } + var kept5 = new Object + print(kept5) + kept5 = null // can't eliminate it's a clobber and it's used + print(kept5) + kept5 = null // can eliminate because we don't care about clobbers of nulls result } }""".stripMargin -- cgit v1.2.3 From 2403d1ddcaa1bd76c1f376a32ec03a36d4dab48b Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Thu, 31 Jan 2013 19:46:51 +0100 Subject: SI-7046 reflection now auto-initializes knownDirectSubclasses knownDirectSubclasses joins the happy family of flags, annotations and privateWithin, which automatically trigger initialization, when used within runtime reflection. --- src/reflect/scala/reflect/internal/Symbols.scala | 6 +++++- test/files/run/t7046.check | 2 ++ test/files/run/t7046.scala | 13 +++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 test/files/run/t7046.check create mode 100644 test/files/run/t7046.scala (limited to 'test/files/run') diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index c1b868f3cb..f4d70a991c 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -86,7 +86,11 @@ trait Symbols extends api.Symbols { self: SymbolTable => case n: TypeName => if (isClass) newClassSymbol(n, pos, newFlags) else newNonClassSymbol(n, pos, newFlags) } - def knownDirectSubclasses = children + def knownDirectSubclasses = { + if (!isCompilerUniverse && needsInitialize(isFlagRelated = false, mask = 0)) initialize + children + } + def baseClasses = info.baseClasses def module = sourceModule def thisPrefix: Type = thisType diff --git a/test/files/run/t7046.check b/test/files/run/t7046.check new file mode 100644 index 0000000000..427f1ce610 --- /dev/null +++ b/test/files/run/t7046.check @@ -0,0 +1,2 @@ +Set(class D, class E) +Set(class D, class E) diff --git a/test/files/run/t7046.scala b/test/files/run/t7046.scala new file mode 100644 index 0000000000..647a15cd18 --- /dev/null +++ b/test/files/run/t7046.scala @@ -0,0 +1,13 @@ +import scala.reflect.runtime.universe._ +import scala.reflect.runtime.{currentMirror => cm} + +sealed class C +class D extends C +class E extends C + +object Test extends App { + val c = cm.staticClass("C") + println(c.knownDirectSubclasses) + c.typeSignature + println(c.knownDirectSubclasses) +} \ No newline at end of file -- cgit v1.2.3 From 71ea3e8278aad030cbe8c9093fe49790a4e419cb Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Fri, 25 Jan 2013 14:16:27 -0800 Subject: no null check for type-tested unapply arg pattern matching on case classes where pattern is not known to be a subclass of the unapply's argument type used to result in code like: ``` if (x1.isInstanceOf[Foo]) { val x2 = x1.asInstanceOf[Foo] if (x2 != null) { // redundant ... } } ``` this wastes byte code on the redundant null check with this patch, when previous type tests imply the variable cannot be null, there's no null check --- .../tools/nsc/typechecker/PatternMatching.scala | 35 +++- test/files/jvm/patmat_opt_no_nullcheck.check | 1 + test/files/jvm/patmat_opt_no_nullcheck.flags | 1 + .../jvm/patmat_opt_no_nullcheck/Analyzed_1.scala | 25 +++ test/files/jvm/patmat_opt_no_nullcheck/test.scala | 8 + test/files/run/inline-ex-handlers.check | 206 ++++++++++----------- 6 files changed, 167 insertions(+), 109 deletions(-) create mode 100644 test/files/jvm/patmat_opt_no_nullcheck.check create mode 100644 test/files/jvm/patmat_opt_no_nullcheck.flags create mode 100644 test/files/jvm/patmat_opt_no_nullcheck/Analyzed_1.scala create mode 100644 test/files/jvm/patmat_opt_no_nullcheck/test.scala (limited to 'test/files/run') diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index 9b2898741e..f32ca4bd8e 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -409,6 +409,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // example check: List[Int] <:< ::[Int] // 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)) { // chain a type-testing extractor before the actual extractor call @@ -416,7 +417,11 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // TODO: the outer check is mandated by the spec for case classes, but we do it for user-defined unapplies as well [SPEC] // (the prefix of the argument passed to the unapply must equal the prefix of the type of the binder) val treeMaker = TypeTestTreeMaker(patBinder, patBinder, extractor.paramType, extractor.paramType)(pos, extractorArgTypeTest = true) - (List(treeMaker), treeMaker.nextBinder, false) + + // check whether typetest implies patBinder is not null, + // 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) @@ -773,11 +778,16 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL def resultType = tpe.finalResultType def isSeq = extractorCall.symbol.name == nme.unapplySeq - /** Create the TreeMaker that embodies this extractor call - * - * `binder` has been casted to `paramType` if necessary - * `binderKnownNonNull` is not used in this subclass - */ + /** Create the TreeMaker that embodies this extractor call + * + * `binder` has been casted to `paramType` if necessary + * `binderKnownNonNull` is not used in this subclass + * + * TODO: implement review feedback by @retronym: + * Passing the pair of values around suggests: + * case class Binder(sym: Symbol, knownNotNull: Boolean). + * Perhaps it hasn't reached critical mass, but it would already clean things up a touch. + */ def treeMaker(patBinderOrCasted: Symbol, binderKnownNonNull: Boolean, pos: Position): TreeMaker = { // the extractor call (applied to the binder bound by the flatMap corresponding to the previous (i.e., enclosing/outer) pattern) val extractorApply = atPos(pos)(spliceApply(patBinderOrCasted)) @@ -1177,6 +1187,17 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL 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 nonNullImpliedByTestChecker(binder: Symbol) = new TypeTestCondStrategy { + 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 + def and(a: Result, b: Result): Result = a || b + } } /** implements the run-time aspects of (§8.2) (typedPattern has already done the necessary type transformations) @@ -1260,6 +1281,8 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // is this purely a type test, e.g. no outer check, no equality tests (used in switch emission) def isPureTypeTest = renderCondition(pureTypeTestChecker) + def impliesBinderNonNull(binder: Symbol) = renderCondition(nonNullImpliedByTestChecker(binder)) + override def toString = "TT"+(expectedTp, testedBinder.name, nextBinderTp) } diff --git a/test/files/jvm/patmat_opt_no_nullcheck.check b/test/files/jvm/patmat_opt_no_nullcheck.check new file mode 100644 index 0000000000..43f53aba12 --- /dev/null +++ b/test/files/jvm/patmat_opt_no_nullcheck.check @@ -0,0 +1 @@ +bytecode identical diff --git a/test/files/jvm/patmat_opt_no_nullcheck.flags b/test/files/jvm/patmat_opt_no_nullcheck.flags new file mode 100644 index 0000000000..1182725e86 --- /dev/null +++ b/test/files/jvm/patmat_opt_no_nullcheck.flags @@ -0,0 +1 @@ +-optimize \ No newline at end of file diff --git a/test/files/jvm/patmat_opt_no_nullcheck/Analyzed_1.scala b/test/files/jvm/patmat_opt_no_nullcheck/Analyzed_1.scala new file mode 100644 index 0000000000..1594eb523c --- /dev/null +++ b/test/files/jvm/patmat_opt_no_nullcheck/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 +case class Foo(x: Any) + +class SameBytecode { + def a = + (Foo(1): Any) match { + case Foo(_: String) => + } + + // there's no null check + def b: Unit = { + val x1: Any = Foo(1) + if (x1.isInstanceOf[Foo]) { + val x3 = x1.asInstanceOf[Foo] + if (x3.x.isInstanceOf[String]) { + val x4 = x3.x.asInstanceOf[String] + val x = () + return + } + } + + throw new MatchError(x1) + } +} \ No newline at end of file diff --git a/test/files/jvm/patmat_opt_no_nullcheck/test.scala b/test/files/jvm/patmat_opt_no_nullcheck/test.scala new file mode 100644 index 0000000000..2927e763d5 --- /dev/null +++ b/test/files/jvm/patmat_opt_no_nullcheck/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")) + } +} diff --git a/test/files/run/inline-ex-handlers.check b/test/files/run/inline-ex-handlers.check index 282542a732..905dfc3ee7 100644 --- a/test/files/run/inline-ex-handlers.check +++ b/test/files/run/inline-ex-handlers.check @@ -26,54 +26,55 @@ --- > locals: value args, variable result, value ex6, value x4, value x5, value x 397c393 -< blocks: [1,2,3,4,5,8,11,13,14,16] +< blocks: [1,2,3,4,5,8,10,11,13] --- -> blocks: [1,2,3,5,8,11,13,14,16,17] +> blocks: [1,2,3,5,8,10,11,13,14] 421c417,426 < 103 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 17 +> ? JUMP 14 > -> 17: +> 14: > 101 LOAD_LOCAL(value ex6) > 101 STORE_LOCAL(value x4) > 101 SCOPE_ENTER value x4 > 106 LOAD_LOCAL(value x4) > 106 IS_INSTANCE REF(class MyException) -> 106 CZJUMP (BOOL)NE ? 5 : 11 +> 106 CZJUMP (BOOL)NE ? 5 : 8 434,436d438 < 101 JUMP 4 < < 4: -450,453d451 +446,449d447 < 106 LOAD_LOCAL(value x5) < 106 CALL_METHOD MyException.message (dynamic) < 106 STORE_LOCAL(value message) < 106 SCOPE_ENTER value message -455c453,454 +451c449,450 < 106 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 106 CALL_METHOD MyException.message (dynamic) -527c526 +523c522 < blocks: [1,2,3,4,6,7,8,9,10] --- > blocks: [1,2,3,4,6,7,8,9,10,11,12,13] -556c555,560 +552c551 < 306 THROW(MyException) --- > ? JUMP 11 -> +553a553,557 > 11: > ? LOAD_LOCAL(variable monitor4) > 305 MONITOR_EXIT > ? JUMP 12 -562c566 +> +558c562 < ? THROW(Throwable) --- > ? JUMP 12 -568c572,579 +564c568,575 < ? THROW(Throwable) --- > ? STORE_LOCAL(value t) @@ -84,7 +85,7 @@ > 304 MONITOR_EXIT > ? STORE_LOCAL(value t) > ? JUMP 13 -583a595,606 +579a591,602 > 13: > 310 LOAD_MODULE object Predef > 310 CALL_PRIMITIVE(StartConcat) @@ -97,35 +98,35 @@ > 310 CALL_METHOD scala.Predef.println (dynamic) > 310 JUMP 2 > -592c615 +588c611 < catch (Throwable) in ArrayBuffer(7, 8, 9, 10) starting at: 6 --- > catch (Throwable) in ArrayBuffer(7, 8, 9, 10, 11) starting at: 6 -595c618 +591c614 < catch (Throwable) in ArrayBuffer(4, 6, 7, 8, 9, 10) starting at: 3 --- > catch (Throwable) in ArrayBuffer(4, 6, 7, 8, 9, 10, 11, 12) starting at: 3 -627c650 +623c646 < blocks: [1,2,3,4,5,6,7,9,10] --- > blocks: [1,2,3,4,5,6,7,9,10,11,12] -651c674,675 +647c670,671 < 78 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) > ? JUMP 11 -652a677,681 +648a673,677 > 11: > 81 LOAD_LOCAL(value e) > ? STORE_LOCAL(variable exc1) > ? JUMP 12 > -680c709,710 +676c705,706 < 81 THROW(Exception) --- > ? STORE_LOCAL(variable exc1) > ? JUMP 12 -696a727,739 +692a723,735 > 12: > 83 LOAD_MODULE object Predef > 83 CONSTANT("finally") @@ -139,88 +140,88 @@ > 84 LOAD_LOCAL(variable exc1) > 84 THROW(Throwable) > -702c745 +698c741 < catch () in ArrayBuffer(4, 6, 7, 9) starting at: 3 --- > catch () in ArrayBuffer(4, 6, 7, 9, 11) starting at: 3 -726c769 +722c765 < locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value message, value x, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value x, value ex6, value x4, value x5, value x -728c771 -< blocks: [1,2,3,4,5,6,9,12,14,17,18,19,22,25,27,28,30,31] +724c767 +< blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25] --- -> blocks: [1,2,3,4,5,6,9,12,14,17,18,19,22,25,27,28,30,31,32,33,34] -752c795,802 +> blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25,26,27,28] +748c791,798 < 172 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 32 +> ? JUMP 26 > -> 32: +> 26: > 170 LOAD_LOCAL(value ex6) > 170 STORE_LOCAL(value x4) > 170 SCOPE_ENTER value x4 -> 170 JUMP 18 -799,802d848 +> 170 JUMP 15 +791,794d840 < 175 LOAD_LOCAL(value x5) < 175 CALL_METHOD MyException.message (dynamic) < 175 STORE_LOCAL(value message) < 175 SCOPE_ENTER value message -804c850,851 +796c842,843 < 176 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 176 CALL_METHOD MyException.message (dynamic) -808c855,856 +800c847,848 < 177 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 177 CALL_METHOD MyException.message (dynamic) -810c858,859 +802c850,851 < 177 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 33 -814c863,864 +> ? JUMP 27 +806c855,856 < 170 THROW(Throwable) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 33 -823a874,879 -> 33: +> ? JUMP 27 +815a866,871 +> 27: > 169 LOAD_LOCAL(value ex6) > 169 STORE_LOCAL(value x4) > 169 SCOPE_ENTER value x4 > 169 JUMP 5 > -838,841d893 +826,829d881 < 180 LOAD_LOCAL(value x5) < 180 CALL_METHOD MyException.message (dynamic) < 180 STORE_LOCAL(value message) < 180 SCOPE_ENTER value message -843c895,896 +831c883,884 < 181 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 181 CALL_METHOD MyException.message (dynamic) -847c900,901 +835c888,889 < 182 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 182 CALL_METHOD MyException.message (dynamic) -849c903,904 +837c891,892 < 182 THROW(MyException) --- > ? STORE_LOCAL(variable exc2) -> ? JUMP 34 -853c908,909 +> ? JUMP 28 +841c896,897 < 169 THROW(Throwable) --- > ? STORE_LOCAL(variable exc2) -> ? JUMP 34 -869a926,938 -> 34: +> ? JUMP 28 +857a914,926 +> 28: > 184 LOAD_MODULE object Predef > 184 CONSTANT("finally") > 184 CALL_METHOD scala.Predef.println (dynamic) @@ -233,159 +234,158 @@ > 185 LOAD_LOCAL(variable exc2) > 185 THROW(Throwable) > -875c944 -< catch (Throwable) in ArrayBuffer(17, 18, 19, 22, 25, 27, 28, 30) starting at: 4 +863c932 +< catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24) starting at: 4 --- -> catch (Throwable) in ArrayBuffer(17, 18, 19, 22, 25, 27, 28, 30, 32) starting at: 4 -878c947 -< catch () in ArrayBuffer(4, 5, 6, 9, 12, 17, 18, 19, 22, 25, 27, 28, 30) starting at: 3 +> catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24, 26) starting at: 4 +866c935 +< catch () in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24) starting at: 3 --- -> catch () in ArrayBuffer(4, 5, 6, 9, 12, 17, 18, 19, 22, 25, 27, 28, 30, 32, 33) starting at: 3 -902c971 +> catch () in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24, 26, 27) starting at: 3 +890c959 < locals: value args, variable result, value e, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value e, value ex6, value x4, value x5, value x -904c973 -< blocks: [1,2,3,6,7,8,11,14,16,17,19] +892c961 +< blocks: [1,2,3,6,7,8,11,13,14,16] --- -> blocks: [1,2,3,6,7,8,11,14,16,17,19,20] -928c997,1004 +> blocks: [1,2,3,6,7,8,11,13,14,16,17] +916c985,992 < 124 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 20 +> ? JUMP 17 > -> 20: +> 17: > 122 LOAD_LOCAL(value ex6) > 122 STORE_LOCAL(value x4) > 122 SCOPE_ENTER value x4 > 122 JUMP 7 -957,960d1032 +941,944d1016 < 127 LOAD_LOCAL(value x5) < 127 CALL_METHOD MyException.message (dynamic) < 127 STORE_LOCAL(value message) < 127 SCOPE_ENTER value message -962c1034,1035 +946c1018,1019 < 127 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 127 CALL_METHOD MyException.message (dynamic) -991c1064 -< catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 14, 16, 17, 19) starting at: 3 +975c1048 +< catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16) starting at: 3 --- -> catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 14, 16, 17, 19, 20) starting at: 3 -1015c1088 +> catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16, 17) starting at: 3 +999c1072 < locals: value args, variable result, value ex6, value x4, value x5, value message, value x, value e --- > locals: value args, variable result, value ex6, value x4, value x5, value x, value e -1017c1090 -< blocks: [1,2,3,4,5,8,11,15,16,17,19] +1001c1074 +< blocks: [1,2,3,4,5,8,12,13,14,16] --- -> blocks: [1,2,3,5,8,11,15,16,17,19,20] -1041c1114,1123 +> blocks: [1,2,3,5,8,12,13,14,16,17] +1025c1098,1107 < 148 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 20 +> ? JUMP 17 > -> 20: +> 17: > 145 LOAD_LOCAL(value ex6) > 145 STORE_LOCAL(value x4) > 145 SCOPE_ENTER value x4 > 154 LOAD_LOCAL(value x4) > 154 IS_INSTANCE REF(class MyException) -> 154 CZJUMP (BOOL)NE ? 5 : 11 -1062,1064d1143 +> 154 CZJUMP (BOOL)NE ? 5 : 8 +1046,1048d1127 < 145 JUMP 4 < < 4: -1078,1081d1156 +1058,1061d1136 < 154 LOAD_LOCAL(value x5) < 154 CALL_METHOD MyException.message (dynamic) < 154 STORE_LOCAL(value message) < 154 SCOPE_ENTER value message -1083c1158,1159 +1063c1138,1139 < 154 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 154 CALL_METHOD MyException.message (dynamic) -1300c1376 +1280c1356 < blocks: [1,2,3,4,5,7] --- > blocks: [1,2,3,4,5,7,8] -1324c1400,1401 +1304c1380,1387 < 38 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) > ? JUMP 8 -1325a1403,1408 +> > 8: > 42 LOAD_MODULE object Predef > 42 CONSTANT("IllegalArgumentException") > 42 CALL_METHOD scala.Predef.println (dynamic) > 42 JUMP 2 -> -1371c1454 +1351c1434 < locals: value args, variable result, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value ex6, value x4, value x5, value x -1373c1456 -< blocks: [1,2,3,4,5,8,11,13,14,16,17,19] +1353c1436 +< blocks: [1,2,3,4,5,8,10,11,13,14,16] --- -> blocks: [1,2,3,5,8,11,13,14,16,17,19,20] -1397c1480,1481 +> blocks: [1,2,3,5,8,10,11,13,14,16,17] +1377c1460,1461 < 203 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 20 -1417c1501,1510 +> ? JUMP 17 +1397c1481,1490 < 209 THROW(MyException) --- > ? STORE_LOCAL(value ex6) -> ? JUMP 20 +> ? JUMP 17 > -> 20: +> 17: > 200 LOAD_LOCAL(value ex6) > 200 STORE_LOCAL(value x4) > 200 SCOPE_ENTER value x4 > 212 LOAD_LOCAL(value x4) > 212 IS_INSTANCE REF(class MyException) -> 212 CZJUMP (BOOL)NE ? 5 : 11 -1430,1432d1522 +> 212 CZJUMP (BOOL)NE ? 5 : 8 +1410,1412d1502 < 200 JUMP 4 < < 4: -1446,1449d1535 +1422,1425d1511 < 212 LOAD_LOCAL(value x5) < 212 CALL_METHOD MyException.message (dynamic) < 212 STORE_LOCAL(value message) < 212 SCOPE_ENTER value message -1451c1537,1538 +1427c1513,1514 < 213 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 213 CALL_METHOD MyException.message (dynamic) -1495c1582 +1471c1558 < blocks: [1,2,3,4,5,7] --- > blocks: [1,2,3,4,5,7,8] -1519c1606,1607 +1495c1582,1583 < 58 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) > ? JUMP 8 -1520a1609,1614 +1496a1585,1590 > 8: > 62 LOAD_MODULE object Predef > 62 CONSTANT("RuntimeException") > 62 CALL_METHOD scala.Predef.println (dynamic) > 62 JUMP 2 > -1568c1662 +1544c1638 < blocks: [1,2,3,4] --- > blocks: [1,2,3,4,5] -1588c1682,1687 +1564c1658,1663 < 229 THROW(MyException) --- > ? JUMP 5 @@ -394,19 +394,19 @@ > ? LOAD_LOCAL(variable monitor1) > 228 MONITOR_EXIT > 228 THROW(Throwable) -1594c1693 +1570c1669 < ? THROW(Throwable) --- > 228 THROW(Throwable) -1622c1721 +1598c1697 < locals: value args, variable result, variable monitor2, variable monitorResult1 --- > locals: value exception$1, value args, variable result, variable monitor2, variable monitorResult1 -1624c1723 +1600c1699 < blocks: [1,2,3,4] --- > blocks: [1,2,3,4,5] -1647c1746,1754 +1623c1722,1730 < 245 THROW(MyException) --- > ? STORE_LOCAL(value exception$1) @@ -418,7 +418,7 @@ > ? LOAD_LOCAL(variable monitor2) > 244 MONITOR_EXIT > 244 THROW(Throwable) -1653c1760 +1629c1736 < ? THROW(Throwable) --- > 244 THROW(Throwable) -- cgit v1.2.3 From 494ba94518c9b40b7bf600ec7b70561842375597 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 29 Jan 2013 00:13:14 -0800 Subject: don't store subpats bound to underscore also, tweak fix in place for SI-5158 to appease SI-6941 don't store mutable fields from scala.* as we can assume these classes are well-behaved and do not mutate their case class fields --- .../tools/nsc/typechecker/PatternMatching.scala | 64 ++++++++++++++++++---- test/files/jvm/patmat_opt_ignore_underscore.check | 1 + test/files/jvm/patmat_opt_ignore_underscore.flags | 1 + .../patmat_opt_ignore_underscore/Analyzed_1.scala | 30 ++++++++++ .../jvm/patmat_opt_ignore_underscore/test.scala | 15 +++++ test/files/run/t6288.check | 10 +--- 6 files changed, 102 insertions(+), 19 deletions(-) create mode 100644 test/files/jvm/patmat_opt_ignore_underscore.check create mode 100644 test/files/jvm/patmat_opt_ignore_underscore.flags create mode 100644 test/files/jvm/patmat_opt_ignore_underscore/Analyzed_1.scala create mode 100644 test/files/jvm/patmat_opt_ignore_underscore/test.scala (limited to 'test/files/run') diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index f32ca4bd8e..8ae2e0a3b8 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -647,6 +647,11 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL case bp => bp } + // never store these in local variables (for PreserveSubPatBinders) + lazy val ignoredSubPatBinders = (subPatBinders zip args).collect{ + case (b, PatternBoundToUnderscore()) => b + }.toSet + def subPatTypes: List[Type] = if(isSeq) { val TypeRef(pre, SeqClass, args) = seqTp @@ -750,13 +755,16 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL def treeMaker(binder: Symbol, binderKnownNonNull: Boolean, pos: Position): TreeMaker = { val paramAccessors = binder.constrParamAccessors // binders corresponding to mutable fields should be stored (SI-5158, SI-6070) + // make an exception for classes under the scala package as they should be well-behaved, + // to optimize matching on List val mutableBinders = - if (paramAccessors exists (_.isMutable)) + if (!binder.info.typeSymbol.hasTransOwner(ScalaPackageClass) && + (paramAccessors exists (_.isMutable))) subPatBinders.zipWithIndex.collect{ case (binder, idx) if paramAccessors(idx).isMutable => binder } else Nil // checks binder ne null before chaining to the next extractor - ProductExtractorTreeMaker(binder, lengthGuard(binder))(subPatBinders, subPatRefs(binder), mutableBinders, binderKnownNonNull) + ProductExtractorTreeMaker(binder, lengthGuard(binder))(subPatBinders, subPatRefs(binder), mutableBinders, binderKnownNonNull, ignoredSubPatBinders) } // reference the (i-1)th case accessor if it exists, otherwise the (i-1)th tuple component @@ -792,7 +800,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // the extractor call (applied to the binder bound by the flatMap corresponding to the previous (i.e., enclosing/outer) pattern) val extractorApply = atPos(pos)(spliceApply(patBinderOrCasted)) val binder = freshSym(pos, pureType(resultInMonad)) // can't simplify this when subPatBinders.isEmpty, since UnitClass.tpe is definitely wrong when isSeq, and resultInMonad should always be correct since it comes directly from the extractor's result type - ExtractorTreeMaker(extractorApply, lengthGuard(binder), binder)(subPatBinders, subPatRefs(binder), resultType.typeSymbol == BooleanClass, checkedLength, patBinderOrCasted) + ExtractorTreeMaker(extractorApply, lengthGuard(binder), binder)(subPatBinders, subPatRefs(binder), resultType.typeSymbol == BooleanClass, checkedLength, patBinderOrCasted, ignoredSubPatBinders) } override protected def seqTree(binder: Symbol): Tree = @@ -849,6 +857,16 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL } } + object PatternBoundToUnderscore { + def unapply(pat: Tree): Boolean = pat match { + case Bind(nme.WILDCARD, _) => true // don't skip when binding an interesting symbol! + case Ident(nme.WILDCARD) => true + case Alternative(ps) => ps forall (PatternBoundToUnderscore.unapply(_)) + case Typed(PatternBoundToUnderscore(), _) => true + case _ => false + } + } + object Bound { def unapply(t: Tree): Option[(Symbol, Tree)] = t match { case t@Bind(n, p) if (t.symbol ne null) && (t.symbol ne NoSymbol) => // pos/t2429 does not satisfy these conditions @@ -1016,10 +1034,17 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL trait PreserveSubPatBinders extends TreeMaker { val subPatBinders: List[Symbol] val subPatRefs: List[Tree] + val ignoredSubPatBinders: Set[Symbol] // unless `debugInfoEmitVars`, this set should contain the bare minimum for correctness // mutable case class fields need to be stored regardless (SI-5158, SI-6070) -- see override in ProductExtractorTreeMaker - def storedBinders: Set[Symbol] = if (debugInfoEmitVars) subPatBinders.toSet else Set.empty + // sub patterns bound to wildcard (_) are never stored as they can't be referenced + // dirty debuggers will have to get dirty to see the wildcards + lazy val storedBinders: Set[Symbol] = + (if (debugInfoEmitVars) subPatBinders.toSet else Set.empty) ++ extraStoredBinders -- ignoredSubPatBinders + + // e.g., mutable fields of a case class in ProductExtractorTreeMaker + def extraStoredBinders: Set[Symbol] def emitVars = storedBinders.nonEmpty @@ -1040,10 +1065,22 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL Substitution(subPatBinders, subPatRefs) >> super.subPatternsAsSubstitution import CODE._ - def bindSubPats(in: Tree): Tree = if (!emitVars) in + def bindSubPats(in: Tree): Tree = + if (!emitVars) in else { - val (subPatBindersStored, subPatRefsStored) = stored.unzip - Block(map2(subPatBindersStored.toList, subPatRefsStored.toList)(VAL(_) === _), in) + // binders in `subPatBindersStored` that are referenced by tree `in` + val usedBinders = new collection.mutable.HashSet[Symbol]() + // all potentially stored subpat binders + val potentiallyStoredBinders = stored.unzip._1.toSet + // compute intersection of all symbols in the tree `in` and all potentially stored subpat binders + in.foreach(t => if (potentiallyStoredBinders(t.symbol)) usedBinders += t.symbol) + + if (usedBinders.isEmpty) in + else { + // only store binders actually used + val (subPatBindersStored, subPatRefsStored) = stored.filter{case (b, _) => usedBinders(b)}.unzip + Block(map2(subPatBindersStored.toList, subPatRefsStored.toList)(VAL(_) === _), in) + } } } @@ -1063,7 +1100,11 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL val subPatRefs: List[Tree], extractorReturnsBoolean: Boolean, val checkedLength: Option[Int], - val prevBinder: Symbol) extends FunTreeMaker with PreserveSubPatBinders { + val prevBinder: Symbol, + val ignoredSubPatBinders: Set[Symbol] + ) extends FunTreeMaker with PreserveSubPatBinders { + + def extraStoredBinders: Set[Symbol] = Set() def chainBefore(next: Tree)(casegen: Casegen): Tree = { val condAndNext = extraCond match { @@ -1107,14 +1148,15 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL val subPatBinders: List[Symbol], val subPatRefs: List[Tree], val mutableBinders: List[Symbol], - binderKnownNonNull: Boolean) extends FunTreeMaker with PreserveSubPatBinders { + binderKnownNonNull: Boolean, + val ignoredSubPatBinders: Set[Symbol] + ) extends FunTreeMaker with PreserveSubPatBinders { import CODE._ val nextBinder = prevBinder // just passing through // mutable binders must be stored to avoid unsoundness or seeing mutation of fields after matching (SI-5158, SI-6070) - // (the implementation could be optimized by duplicating code from `super.storedBinders`, but this seems more elegant) - override def storedBinders: Set[Symbol] = super.storedBinders ++ mutableBinders.toSet + def extraStoredBinders: Set[Symbol] = mutableBinders.toSet def chainBefore(next: Tree)(casegen: Casegen): Tree = { val nullCheck = REF(prevBinder) OBJ_NE NULL diff --git a/test/files/jvm/patmat_opt_ignore_underscore.check b/test/files/jvm/patmat_opt_ignore_underscore.check new file mode 100644 index 0000000000..43f53aba12 --- /dev/null +++ b/test/files/jvm/patmat_opt_ignore_underscore.check @@ -0,0 +1 @@ +bytecode identical diff --git a/test/files/jvm/patmat_opt_ignore_underscore.flags b/test/files/jvm/patmat_opt_ignore_underscore.flags new file mode 100644 index 0000000000..1182725e86 --- /dev/null +++ b/test/files/jvm/patmat_opt_ignore_underscore.flags @@ -0,0 +1 @@ +-optimize \ No newline at end of file diff --git a/test/files/jvm/patmat_opt_ignore_underscore/Analyzed_1.scala b/test/files/jvm/patmat_opt_ignore_underscore/Analyzed_1.scala new file mode 100644 index 0000000000..ac59e41ba7 --- /dev/null +++ b/test/files/jvm/patmat_opt_ignore_underscore/Analyzed_1.scala @@ -0,0 +1,30 @@ +// this class's bytecode, compiled under -optimize is analyzed by the test +// method a's bytecode should be identical to method b's bytecode +// this is not the best test for shielding against regressing on this particular issue, +// but it sets the stage for checking the bytecode emitted by the pattern matcher and +// comparing it to manually tuned code using if/then/else etc. +class SameBytecode { + case class Foo(x: Any, y: String) + + def a = + Foo(1, "a") match { + case Foo(_: String, 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, isInstanceOf 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)) { + if (x1.x.isInstanceOf[String]) { + val x3 = x1.x.asInstanceOf[String] + return x1.y + } + } + + throw new MatchError(x1) + } +} \ No newline at end of file diff --git a/test/files/jvm/patmat_opt_ignore_underscore/test.scala b/test/files/jvm/patmat_opt_ignore_underscore/test.scala new file mode 100644 index 0000000000..6179101a7e --- /dev/null +++ b/test/files/jvm/patmat_opt_ignore_underscore/test.scala @@ -0,0 +1,15 @@ +import scala.tools.partest.BytecodeTest + +import scala.tools.nsc.util.JavaClassPath +import java.io.InputStream +import scala.tools.asm +import asm.ClassReader +import asm.tree.{ClassNode, InsnList} +import scala.collection.JavaConverters._ + +object Test extends BytecodeTest { + def show: Unit = { + val classNode = loadClassNode("SameBytecode") + sameBytecode(getMethod(classNode, "a"), getMethod(classNode, "b")) + } +} diff --git a/test/files/run/t6288.check b/test/files/run/t6288.check index af6bd5d269..4895c2c007 100644 --- a/test/files/run/t6288.check +++ b/test/files/run/t6288.check @@ -11,10 +11,7 @@ [64]case5()[84]{ [84] val o7: [84]Option[Int] = [84][84]Case3.unapply([84]x1); [84]if ([84]o7.isEmpty.unary_!) - [84]{ - [90]val nr: [90]Int = [90]o7.get; - [97][97]matchEnd4([97]()) - } + [97][97]matchEnd4([97]()) else [84][84]case6() }; @@ -38,10 +35,7 @@ [195] val o7: [195]Option[List[Int]] = [195][195]Case4.unapplySeq([195]x1); [195]if ([195]o7.isEmpty.unary_!) [195]if ([195][195][195][195]o7.get.!=([195]null).&&([195][195][195][195]o7.get.lengthCompare([195]1).==([195]0))) - [195]{ - [201]val nr: [201]Int = [201][201]o7.get.apply([201]0); - [208][208]matchEnd4([208]()) - } + [208][208]matchEnd4([208]()) else [195][195]case6() else -- cgit v1.2.3 From b92396b57912f040d8f536b8a60b844ff586ff0d Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Tue, 29 Jan 2013 19:16:37 -0800 Subject: SI-6686 drop valdef unused in flatMapCond's block --- .../tools/nsc/typechecker/PatternMatching.scala | 16 ++- .../patmat_opt_ignore_underscore/Analyzed_1.scala | 1 - .../jvm/patmat_opt_no_nullcheck/Analyzed_1.scala | 1 - .../patmat_opt_primitive_typetest/Analyzed_1.scala | 1 - test/files/run/inline-ex-handlers.check | 136 ++++++++++----------- test/files/run/t6288b-jump-position.check | 6 +- 6 files changed, 80 insertions(+), 81 deletions(-) (limited to 'test/files/run') diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index c6f80c9b20..4b53802d95 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -3792,11 +3792,17 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL // nextBinder: T // next == MatchMonad[U] // returns MatchMonad[U] - def flatMapCond(cond: Tree, res: Tree, nextBinder: Symbol, next: Tree): Tree = - ifThenElseZero(cond, BLOCK( - VAL(nextBinder) === res, - next - )) + def flatMapCond(cond: Tree, res: Tree, nextBinder: Symbol, next: Tree): Tree = { + val rest = + // only emit a local val for `nextBinder` if it's actually referenced in `next` + if (next.exists(_.symbol eq nextBinder)) + BLOCK( + VAL(nextBinder) === res, + next + ) + else next + ifThenElseZero(cond, rest) + } // guardTree: Boolean // next: MatchMonad[T] diff --git a/test/files/jvm/patmat_opt_ignore_underscore/Analyzed_1.scala b/test/files/jvm/patmat_opt_ignore_underscore/Analyzed_1.scala index ac59e41ba7..fa3639380d 100644 --- a/test/files/jvm/patmat_opt_ignore_underscore/Analyzed_1.scala +++ b/test/files/jvm/patmat_opt_ignore_underscore/Analyzed_1.scala @@ -20,7 +20,6 @@ class SameBytecode { val x1 = Foo(1, "a") if (x1.ne(null)) { if (x1.x.isInstanceOf[String]) { - val x3 = x1.x.asInstanceOf[String] return x1.y } } diff --git a/test/files/jvm/patmat_opt_no_nullcheck/Analyzed_1.scala b/test/files/jvm/patmat_opt_no_nullcheck/Analyzed_1.scala index 1594eb523c..3a594c401e 100644 --- a/test/files/jvm/patmat_opt_no_nullcheck/Analyzed_1.scala +++ b/test/files/jvm/patmat_opt_no_nullcheck/Analyzed_1.scala @@ -14,7 +14,6 @@ class SameBytecode { if (x1.isInstanceOf[Foo]) { val x3 = x1.asInstanceOf[Foo] if (x3.x.isInstanceOf[String]) { - val x4 = x3.x.asInstanceOf[String] val x = () return } diff --git a/test/files/jvm/patmat_opt_primitive_typetest/Analyzed_1.scala b/test/files/jvm/patmat_opt_primitive_typetest/Analyzed_1.scala index c5b8811449..e5db6c4dd0 100644 --- a/test/files/jvm/patmat_opt_primitive_typetest/Analyzed_1.scala +++ b/test/files/jvm/patmat_opt_primitive_typetest/Analyzed_1.scala @@ -16,7 +16,6 @@ class SameBytecode { def b: String = { val x1 = Foo(1, "a") if (x1.ne(null)) { - val x3 = x1.x return x1.y } diff --git a/test/files/run/inline-ex-handlers.check b/test/files/run/inline-ex-handlers.check index 905dfc3ee7..f2f0b60687 100644 --- a/test/files/run/inline-ex-handlers.check +++ b/test/files/run/inline-ex-handlers.check @@ -21,15 +21,15 @@ < 92 JUMP 7 < < 7: -395c391 +391c387 < locals: value args, variable result, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value ex6, value x4, value x5, value x -397c393 +393c389 < blocks: [1,2,3,4,5,8,10,11,13] --- > blocks: [1,2,3,5,8,10,11,13,14] -421c417,426 +417c413,422 < 103 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -42,39 +42,39 @@ > 106 LOAD_LOCAL(value x4) > 106 IS_INSTANCE REF(class MyException) > 106 CZJUMP (BOOL)NE ? 5 : 8 -434,436d438 +430,432d434 < 101 JUMP 4 < < 4: -446,449d447 +442,445d443 < 106 LOAD_LOCAL(value x5) < 106 CALL_METHOD MyException.message (dynamic) < 106 STORE_LOCAL(value message) < 106 SCOPE_ENTER value message -451c449,450 +447c445,446 < 106 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 106 CALL_METHOD MyException.message (dynamic) -523c522 +519c518 < blocks: [1,2,3,4,6,7,8,9,10] --- > blocks: [1,2,3,4,6,7,8,9,10,11,12,13] -552c551 +548c547 < 306 THROW(MyException) --- > ? JUMP 11 -553a553,557 +549a549,553 > 11: > ? LOAD_LOCAL(variable monitor4) > 305 MONITOR_EXIT > ? JUMP 12 > -558c562 +554c558 < ? THROW(Throwable) --- > ? JUMP 12 -564c568,575 +560c564,571 < ? THROW(Throwable) --- > ? STORE_LOCAL(value t) @@ -85,7 +85,7 @@ > 304 MONITOR_EXIT > ? STORE_LOCAL(value t) > ? JUMP 13 -579a591,602 +575a587,598 > 13: > 310 LOAD_MODULE object Predef > 310 CALL_PRIMITIVE(StartConcat) @@ -98,35 +98,35 @@ > 310 CALL_METHOD scala.Predef.println (dynamic) > 310 JUMP 2 > -588c611 +584c607 < catch (Throwable) in ArrayBuffer(7, 8, 9, 10) starting at: 6 --- > catch (Throwable) in ArrayBuffer(7, 8, 9, 10, 11) starting at: 6 -591c614 +587c610 < catch (Throwable) in ArrayBuffer(4, 6, 7, 8, 9, 10) starting at: 3 --- > catch (Throwable) in ArrayBuffer(4, 6, 7, 8, 9, 10, 11, 12) starting at: 3 -623c646 +619c642 < blocks: [1,2,3,4,5,6,7,9,10] --- > blocks: [1,2,3,4,5,6,7,9,10,11,12] -647c670,671 +643c666,667 < 78 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) > ? JUMP 11 -648a673,677 +644a669,673 > 11: > 81 LOAD_LOCAL(value e) > ? STORE_LOCAL(variable exc1) > ? JUMP 12 > -676c705,706 +672c701,702 < 81 THROW(Exception) --- > ? STORE_LOCAL(variable exc1) > ? JUMP 12 -692a723,735 +688a719,731 > 12: > 83 LOAD_MODULE object Predef > 83 CONSTANT("finally") @@ -140,19 +140,19 @@ > 84 LOAD_LOCAL(variable exc1) > 84 THROW(Throwable) > -698c741 +694c737 < catch () in ArrayBuffer(4, 6, 7, 9) starting at: 3 --- > catch () in ArrayBuffer(4, 6, 7, 9, 11) starting at: 3 -722c765 +718c761 < locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value message, value x, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value ex6, variable exc2, value x4, value x5, value x, value ex6, value x4, value x5, value x -724c767 +720c763 < blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25] --- > blocks: [1,2,3,4,5,6,9,11,14,15,16,19,21,22,24,25,26,27,28] -748c791,798 +744c787,794 < 172 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -163,64 +163,64 @@ > 170 STORE_LOCAL(value x4) > 170 SCOPE_ENTER value x4 > 170 JUMP 15 -791,794d840 +787,790d836 < 175 LOAD_LOCAL(value x5) < 175 CALL_METHOD MyException.message (dynamic) < 175 STORE_LOCAL(value message) < 175 SCOPE_ENTER value message -796c842,843 +792c838,839 < 176 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 176 CALL_METHOD MyException.message (dynamic) -800c847,848 +796c843,844 < 177 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 177 CALL_METHOD MyException.message (dynamic) -802c850,851 +798c846,847 < 177 THROW(MyException) --- > ? STORE_LOCAL(value ex6) > ? JUMP 27 -806c855,856 +802c851,852 < 170 THROW(Throwable) --- > ? STORE_LOCAL(value ex6) > ? JUMP 27 -815a866,871 +811a862,867 > 27: > 169 LOAD_LOCAL(value ex6) > 169 STORE_LOCAL(value x4) > 169 SCOPE_ENTER value x4 > 169 JUMP 5 > -826,829d881 +822,825d877 < 180 LOAD_LOCAL(value x5) < 180 CALL_METHOD MyException.message (dynamic) < 180 STORE_LOCAL(value message) < 180 SCOPE_ENTER value message -831c883,884 +827c879,880 < 181 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 181 CALL_METHOD MyException.message (dynamic) -835c888,889 +831c884,885 < 182 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 182 CALL_METHOD MyException.message (dynamic) -837c891,892 +833c887,888 < 182 THROW(MyException) --- > ? STORE_LOCAL(variable exc2) > ? JUMP 28 -841c896,897 +837c892,893 < 169 THROW(Throwable) --- > ? STORE_LOCAL(variable exc2) > ? JUMP 28 -857a914,926 +853a910,922 > 28: > 184 LOAD_MODULE object Predef > 184 CONSTANT("finally") @@ -234,23 +234,23 @@ > 185 LOAD_LOCAL(variable exc2) > 185 THROW(Throwable) > -863c932 +859c928 < catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24) starting at: 4 --- > catch (Throwable) in ArrayBuffer(14, 15, 16, 19, 21, 22, 24, 26) starting at: 4 -866c935 +862c931 < catch () in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24) starting at: 3 --- > catch () in ArrayBuffer(4, 5, 6, 9, 14, 15, 16, 19, 21, 22, 24, 26, 27) starting at: 3 -890c959 +886c955 < locals: value args, variable result, value e, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value e, value ex6, value x4, value x5, value x -892c961 +888c957 < blocks: [1,2,3,6,7,8,11,13,14,16] --- > blocks: [1,2,3,6,7,8,11,13,14,16,17] -916c985,992 +912c981,988 < 124 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -261,29 +261,29 @@ > 122 STORE_LOCAL(value x4) > 122 SCOPE_ENTER value x4 > 122 JUMP 7 -941,944d1016 +937,940d1012 < 127 LOAD_LOCAL(value x5) < 127 CALL_METHOD MyException.message (dynamic) < 127 STORE_LOCAL(value message) < 127 SCOPE_ENTER value message -946c1018,1019 +942c1014,1015 < 127 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 127 CALL_METHOD MyException.message (dynamic) -975c1048 +971c1044 < catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16) starting at: 3 --- > catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 13, 14, 16, 17) starting at: 3 -999c1072 +995c1068 < locals: value args, variable result, value ex6, value x4, value x5, value message, value x, value e --- > locals: value args, variable result, value ex6, value x4, value x5, value x, value e -1001c1074 +997c1070 < blocks: [1,2,3,4,5,8,12,13,14,16] --- > blocks: [1,2,3,5,8,12,13,14,16,17] -1025c1098,1107 +1021c1094,1103 < 148 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -296,25 +296,25 @@ > 154 LOAD_LOCAL(value x4) > 154 IS_INSTANCE REF(class MyException) > 154 CZJUMP (BOOL)NE ? 5 : 8 -1046,1048d1127 +1042,1044d1123 < 145 JUMP 4 < < 4: -1058,1061d1136 +1054,1057d1132 < 154 LOAD_LOCAL(value x5) < 154 CALL_METHOD MyException.message (dynamic) < 154 STORE_LOCAL(value message) < 154 SCOPE_ENTER value message -1063c1138,1139 +1059c1134,1135 < 154 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 154 CALL_METHOD MyException.message (dynamic) -1280c1356 +1276c1352 < blocks: [1,2,3,4,5,7] --- > blocks: [1,2,3,4,5,7,8] -1304c1380,1387 +1300c1376,1383 < 38 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) @@ -325,20 +325,20 @@ > 42 CONSTANT("IllegalArgumentException") > 42 CALL_METHOD scala.Predef.println (dynamic) > 42 JUMP 2 -1351c1434 +1347c1430 < locals: value args, variable result, value ex6, value x4, value x5, value message, value x --- > locals: value args, variable result, value ex6, value x4, value x5, value x -1353c1436 +1349c1432 < blocks: [1,2,3,4,5,8,10,11,13,14,16] --- > blocks: [1,2,3,5,8,10,11,13,14,16,17] -1377c1460,1461 +1373c1456,1457 < 203 THROW(MyException) --- > ? STORE_LOCAL(value ex6) > ? JUMP 17 -1397c1481,1490 +1393c1477,1486 < 209 THROW(MyException) --- > ? STORE_LOCAL(value ex6) @@ -351,41 +351,41 @@ > 212 LOAD_LOCAL(value x4) > 212 IS_INSTANCE REF(class MyException) > 212 CZJUMP (BOOL)NE ? 5 : 8 -1410,1412d1502 +1406,1408d1498 < 200 JUMP 4 < < 4: -1422,1425d1511 +1418,1421d1507 < 212 LOAD_LOCAL(value x5) < 212 CALL_METHOD MyException.message (dynamic) < 212 STORE_LOCAL(value message) < 212 SCOPE_ENTER value message -1427c1513,1514 +1423c1509,1510 < 213 LOAD_LOCAL(value message) --- > ? LOAD_LOCAL(value x5) > 213 CALL_METHOD MyException.message (dynamic) -1471c1558 +1467c1554 < blocks: [1,2,3,4,5,7] --- > blocks: [1,2,3,4,5,7,8] -1495c1582,1583 +1491c1578,1579 < 58 THROW(IllegalArgumentException) --- > ? STORE_LOCAL(value e) > ? JUMP 8 -1496a1585,1590 +1492a1581,1586 > 8: > 62 LOAD_MODULE object Predef > 62 CONSTANT("RuntimeException") > 62 CALL_METHOD scala.Predef.println (dynamic) > 62 JUMP 2 > -1544c1638 +1540c1634 < blocks: [1,2,3,4] --- > blocks: [1,2,3,4,5] -1564c1658,1663 +1560c1654,1659 < 229 THROW(MyException) --- > ? JUMP 5 @@ -394,19 +394,19 @@ > ? LOAD_LOCAL(variable monitor1) > 228 MONITOR_EXIT > 228 THROW(Throwable) -1570c1669 +1566c1665 < ? THROW(Throwable) --- > 228 THROW(Throwable) -1598c1697 +1594c1693 < locals: value args, variable result, variable monitor2, variable monitorResult1 --- > locals: value exception$1, value args, variable result, variable monitor2, variable monitorResult1 -1600c1699 +1596c1695 < blocks: [1,2,3,4] --- > blocks: [1,2,3,4,5] -1623c1722,1730 +1619c1718,1726 < 245 THROW(MyException) --- > ? STORE_LOCAL(value exception$1) @@ -418,7 +418,7 @@ > ? LOAD_LOCAL(variable monitor2) > 244 MONITOR_EXIT > 244 THROW(Throwable) -1629c1736 +1625c1732 < ? THROW(Throwable) --- > 244 THROW(Throwable) diff --git a/test/files/run/t6288b-jump-position.check b/test/files/run/t6288b-jump-position.check index 45ec31c308..ece88b18f0 100644 --- a/test/files/run/t6288b-jump-position.check +++ b/test/files/run/t6288b-jump-position.check @@ -19,7 +19,7 @@ object Case3 extends Object { Exception handlers: def main(args: Array[String] (ARRAY[REF(class String)])): Unit { - locals: value args, value x1, value x2, value x + locals: value args, value x1, value x startBlock: 1 blocks: [1,2,3,6,7] @@ -35,10 +35,6 @@ object Case3 extends Object { 5 CZJUMP (BOOL)NE ? 3 : 6 3: - 5 LOAD_LOCAL(value x1) - 5 CHECK_CAST REF(class String) - 5 STORE_LOCAL(value x2) - 5 SCOPE_ENTER value x2 6 LOAD_MODULE object Predef 6 CONSTANT("case 0") 6 CALL_METHOD scala.Predef.println (dynamic) -- cgit v1.2.3 From f1701f704a2485fcc2eb6d5d8b5d0228beddd9b3 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Thu, 31 Jan 2013 20:40:16 +0100 Subject: SI-7008 @throws annotations are now populated in reflect Runtime reflection in JavaMirrors previously forgot to fill in @throws when importing Java reflection artifacts. Now this is fixed. Note that generic exception types used in `throws` specifications will be garbled (i.e. erased), because we don't use `getGenericExceptionTypes` in favor of just `getExceptionTypes` to stay compatible with the behavior of ClassfileParser. That's a bug, but a separate one and should be fixed separately. Also note that this commit updated javac-artifacts.jar, because we need to test how reflection works with javac-produced classfiles. The sources that were used to produce those classfiles can be found in the jar next to the classfiles. --- .../tools/nsc/symtab/classfile/ClassfileParser.scala | 8 +------- .../scala/reflect/internal/AnnotationInfos.scala | 17 ++++++++++++++--- src/reflect/scala/reflect/runtime/JavaMirrors.scala | 8 ++++++++ test/files/lib/javac-artifacts.jar.desired.sha1 | 2 +- test/files/run/t7008.check | 9 +++++++++ test/files/run/t7008/Impls_Macros_1.scala | 12 ++++++++++++ test/files/run/t7008/Test_2.scala | 9 +++++++++ 7 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 test/files/run/t7008.check create mode 100644 test/files/run/t7008/Impls_Macros_1.scala create mode 100644 test/files/run/t7008/Test_2.scala (limited to 'test/files/run') diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala index 4b1d3c34f3..7ac7b4d637 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala @@ -1043,13 +1043,7 @@ abstract class ClassfileParser { val nClasses = in.nextChar for (n <- 0 until nClasses) { val cls = pool.getClassSymbol(in.nextChar.toInt) - val tp = if (cls.isMonomorphicType) cls.tpe else { - debuglog(s"Encountered polymorphic exception `${cls.fullName}` while parsing class file.") - // in case we encounter polymorphic exception the best we can do is to convert that type to - // monomorphic one by introducing existientals, see SI-7009 for details - typer.packSymbols(cls.typeParams, cls.tpe) - } - sym.addAnnotation(appliedType(definitions.ThrowsClass, tp), Literal(Constant(tp))) + sym.addThrowsAnnotation(cls) } } diff --git a/src/reflect/scala/reflect/internal/AnnotationInfos.scala b/src/reflect/scala/reflect/internal/AnnotationInfos.scala index 6a5a742cc7..29879f9806 100644 --- a/src/reflect/scala/reflect/internal/AnnotationInfos.scala +++ b/src/reflect/scala/reflect/internal/AnnotationInfos.scala @@ -33,6 +33,17 @@ trait AnnotationInfos extends api.Annotations { self: SymbolTable => case ThrownException(exc) => exc } + def addThrowsAnnotation(excSym: Symbol): Self = { + val excTpe = if (excSym.isMonomorphicType) excSym.tpe else { + debuglog(s"Encountered polymorphic exception `${excSym.fullName}` while parsing class file.") + // in case we encounter polymorphic exception the best we can do is to convert that type to + // monomorphic one by introducing existentials, see SI-7009 for details + existentialAbstraction(excSym.typeParams, excSym.tpe) + } + val throwsAnn = AnnotationInfo(appliedType(definitions.ThrowsClass, excTpe), List(Literal(Constant(excTpe))), Nil) + withAnnotations(List(throwsAnn)) + } + /** Tests for, get, or remove an annotation */ def hasAnnotation(cls: Symbol): Boolean = //OPT inlined from exists to save on #closures; was: annotations exists (_ matches cls) @@ -330,14 +341,14 @@ trait AnnotationInfos extends api.Annotations { self: SymbolTable => implicit val AnnotationTag = ClassTag[AnnotationInfo](classOf[AnnotationInfo]) object UnmappableAnnotation extends CompleteAnnotationInfo(NoType, Nil, Nil) - + /** Extracts symbol of thrown exception from AnnotationInfo. - * + * * Supports both “old-style” `@throws(classOf[Exception])` * as well as “new-stye” `@throws[Exception]("cause")` annotations. */ object ThrownException { - def unapply(ann: AnnotationInfo): Option[Symbol] = + def unapply(ann: AnnotationInfo): Option[Symbol] = ann match { case AnnotationInfo(tpe, _, _) if tpe.typeSymbol != ThrowsClass => None diff --git a/src/reflect/scala/reflect/runtime/JavaMirrors.scala b/src/reflect/scala/reflect/runtime/JavaMirrors.scala index 01e0634902..698b60b929 100644 --- a/src/reflect/scala/reflect/runtime/JavaMirrors.scala +++ b/src/reflect/scala/reflect/runtime/JavaMirrors.scala @@ -610,11 +610,19 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni /** * Copy all annotations of Java annotated element `jann` over to Scala symbol `sym`. + * Also creates `@throws` annotations if necessary. * Pre: `sym` is already initialized with a concrete type. * Note: If `sym` is a method or constructor, its parameter annotations are copied as well. */ private def copyAnnotations(sym: Symbol, jann: AnnotatedElement) { sym setAnnotations (jann.getAnnotations map JavaAnnotationProxy).toList + // FIXME: we're not using getGenericExceptionTypes here to be consistent with ClassfileParser + val jexTpes = jann match { + case jm: jMethod => jm.getExceptionTypes.toList + case jconstr: jConstructor[_] => jconstr.getExceptionTypes.toList + case _ => Nil + } + jexTpes foreach (jexTpe => sym.addThrowsAnnotation(classSymbol(jexTpe))) } /** diff --git a/test/files/lib/javac-artifacts.jar.desired.sha1 b/test/files/lib/javac-artifacts.jar.desired.sha1 index a49c986386..508141b771 100644 --- a/test/files/lib/javac-artifacts.jar.desired.sha1 +++ b/test/files/lib/javac-artifacts.jar.desired.sha1 @@ -1 +1 @@ -61931a51bb1a2d308d214b96d06e9a8808515dcf ?javac-artifacts.jar +a05ca69ac68fa7006b1b8ed98597046e105b4047 ?javac-artifacts.jar diff --git a/test/files/run/t7008.check b/test/files/run/t7008.check new file mode 100644 index 0000000000..ee077f90ff --- /dev/null +++ b/test/files/run/t7008.check @@ -0,0 +1,9 @@ +: List(throws[NullPointerException](classOf[java.lang.NullPointerException])) +bar: List(throws[Exception](classOf[java.lang.Exception])) +baz: List(throws[IllegalStateException](classOf[java.lang.IllegalStateException])) +foo: List(throws[Exception](classOf[java.lang.Exception])) +============= +: List(throws[java.lang.NullPointerException](classOf[java.lang.NullPointerException])) +bar: List(throws[java.lang.Exception](classOf[java.lang.Exception])) +baz: List(throws[java.lang.IllegalStateException](classOf[java.lang.IllegalStateException])) +foo: List(throws[java.lang.Exception](classOf[java.lang.Exception])) diff --git a/test/files/run/t7008/Impls_Macros_1.scala b/test/files/run/t7008/Impls_Macros_1.scala new file mode 100644 index 0000000000..f2eb7425f5 --- /dev/null +++ b/test/files/run/t7008/Impls_Macros_1.scala @@ -0,0 +1,12 @@ +import language.experimental.macros +import scala.reflect.macros.Context + +object Macros { + def impl(c: Context) = { + val decls = c.typeOf[JavaClassWithCheckedExceptions[_]].declarations.toList + val s = decls.sortBy(_.name.toString).map(decl => (s"${decl.name}: ${decl.annotations}")).mkString(scala.compat.Platform.EOL) + c.universe.reify(println(c.literal(s).splice)) + } + + def foo = macro impl +} \ No newline at end of file diff --git a/test/files/run/t7008/Test_2.scala b/test/files/run/t7008/Test_2.scala new file mode 100644 index 0000000000..b67faa327f --- /dev/null +++ b/test/files/run/t7008/Test_2.scala @@ -0,0 +1,9 @@ +import scala.reflect.runtime.universe._ + +object Test extends App { + Macros.foo + println("=============") + + val decls = typeOf[JavaClassWithCheckedExceptions[_]].declarations.toList + decls sortBy (_.name.toString) foreach (decl => println(s"${decl.name}: ${decl.annotations}")) +} \ No newline at end of file -- cgit v1.2.3 From adf50a3ac0c6861a77a781abc0814c5d17927175 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Fri, 1 Feb 2013 19:23:00 +0100 Subject: evicts javac-artifacts.jar Apparently, the usual _1, _2, _3... naming scheme also works for java files, which need to be compiled together with partests. This allows us to get rid of javac-artifacts.jar. --- test/files/lib/javac-artifacts.jar.desired.sha1 | 1 - test/files/run/reflection-java-annotations.check | 2 +- test/files/run/reflection-java-annotations.scala | 7 ---- .../JavaAnnottee_1.java | 47 ++++++++++++++++++++++ .../JavaComplexAnnotation_1.java | 34 ++++++++++++++++ .../JavaSimpleAnnotation_1.java | 21 ++++++++++ .../JavaSimpleEnumeration_1.java | 4 ++ .../run/reflection-java-annotations/Test_2.scala | 7 ++++ test/files/run/reflection-java-crtp.scala | 8 ---- .../JavaSimpleEnumeration_1.java | 4 ++ test/files/run/reflection-java-crtp/Main_2.scala | 8 ++++ test/files/run/t6548.check | 2 +- test/files/run/t6548.scala | 12 ------ .../run/t6548/JavaAnnotationWithNestedEnum_1.java | 17 ++++++++ test/files/run/t6548/Test_2.scala | 12 ++++++ test/files/run/t7008/Impls_Macros_1.scala | 12 ------ test/files/run/t7008/Impls_Macros_2.scala | 12 ++++++ .../t7008/JavaClassWithCheckedExceptions_1.java | 7 ++++ test/files/run/t7008/Test_2.scala | 9 ----- test/files/run/t7008/Test_3.scala | 9 +++++ 20 files changed, 184 insertions(+), 51 deletions(-) delete mode 100644 test/files/lib/javac-artifacts.jar.desired.sha1 delete mode 100644 test/files/run/reflection-java-annotations.scala create mode 100644 test/files/run/reflection-java-annotations/JavaAnnottee_1.java create mode 100644 test/files/run/reflection-java-annotations/JavaComplexAnnotation_1.java create mode 100644 test/files/run/reflection-java-annotations/JavaSimpleAnnotation_1.java create mode 100644 test/files/run/reflection-java-annotations/JavaSimpleEnumeration_1.java create mode 100644 test/files/run/reflection-java-annotations/Test_2.scala delete mode 100644 test/files/run/reflection-java-crtp.scala create mode 100644 test/files/run/reflection-java-crtp/JavaSimpleEnumeration_1.java create mode 100644 test/files/run/reflection-java-crtp/Main_2.scala delete mode 100644 test/files/run/t6548.scala create mode 100644 test/files/run/t6548/JavaAnnotationWithNestedEnum_1.java create mode 100644 test/files/run/t6548/Test_2.scala delete mode 100644 test/files/run/t7008/Impls_Macros_1.scala create mode 100644 test/files/run/t7008/Impls_Macros_2.scala create mode 100644 test/files/run/t7008/JavaClassWithCheckedExceptions_1.java delete mode 100644 test/files/run/t7008/Test_2.scala create mode 100644 test/files/run/t7008/Test_3.scala (limited to 'test/files/run') diff --git a/test/files/lib/javac-artifacts.jar.desired.sha1 b/test/files/lib/javac-artifacts.jar.desired.sha1 deleted file mode 100644 index 508141b771..0000000000 --- a/test/files/lib/javac-artifacts.jar.desired.sha1 +++ /dev/null @@ -1 +0,0 @@ -a05ca69ac68fa7006b1b8ed98597046e105b4047 ?javac-artifacts.jar diff --git a/test/files/run/reflection-java-annotations.check b/test/files/run/reflection-java-annotations.check index 53c53cfbcc..2d37fff1f4 100644 --- a/test/files/run/reflection-java-annotations.check +++ b/test/files/run/reflection-java-annotations.check @@ -1 +1 @@ -List(JavaComplexAnnotation(v1 = 1, v10 = "hello", v101 = [101, 101], v102 = [102, 102], v103 = ['g', 'g'], v104 = [104, 104], v105 = [105L, 105L], v106 = [106.0, 106.0], v107 = [107.0, 107.0], v108 = [false, true], v11 = classOf[JavaAnnottee], v110 = ["hello", "world"], v111 = [classOf[JavaSimpleAnnotation], classOf[JavaComplexAnnotation]], v112 = [FOO, BAR], v113 = [JavaSimpleAnnotation(v1 = 21, v10 = "world2", v11 = classOf[JavaComplexAnnotation], v12 = BAR, v2 = 22, v3 = '\027', v4 = 24, v5 = 25L, v6 = 26.0, v7 = 27.0, v8 = false)], v12 = FOO, v13 = JavaSimpleAnnotation(v1 = 11, v10 = "world1", v11 = classOf[JavaSimpleAnnotation], v12 = FOO, v2 = 12, v3 = '\r', v4 = 14, v5 = 15L, v6 = 16.0, v7 = 17.0, v8 = false), v2 = 2, v3 = '\03', v4 = 4, v5 = 5L, v6 = 6.0, v7 = 7.0, v8 = false)) +List(JavaComplexAnnotation_1(v1 = 1, v10 = "hello", v101 = [101, 101], v102 = [102, 102], v103 = ['g', 'g'], v104 = [104, 104], v105 = [105L, 105L], v106 = [106.0, 106.0], v107 = [107.0, 107.0], v108 = [false, true], v11 = classOf[JavaAnnottee_1], v110 = ["hello", "world"], v111 = [classOf[JavaSimpleAnnotation_1], classOf[JavaComplexAnnotation_1]], v112 = [FOO, BAR], v113 = [JavaSimpleAnnotation_1(v1 = 21, v10 = "world2", v11 = classOf[JavaComplexAnnotation_1], v12 = BAR, v2 = 22, v3 = '\027', v4 = 24, v5 = 25L, v6 = 26.0, v7 = 27.0, v8 = false)], v12 = FOO, v13 = JavaSimpleAnnotation_1(v1 = 11, v10 = "world1", v11 = classOf[JavaSimpleAnnotation_1], v12 = FOO, v2 = 12, v3 = '\r', v4 = 14, v5 = 15L, v6 = 16.0, v7 = 17.0, v8 = false), v2 = 2, v3 = '\03', v4 = 4, v5 = 5L, v6 = 6.0, v7 = 7.0, v8 = false)) diff --git a/test/files/run/reflection-java-annotations.scala b/test/files/run/reflection-java-annotations.scala deleted file mode 100644 index 2e3fed48ce..0000000000 --- a/test/files/run/reflection-java-annotations.scala +++ /dev/null @@ -1,7 +0,0 @@ -object Test extends App { - import scala.reflect.runtime.universe._ - val sym = typeOf[JavaAnnottee].typeSymbol - sym.typeSignature - sym.annotations foreach (_.javaArgs) - println(sym.annotations) -} \ No newline at end of file diff --git a/test/files/run/reflection-java-annotations/JavaAnnottee_1.java b/test/files/run/reflection-java-annotations/JavaAnnottee_1.java new file mode 100644 index 0000000000..b241f5d25e --- /dev/null +++ b/test/files/run/reflection-java-annotations/JavaAnnottee_1.java @@ -0,0 +1,47 @@ +@JavaComplexAnnotation_1( + v1 = (byte)1, + v2 = (short)2, + v3 = (char)3, + v4 = (int)4, + v5 = (long)5, + v6 = (float)6, + v7 = (double)7, + v10 = "hello", + v11 = JavaAnnottee_1.class, + v12 = JavaSimpleEnumeration_1.FOO, + v13 = @JavaSimpleAnnotation_1( + v1 = (byte)11, + v2 = (short)12, + v3 = (char)13, + v4 = (int)14, + v5 = (long)15, + v6 = (float)16, + v7 = (double)17, + v10 = "world1", + v11 = JavaSimpleAnnotation_1.class, + v12 = JavaSimpleEnumeration_1.FOO + ), + v101 = {(byte)101, (byte)101}, + v102 = {(short)102, (short)102}, + v103 = {(char)103, (char)103}, + v104 = {(int)104, (int)104}, + v105 = {(long)105, (long)105}, + v106 = {(float)106, (float)106}, + v107 = {(double)107, (double)107}, + v108 = {false, true}, + v110 = {"hello", "world"}, + v111 = {JavaSimpleAnnotation_1.class, JavaComplexAnnotation_1.class}, + v112 = {JavaSimpleEnumeration_1.FOO, JavaSimpleEnumeration_1.BAR}, + v113 = {@JavaSimpleAnnotation_1( + v1 = (byte)21, + v2 = (short)22, + v3 = (char)23, + v4 = (int)24, + v5 = (long)25, + v6 = (float)26, + v7 = (double)27, + v10 = "world2", + v11 = JavaComplexAnnotation_1.class, + v12 = JavaSimpleEnumeration_1.BAR + )}) +public class JavaAnnottee_1 {} \ No newline at end of file diff --git a/test/files/run/reflection-java-annotations/JavaComplexAnnotation_1.java b/test/files/run/reflection-java-annotations/JavaComplexAnnotation_1.java new file mode 100644 index 0000000000..645eeb9399 --- /dev/null +++ b/test/files/run/reflection-java-annotations/JavaComplexAnnotation_1.java @@ -0,0 +1,34 @@ +import java.lang.annotation.Target; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.FIELD, ElementType.METHOD, ElementType.TYPE}) +public @interface JavaComplexAnnotation_1 { + byte v1(); + short v2(); + char v3(); + int v4(); + long v5(); + float v6(); + double v7(); + boolean v8() default false; + // void v9(); + String v10(); + Class v11(); + JavaSimpleEnumeration_1 v12(); + JavaSimpleAnnotation_1 v13(); + byte[] v101(); + short[] v102(); + char[] v103(); + int[] v104(); + long[] v105(); + float[] v106(); + double[] v107(); + boolean[] v108(); + String[] v110(); + Class[] v111(); + JavaSimpleEnumeration_1[] v112(); + JavaSimpleAnnotation_1[] v113(); +} \ No newline at end of file diff --git a/test/files/run/reflection-java-annotations/JavaSimpleAnnotation_1.java b/test/files/run/reflection-java-annotations/JavaSimpleAnnotation_1.java new file mode 100644 index 0000000000..c0f92fad2c --- /dev/null +++ b/test/files/run/reflection-java-annotations/JavaSimpleAnnotation_1.java @@ -0,0 +1,21 @@ +import java.lang.annotation.Target; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.FIELD, ElementType.METHOD, ElementType.TYPE}) +public @interface JavaSimpleAnnotation_1 { + byte v1(); + short v2(); + char v3(); + int v4(); + long v5(); + float v6(); + double v7(); + boolean v8() default false; + // void v9(); + String v10(); + Class v11(); + JavaSimpleEnumeration_1 v12(); +} \ No newline at end of file diff --git a/test/files/run/reflection-java-annotations/JavaSimpleEnumeration_1.java b/test/files/run/reflection-java-annotations/JavaSimpleEnumeration_1.java new file mode 100644 index 0000000000..39246141cc --- /dev/null +++ b/test/files/run/reflection-java-annotations/JavaSimpleEnumeration_1.java @@ -0,0 +1,4 @@ +enum JavaSimpleEnumeration_1 { + FOO, + BAR +} \ No newline at end of file diff --git a/test/files/run/reflection-java-annotations/Test_2.scala b/test/files/run/reflection-java-annotations/Test_2.scala new file mode 100644 index 0000000000..d2c3157071 --- /dev/null +++ b/test/files/run/reflection-java-annotations/Test_2.scala @@ -0,0 +1,7 @@ +object Test extends App { + import scala.reflect.runtime.universe._ + val sym = typeOf[JavaAnnottee_1].typeSymbol + sym.typeSignature + sym.annotations foreach (_.javaArgs) + println(sym.annotations) +} \ No newline at end of file diff --git a/test/files/run/reflection-java-crtp.scala b/test/files/run/reflection-java-crtp.scala deleted file mode 100644 index 260d3540dc..0000000000 --- a/test/files/run/reflection-java-crtp.scala +++ /dev/null @@ -1,8 +0,0 @@ -object Test extends App { - import scala.reflect.runtime.universe._ - val enum = typeOf[JavaSimpleEnumeration].baseClasses(1).asClass - // make sure that the E's in Enum> are represented by the same symbol - val e1 = enum.typeParams(0).asType - val TypeBounds(_, TypeRef(_, _, List(TypeRef(_, e2: TypeSymbol, _)))) = e1.typeSignature - println(e1, e2, e1 eq e2) -} \ No newline at end of file diff --git a/test/files/run/reflection-java-crtp/JavaSimpleEnumeration_1.java b/test/files/run/reflection-java-crtp/JavaSimpleEnumeration_1.java new file mode 100644 index 0000000000..39246141cc --- /dev/null +++ b/test/files/run/reflection-java-crtp/JavaSimpleEnumeration_1.java @@ -0,0 +1,4 @@ +enum JavaSimpleEnumeration_1 { + FOO, + BAR +} \ No newline at end of file diff --git a/test/files/run/reflection-java-crtp/Main_2.scala b/test/files/run/reflection-java-crtp/Main_2.scala new file mode 100644 index 0000000000..fb5668f323 --- /dev/null +++ b/test/files/run/reflection-java-crtp/Main_2.scala @@ -0,0 +1,8 @@ +object Test extends App { + import scala.reflect.runtime.universe._ + val enum = typeOf[JavaSimpleEnumeration_1].baseClasses(1).asClass + // make sure that the E's in Enum> are represented by the same symbol + val e1 = enum.typeParams(0).asType + val TypeBounds(_, TypeRef(_, _, List(TypeRef(_, e2: TypeSymbol, _)))) = e1.typeSignature + println(e1, e2, e1 eq e2) +} \ No newline at end of file diff --git a/test/files/run/t6548.check b/test/files/run/t6548.check index e82cae110a..5dfcb12e02 100644 --- a/test/files/run/t6548.check +++ b/test/files/run/t6548.check @@ -1,2 +1,2 @@ false -List(JavaAnnotationWithNestedEnum(value = VALUE)) +List(JavaAnnotationWithNestedEnum_1(value = VALUE)) diff --git a/test/files/run/t6548.scala b/test/files/run/t6548.scala deleted file mode 100644 index be3eb5b932..0000000000 --- a/test/files/run/t6548.scala +++ /dev/null @@ -1,12 +0,0 @@ -import scala.reflect.runtime.universe._ -import scala.reflect.runtime.{currentMirror => cm} - -class Bean { - @JavaAnnotationWithNestedEnum(JavaAnnotationWithNestedEnum.Value.VALUE) - def value = 1 -} - -object Test extends App { - println(cm.staticClass("Bean").isCaseClass) - println(typeOf[Bean].declaration(newTermName("value")).annotations) -} diff --git a/test/files/run/t6548/JavaAnnotationWithNestedEnum_1.java b/test/files/run/t6548/JavaAnnotationWithNestedEnum_1.java new file mode 100644 index 0000000000..32004de537 --- /dev/null +++ b/test/files/run/t6548/JavaAnnotationWithNestedEnum_1.java @@ -0,0 +1,17 @@ +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ElementType.ANNOTATION_TYPE, ElementType.METHOD, ElementType.FIELD, + ElementType.TYPE, ElementType.PARAMETER}) +@Retention(RetentionPolicy.RUNTIME) +public @interface JavaAnnotationWithNestedEnum_1 +{ + public Value value() default Value.VALUE; + + public enum Value + { + VALUE; + } +} \ No newline at end of file diff --git a/test/files/run/t6548/Test_2.scala b/test/files/run/t6548/Test_2.scala new file mode 100644 index 0000000000..6e4f6ba92a --- /dev/null +++ b/test/files/run/t6548/Test_2.scala @@ -0,0 +1,12 @@ +import scala.reflect.runtime.universe._ +import scala.reflect.runtime.{currentMirror => cm} + +class Bean { + @JavaAnnotationWithNestedEnum_1(JavaAnnotationWithNestedEnum_1.Value.VALUE) + def value = 1 +} + +object Test extends App { + println(cm.staticClass("Bean").isCaseClass) + println(typeOf[Bean].declaration(newTermName("value")).annotations) +} diff --git a/test/files/run/t7008/Impls_Macros_1.scala b/test/files/run/t7008/Impls_Macros_1.scala deleted file mode 100644 index f2eb7425f5..0000000000 --- a/test/files/run/t7008/Impls_Macros_1.scala +++ /dev/null @@ -1,12 +0,0 @@ -import language.experimental.macros -import scala.reflect.macros.Context - -object Macros { - def impl(c: Context) = { - val decls = c.typeOf[JavaClassWithCheckedExceptions[_]].declarations.toList - val s = decls.sortBy(_.name.toString).map(decl => (s"${decl.name}: ${decl.annotations}")).mkString(scala.compat.Platform.EOL) - c.universe.reify(println(c.literal(s).splice)) - } - - def foo = macro impl -} \ No newline at end of file diff --git a/test/files/run/t7008/Impls_Macros_2.scala b/test/files/run/t7008/Impls_Macros_2.scala new file mode 100644 index 0000000000..7a17314085 --- /dev/null +++ b/test/files/run/t7008/Impls_Macros_2.scala @@ -0,0 +1,12 @@ +import language.experimental.macros +import scala.reflect.macros.Context + +object Macros { + def impl(c: Context) = { + val decls = c.typeOf[JavaClassWithCheckedExceptions_1[_]].declarations.toList + val s = decls.sortBy(_.name.toString).map(decl => (s"${decl.name}: ${decl.annotations}")).mkString(scala.compat.Platform.EOL) + c.universe.reify(println(c.literal(s).splice)) + } + + def foo = macro impl +} \ No newline at end of file diff --git a/test/files/run/t7008/JavaClassWithCheckedExceptions_1.java b/test/files/run/t7008/JavaClassWithCheckedExceptions_1.java new file mode 100644 index 0000000000..dda2128302 --- /dev/null +++ b/test/files/run/t7008/JavaClassWithCheckedExceptions_1.java @@ -0,0 +1,7 @@ +class JavaClassWithCheckedExceptions_1 { + public JavaClassWithCheckedExceptions_1() throws NullPointerException {} + + public void bar() throws E1 {} + public void baz(int x) throws IllegalStateException {} + public void foo() throws E2 {} +} \ No newline at end of file diff --git a/test/files/run/t7008/Test_2.scala b/test/files/run/t7008/Test_2.scala deleted file mode 100644 index b67faa327f..0000000000 --- a/test/files/run/t7008/Test_2.scala +++ /dev/null @@ -1,9 +0,0 @@ -import scala.reflect.runtime.universe._ - -object Test extends App { - Macros.foo - println("=============") - - val decls = typeOf[JavaClassWithCheckedExceptions[_]].declarations.toList - decls sortBy (_.name.toString) foreach (decl => println(s"${decl.name}: ${decl.annotations}")) -} \ No newline at end of file diff --git a/test/files/run/t7008/Test_3.scala b/test/files/run/t7008/Test_3.scala new file mode 100644 index 0000000000..b2961a829e --- /dev/null +++ b/test/files/run/t7008/Test_3.scala @@ -0,0 +1,9 @@ +import scala.reflect.runtime.universe._ + +object Test extends App { + Macros.foo + println("=============") + + val decls = typeOf[JavaClassWithCheckedExceptions_1[_]].declarations.toList + decls sortBy (_.name.toString) foreach (decl => println(s"${decl.name}: ${decl.annotations}")) +} \ No newline at end of file -- cgit v1.2.3 From 8ae0e2a37764e0193dccd5658fd35f56edcbfd69 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Fri, 1 Feb 2013 11:55:01 -0800 Subject: SI-7039 unapplySeq result type independent of subpattern count Fixes a bug in the implementation of the `unapplySeq` part of the spec below. An `unapply` method with result type `R` in an object `x` matches the pattern `x(p_1, ..., p_n)` if it takes exactly one argument and, either: - `n = 0` and `R =:= Boolean`, or - `n = 1` and `R <:< Option[T]`, for some type `T`. The argument pattern `p1` is typed in turn with expected type `T`. - Or, `n > 1` and `R <:< Option[Product_n[T_1, ..., T_n]]`, for some types `T_1, ..., T_n`. The argument patterns `p_1, ..., p_n` are typed with expected types `T_1, ..., T_n`. An `unapplySeq` method in an object `x` matches the pattern `x(p_1, ..., p_n)` if it takes exactly one argument and its result type is of the form `Option[S]`, where either: - `S` is a subtype of `Seq[U]` for some element type `U`, (set `m = 0`) - or `S` is a `ProductX[T_1, ..., T_m]` and `T_m <: Seq[U]` (`m <= n`). The argument patterns `p_1, ..., p_n` are typed with expected types `T_1, ..., T_m, U, ..., U`. Here, `U` is repeated `n-m` times. --- .../scala/tools/nsc/typechecker/Infer.scala | 70 +++++++++++++--------- .../tools/nsc/typechecker/PatternMatching.scala | 2 +- test/files/run/t7039.check | 1 + test/files/run/t7039.scala | 11 ++++ 4 files changed, 55 insertions(+), 29 deletions(-) create mode 100644 test/files/run/t7039.check create mode 100644 test/files/run/t7039.scala (limited to 'test/files/run') diff --git a/src/compiler/scala/tools/nsc/typechecker/Infer.scala b/src/compiler/scala/tools/nsc/typechecker/Infer.scala index 581f9f3bfa..d724bc430e 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Infer.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Infer.scala @@ -58,20 +58,31 @@ trait Infer extends Checkable { * @throws TypeError when the unapply[Seq] definition is ill-typed * @returns (null, null) when the expected number of sub-patterns cannot be satisfied by the given extractor * - * From the spec: + * This is the spec currently implemented -- TODO: update it. + * * 8.1.8 ExtractorPatterns * * An extractor pattern x(p1, ..., pn) where n ≥ 0 is of the same syntactic form as a constructor pattern. * However, instead of a case class, the stable identifier x denotes an object which has a member method named unapply or unapplySeq that matches the pattern. - * An unapply method in an object x matches the pattern x(p1, ..., pn) if it takes exactly one argument and one of the following applies: * - * n = 0 and unapply’s result type is Boolean. + * An `unapply` method with result type `R` in an object `x` matches the + * pattern `x(p_1, ..., p_n)` if it takes exactly one argument and, either: + * - `n = 0` and `R =:= Boolean`, or + * - `n = 1` and `R <:< Option[T]`, for some type `T`. + * The argument pattern `p1` is typed in turn with expected type `T`. + * - Or, `n > 1` and `R <:< Option[Product_n[T_1, ..., T_n]]`, for some + * types `T_1, ..., T_n`. The argument patterns `p_1, ..., p_n` are + * typed with expected types `T_1, ..., T_n`. + * + * An `unapplySeq` method in an object `x` matches the pattern `x(p_1, ..., p_n)` + * if it takes exactly one argument and its result type is of the form `Option[S]`, + * where either: + * - `S` is a subtype of `Seq[U]` for some element type `U`, (set `m = 0`) + * - or `S` is a `ProductX[T_1, ..., T_m]` and `T_m <: Seq[U]` (`m <= n`). * - * n = 1 and unapply’s result type is Option[T], for some type T. - * the (only) argument pattern p1 is typed in turn with expected type T + * The argument patterns `p_1, ..., p_n` are typed with expected types + * `T_1, ..., T_m, U, ..., U`. Here, `U` is repeated `n-m` times. * - * n > 1 and unapply’s result type is Option[(T1, ..., Tn)], for some types T1, ..., Tn. - * the argument patterns p1, ..., pn are typed in turn with expected types T1, ..., Tn */ def extractorFormalTypes(pos: Position, resTp: Type, nbSubPats: Int, unappSym: Symbol): (List[Type], List[Type]) = { val isUnapplySeq = unappSym.name == nme.unapplySeq @@ -83,31 +94,34 @@ trait Infer extends Checkable { else toRepeated } + // empty list --> error, otherwise length == 1 + lazy val optionArgs = resTp.baseType(OptionClass).typeArgs + // empty list --> not a ProductN, otherwise product element types + def productArgs = getProductArgs(optionArgs.head) + val formals = - if (nbSubPats == 0 && booleanExtractor && !isUnapplySeq) Nil - else resTp.baseType(OptionClass).typeArgs match { - case optionTArg :: Nil => - def productArgs = getProductArgs(optionTArg) + // convert Seq[T] to the special repeated argument type + // so below we can use formalTypes to expand formals to correspond to the number of actuals + if (isUnapplySeq) { + if (optionArgs.nonEmpty) + productArgs match { + case Nil => List(seqToRepeatedChecked(optionArgs.head)) + case normalTps :+ seqTp => normalTps :+ seqToRepeatedChecked(seqTp) + } + else throw new TypeError(s"result type $resTp of unapplySeq defined in ${unappSym.fullLocationString} does not conform to Option[_]") + } else { + if (booleanExtractor && nbSubPats == 0) Nil + else if (optionArgs.nonEmpty) if (nbSubPats == 1) { - if (isUnapplySeq) List(seqToRepeatedChecked(optionTArg)) - else { - val productArity = productArgs.size - if (productArity > 1 && settings.lint.value) - global.currentUnit.warning(pos, s"extractor pattern binds a single value to a Product${productArity} of type ${optionTArg}") - List(optionTArg) - } + val productArity = productArgs.size + if (productArity > 1 && settings.lint.value) + global.currentUnit.warning(pos, s"extractor pattern binds a single value to a Product${productArity} of type ${optionArgs.head}") + optionArgs } // TODO: update spec to reflect we allow any ProductN, not just TupleN - else productArgs match { - case Nil if isUnapplySeq => List(seqToRepeatedChecked(optionTArg)) - case tps if isUnapplySeq => tps.init :+ seqToRepeatedChecked(tps.last) - case tps => tps - } - case _ => - if (isUnapplySeq) - throw new TypeError(s"result type $resTp of unapplySeq defined in ${unappSym.owner+unappSym.owner.locationString} not in {Option[_], Some[_]}") - else - throw new TypeError(s"result type $resTp of unapply defined in ${unappSym.owner+unappSym.owner.locationString} not in {Boolean, Option[_], Some[_]}") + else productArgs + else + throw new TypeError(s"result type $resTp of unapply defined in ${unappSym.fullLocationString} does not conform to Option[_] or Boolean") } // for unapplySeq, replace last vararg by as many instances as required by nbSubPats diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index b0745b4c09..59ced09451 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -800,7 +800,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL protected lazy val rawSubPatTypes = if (resultInMonad.typeSymbol eq UnitClass) Nil - else if(nbSubPats == 1) List(resultInMonad) + else if(!isSeq && nbSubPats == 1) List(resultInMonad) else getProductArgs(resultInMonad) match { case Nil => List(resultInMonad) case x => x diff --git a/test/files/run/t7039.check b/test/files/run/t7039.check new file mode 100644 index 0000000000..954906040f --- /dev/null +++ b/test/files/run/t7039.check @@ -0,0 +1 @@ +Matched! diff --git a/test/files/run/t7039.scala b/test/files/run/t7039.scala new file mode 100644 index 0000000000..475c4ae267 --- /dev/null +++ b/test/files/run/t7039.scala @@ -0,0 +1,11 @@ +object UnapplySeqTest { + def unapplySeq(any: Any): Option[(Int, Seq[Int])] = Some((5, List(1))) +} + +object Test extends App { + null match { + case UnapplySeqTest(5) => println("uh-oh") + case UnapplySeqTest(5, 1) => println("Matched!") // compiles + case UnapplySeqTest(5, xs @ _*) => println("toooo long: "+ (xs: Seq[Int])) + } +} \ No newline at end of file -- cgit v1.2.3 From c7d489e21f234bf1a2ea04c6b68990c53b5b387d Mon Sep 17 00:00:00 2001 From: James Iry Date: Sat, 2 Feb 2013 09:27:21 -0800 Subject: SI-5313 Test clobbers on the back edge of a loop I realized I was missing a test case for a local store early in a loop that was unused but turned out to be a clobber of a store later in the loop. --- test/files/run/t5313.check | 2 ++ test/files/run/t5313.scala | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'test/files/run') diff --git a/test/files/run/t5313.check b/test/files/run/t5313.check index aa30c0efa5..7a48b2b711 100644 --- a/test/files/run/t5313.check +++ b/test/files/run/t5313.check @@ -8,3 +8,5 @@ STORE_LOCAL(variable kept4) STORE_LOCAL(variable kept4) STORE_LOCAL(variable kept5) STORE_LOCAL(variable kept5) +STORE_LOCAL(variable kept6) +STORE_LOCAL(variable kept6) diff --git a/test/files/run/t5313.scala b/test/files/run/t5313.scala index 64009e29af..7da8726a1f 100644 --- a/test/files/run/t5313.scala +++ b/test/files/run/t5313.scala @@ -7,7 +7,7 @@ object Test extends IcodeTest { override def code = """class Foo { - def foo = true + def randomBoolean = util.Random.nextInt % 2 == 0 def bar = { var kept1 = new Object val result = new java.lang.ref.WeakReference(kept1) @@ -19,7 +19,7 @@ object Test extends IcodeTest { var erased4 = erased2 // and this val erased5 = erased4 // and this var kept2: Object = new Object // ultimately can't be eliminated - while(foo) { + while(randomBoolean) { val kept3 = kept2 kept2 = null // this can't, because it clobbers kept2, which is used erased4 = null // safe to eliminate @@ -36,6 +36,12 @@ object Test extends IcodeTest { kept5 = null // can't eliminate it's a clobber and it's used print(kept5) kept5 = null // can eliminate because we don't care about clobbers of nulls + while(randomBoolean) { + var kept6: AnyRef = null // not used, but have to keep because it clobbers the next used store + // on the back edge of the loop + kept6 = new Object // used + println(kept6) + } result } }""".stripMargin -- cgit v1.2.3 From 02dd4c974f33d137ea353a72e27efb70928fb378 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Mon, 4 Feb 2013 22:23:54 +0100 Subject: reflecting @throws defined in Scala code As per Jason's comment: How are Scala classes containing @throws annots treated? I can't figure out whether we pickle the annotation in addition to adding the exception to the signature. If we do, might we end up with duplicate annotations in runtime reflection? This warrants a test. See the context of the discussion here: https://github.com/scala/scala/pull/2040/files#r2874769. No, we won't end up with duplicates, because classes defined in Scala are loaded in a different completer. But I'll add a test - you can never have too many of those. --- test/files/run/t7008-scala-defined.check | 7 +++++++ test/files/run/t7008-scala-defined/Impls_Macros_2.scala | 12 ++++++++++++ .../ScalaClassWithCheckedExceptions_1.scala | 6 ++++++ test/files/run/t7008-scala-defined/Test_3.scala | 9 +++++++++ 4 files changed, 34 insertions(+) create mode 100644 test/files/run/t7008-scala-defined.check create mode 100644 test/files/run/t7008-scala-defined/Impls_Macros_2.scala create mode 100644 test/files/run/t7008-scala-defined/ScalaClassWithCheckedExceptions_1.scala create mode 100644 test/files/run/t7008-scala-defined/Test_3.scala (limited to 'test/files/run') diff --git a/test/files/run/t7008-scala-defined.check b/test/files/run/t7008-scala-defined.check new file mode 100644 index 0000000000..84ed62619e --- /dev/null +++ b/test/files/run/t7008-scala-defined.check @@ -0,0 +1,7 @@ +: List(throws[NullPointerException]("")) +bar: List(throws[E1]("")) +baz: List(throws[IllegalStateException]("")) +============= +: List(throws[NullPointerException]("")) +bar: List(throws[E1]("")) +baz: List(throws[IllegalStateException]("")) diff --git a/test/files/run/t7008-scala-defined/Impls_Macros_2.scala b/test/files/run/t7008-scala-defined/Impls_Macros_2.scala new file mode 100644 index 0000000000..94fd99018e --- /dev/null +++ b/test/files/run/t7008-scala-defined/Impls_Macros_2.scala @@ -0,0 +1,12 @@ +import language.experimental.macros +import scala.reflect.macros.Context + +object Macros { + def impl(c: Context) = { + val decls = c.typeOf[ScalaClassWithCheckedExceptions_1[_]].declarations.toList + val s = decls.sortBy(_.name.toString).map(decl => (s"${decl.name}: ${decl.annotations}")).mkString(scala.compat.Platform.EOL) + c.universe.reify(println(c.literal(s).splice)) + } + + def foo = macro impl +} \ No newline at end of file diff --git a/test/files/run/t7008-scala-defined/ScalaClassWithCheckedExceptions_1.scala b/test/files/run/t7008-scala-defined/ScalaClassWithCheckedExceptions_1.scala new file mode 100644 index 0000000000..7783c873ec --- /dev/null +++ b/test/files/run/t7008-scala-defined/ScalaClassWithCheckedExceptions_1.scala @@ -0,0 +1,6 @@ +class ScalaClassWithCheckedExceptions_1[E1 <: Exception] @throws[NullPointerException]("") () { + @throws[E1]("") def bar() {} + @throws[IllegalStateException]("") def baz(x: Int) {} + // FIXME: SI-7066 + // @throws[E2]("") def foo[E2 <: Exception] {} +} \ No newline at end of file diff --git a/test/files/run/t7008-scala-defined/Test_3.scala b/test/files/run/t7008-scala-defined/Test_3.scala new file mode 100644 index 0000000000..03bb79d311 --- /dev/null +++ b/test/files/run/t7008-scala-defined/Test_3.scala @@ -0,0 +1,9 @@ +import scala.reflect.runtime.universe._ + +object Test extends App { + Macros.foo + println("=============") + + val decls = typeOf[ScalaClassWithCheckedExceptions_1[_]].declarations.toList + decls sortBy (_.name.toString) foreach (decl => println(s"${decl.name}: ${decl.annotations}")) +} \ No newline at end of file -- cgit v1.2.3