From eac72bbee3bfa6c24f8b32400a136c085312030e Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Tue, 28 Sep 2010 22:13:25 +0000 Subject: Integrating feedback from martin and iulian int... Integrating feedback from martin and iulian into recent patches and prettifying checker output. No review. --- src/compiler/scala/tools/nsc/ast/TreeDSL.scala | 10 ++-- .../scala/tools/nsc/backend/icode/Checkers.scala | 59 +++++++++++++++++----- .../scala/tools/nsc/backend/icode/TypeKinds.scala | 1 + .../scala/tools/nsc/backend/icode/TypeStacks.scala | 6 +-- .../scala/tools/nsc/transform/ExplicitOuter.scala | 8 +-- 5 files changed, 57 insertions(+), 27 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/ast/TreeDSL.scala b/src/compiler/scala/tools/nsc/ast/TreeDSL.scala index 3b7d4aa26d..fa443447aa 100644 --- a/src/compiler/scala/tools/nsc/ast/TreeDSL.scala +++ b/src/compiler/scala/tools/nsc/ast/TreeDSL.scala @@ -193,7 +193,7 @@ trait TreeDSL { def defaultPos = sym.pos final def ===(rhs: Tree): ResultTreeType = - atPos(sym.pos)(mkTree(rhs) setSymbol sym) + atPos(pos)(mkTree(rhs) setSymbol sym) } trait ValCreator { self: VODDStart => @@ -238,9 +238,9 @@ trait TreeDSL { } class IfStart(cond: Tree, thenp: Tree) { - def THEN(x: Tree) = new IfStart(cond, x) + def THEN(x: Tree) = new IfStart(cond, x) def ELSE(elsep: Tree) = If(cond, thenp, elsep) - def ENDIF = If(cond, thenp, EmptyTree) + def ENDIF = If(cond, thenp, EmptyTree) } class TryStart(body: Tree, catches: List[CaseDef], fin: Tree) { def CATCH(xs: CaseDef*) = new TryStart(body, xs.toList, fin) @@ -301,9 +301,7 @@ trait TreeDSL { def TRY(tree: Tree) = new TryStart(tree, Nil, EmptyTree) def BLOCK(xs: Tree*) = Block(xs.init.toList, xs.last) def NOT(tree: Tree) = Select(tree, getMember(BooleanClass, nme.UNARY_!)) - - private val _SOME = scalaDot(nme.Some) - def SOME(xs: Tree*) = Apply(_SOME, List(makeTupleTerm(xs.toList, true))) + def SOME(xs: Tree*) = Apply(scalaDot(nme.Some), List(makeTupleTerm(xs.toList, true))) /** Typed trees from symbols. */ def THIS(sym: Symbol) = gen.mkAttributedThis(sym) diff --git a/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala b/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala index 8f402876bb..b4a5eca03c 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/Checkers.scala @@ -65,6 +65,7 @@ abstract class Checkers { val emptyStack = new TypeStack() val STRING = REFERENCE(definitions.StringClass) + val BOXED_UNIT = REFERENCE(definitions.BoxedUnitClass) val SCALA_NOTHING = REFERENCE(definitions.NothingClass) val SCALA_NULL = REFERENCE(definitions.NullClass) @@ -150,27 +151,52 @@ abstract class Checkers { def meet(bl: BasicBlock) { val preds = bl.predecessors + /** XXX workaround #1: one stack empty, the other has BoxedUnit. + * One example where this arises is: + * + * def f(b: Boolean): Unit = synchronized { if (b) () } + */ + def isAllUnits(s1: TypeStack, s2: TypeStack) = { + List(s1, s2) forall (x => x.types forall (_ == BOXED_UNIT)) + } + /** XXX workaround #2: different stacks heading into an exception + * handler which will clear them anyway. Examples where it arises: + * + * var bippy: Int = synchronized { if (b) 5 else 10 } + */ + def isHandlerBlock() = bl.exceptionHandlerStart + + /** The presence of emptyStack means that path has not yet been checked + * (and may not be empty) thus the reference eq tests. + */ def meet2(s1: TypeStack, s2: TypeStack): TypeStack = { + def workaround(msg: String) = { + checkerDebug(msg + ": " + method + " at block " + bl) + checkerDebug(" s1: " + s1) + checkerDebug(" s2: " + s2) + new TypeStack() + } + if (s1 eq emptyStack) s2 else if (s2 eq emptyStack) s1 - else { - if (s1.isEmpty && s2.isEmpty) { - // PP: I do not know the significance of this condition, but it arises a lot - // so I'm taking the intuitive position that any two empty stacks are as good - // as another, rather than throwing an exception as it did. - // If the reference eq test is achieving something please document. - return emptyStack - } - else if (s1.length != s2.length) + else if (s1.length != s2.length) { + if (isAllUnits(s1, s2)) + workaround("Ignoring mismatched boxed units") + else if (isHandlerBlock) + workaround("Ignoring mismatched stacks entering exception handler") + else throw new CheckerException("Incompatible stacks: " + s1 + " and " + s2 + " in " + method + " at entry to block: " + bl); + } + else { + val newStack = new TypeStack((s1.types, s2.types).zipped map lub) + if (newStack.nonEmpty) + checkerDebug("Checker created new stack:\n (%s, %s) => %s".format(s1, s2, newStack)) - val types = (s1.types, s2.types).zipped map lub - checkerDebug("Checker created new stack: (%s, %s) => %s".format(s1.types, s2.types, types)) - new TypeStack(types) + newStack } } - if (preds != Nil) { + if (preds.nonEmpty) { in(bl) = (preds map out.apply) reduceLeft meet2; log("Input changed for block: " + bl +" to: " + in(bl)); } @@ -221,8 +247,13 @@ abstract class Checkers { def pushStack(xs: TypeKind*): Unit = { xs foreach { x => - if (x == UNIT) + if (x == UNIT) { + /** PP: I admit I'm not yet figuring out how the stacks will balance when + * we ignore UNIT, but I expect this knowledge will emerge naturally. + * In the meantime I'm logging it to help me out. + */ logChecker("Ignoring pushed UNIT") + } else { stack push x checkerDebug(sizeString(true) + x) diff --git a/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala b/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala index b18c0da2a9..e72db4abc2 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala @@ -246,6 +246,7 @@ trait TypeKinds { self: ICodes => /** A class type. */ final case class REFERENCE(cls: Symbol) extends TypeKind { + override def toString = "REF(" + cls + ")" assert(cls ne null, "REFERENCE to null class symbol.") assert(cls != ArrayClass, diff --git a/src/compiler/scala/tools/nsc/backend/icode/TypeStacks.scala b/src/compiler/scala/tools/nsc/backend/icode/TypeStacks.scala index 74abec78bc..35fad7db87 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/TypeStacks.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/TypeStacks.scala @@ -24,7 +24,7 @@ trait TypeStacks { class TypeStack(var types: Rep) { if (types.nonEmpty) - checkerDebug("TypeStack init: " + types.mkString(", ")) + checkerDebug("Created " + this) def this() = this(Nil) def this(that: TypeStack) = this(that.types) @@ -79,8 +79,8 @@ trait TypeStacks { /* This method returns a String representation of the stack */ override def toString() = - if (types.isEmpty) "TypeStack()" - else types.mkString("TypeStack(" + types.size + " elems) {\n ", "\n ", "\n}") + if (types.isEmpty) "[]" + else types.mkString("[", " ", "]") override def hashCode() = types.hashCode() override def equals(other: Any): Boolean = other match { diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala index e65965d3ff..160a0d305e 100644 --- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala +++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala @@ -316,8 +316,8 @@ abstract class ExplicitOuter extends InfoTransform /** If we don't re-type the tree, we see self-type related crashes like #266. */ localTyper typed { - (DEF(outerAcc) withType null) === rhs - } setPos currentClass.pos + (DEF(outerAcc) withPos currentClass.pos withType null) === rhs + } } /** The definition tree of the outer accessor for class mixinClass. @@ -334,11 +334,11 @@ abstract class ExplicitOuter extends InfoTransform else gen.mkAttributedQualifier(currentClass.thisType baseType mixinClass prefix) localTyper typed { - DEF(outerAcc) === { + (DEF(outerAcc) withPos currentClass.pos) === { // Need to cast for nested outer refs in presence of self-types. See ticket #3274. transformer.transform(path) AS_ANY outerAcc.info.resultType } - } setPos currentClass.pos + } } /** If FLAG is set on symbol, sets notFLAG (this exists in anticipation of generalizing). */ -- cgit v1.2.3