From f9c73f819ef40293caff08b03f8d816d45fc2b72 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 1 Jan 2012 21:48:44 +0100 Subject: Hardening of adaptoNewRun to survive issues in presentation compiler. Previously, adaptToNewRun crashed when an object declaration got replaced by a value with the same name, because the module class no longer existed. This caused crashes in the presentation compiler when class files disappeared because of a clean build. The new behavior avoids assertion errors. --- src/compiler/scala/reflect/internal/Types.scala | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/compiler/scala/reflect/internal/Types.scala b/src/compiler/scala/reflect/internal/Types.scala index 690f9b7204..a644baf9f9 100644 --- a/src/compiler/scala/reflect/internal/Types.scala +++ b/src/compiler/scala/reflect/internal/Types.scala @@ -4284,10 +4284,14 @@ A type's typeSymbol should never be inspected directly. definitions.RootPackage } else if (sym.isModuleClass) { val sourceModule1 = adaptToNewRun(pre, sym.sourceModule) - val result = sourceModule1.moduleClass - val msg = "sym = %s, sourceModule = %s, sourceModule.moduleClass = %s => sourceModule1 = %s, sourceModule1.moduleClass = %s" - assert(result != NoSymbol, msg.format(sym, sym.sourceModule, sym.sourceModule.moduleClass, sourceModule1, sourceModule1.moduleClass)) - result + var result = sourceModule1.moduleClass + if (result == NoSymbol) result = sourceModule1.initialize.moduleClass + if (result != NoSymbol) result + else { + val msg = "Cannot adapt module class; sym = %s, sourceModule = %s, sourceModule.moduleClass = %s => sourceModule1 = %s, sourceModule1.moduleClass = %s" + debuglog(msg.format(sym, sym.sourceModule, sym.sourceModule.moduleClass, sourceModule1, sourceModule1.moduleClass)) + sym + } } else if ((pre eq NoPrefix) || (pre eq NoType) || sym.isPackageClass) { sym } else { -- cgit v1.2.3 From 4ec2e7163774bddd6bc61ad08429e1daa3dd8a78 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 2 Jan 2012 19:10:41 +0100 Subject: Changed boxing of free mutable variables to be flexible wrt when liftcode takes place. A major redesign that unifies the various different approaches to boxing of free variables. Free variables are marked with CAPTURED and eliminated by LambdaLift. I also added some hooks in MacroContext that a reifier needs to use. --- src/compiler/scala/tools/nsc/Global.scala | 3 +- src/compiler/scala/tools/nsc/MacroContext.scala | 10 ++ src/compiler/scala/tools/nsc/ast/Trees.scala | 22 +++++ .../scala/tools/nsc/transform/LambdaLift.scala | 67 ++++++++----- .../scala/tools/nsc/transform/LiftCode.scala | 104 ++------------------- .../scala/tools/nsc/typechecker/Duplicators.scala | 2 +- .../scala/tools/nsc/typechecker/Typers.scala | 4 + src/library/scala/reflect/api/MacroContext.scala | 15 +++ 8 files changed, 105 insertions(+), 122 deletions(-) create mode 100644 src/compiler/scala/tools/nsc/MacroContext.scala create mode 100644 src/library/scala/reflect/api/MacroContext.scala diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index f70aa60309..2dd32e355b 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -39,6 +39,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter) extends Symb with Trees with TreePrinters with DocComments + with MacroContext with symtab.Positions { override def settings = currentSettings @@ -151,7 +152,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter) extends Symb /** Register top level class (called on entering the class) */ def registerTopLevelSym(sym: Symbol) {} - + // ------------------ Reporting ------------------------------------- // not deprecated yet, but a method called "error" imported into diff --git a/src/compiler/scala/tools/nsc/MacroContext.scala b/src/compiler/scala/tools/nsc/MacroContext.scala new file mode 100644 index 0000000000..e739eade3a --- /dev/null +++ b/src/compiler/scala/tools/nsc/MacroContext.scala @@ -0,0 +1,10 @@ +package scala.tools.nsc + +import symtab.Flags._ + +trait MacroContext extends reflect.api.MacroContext { self: Global => + + def captureVariable(vble: Symbol): Unit = vble setFlag CAPTURED + + def referenceCapturedVariable(id: Ident): Tree = ReferenceToBoxed(id) +} diff --git a/src/compiler/scala/tools/nsc/ast/Trees.scala b/src/compiler/scala/tools/nsc/ast/Trees.scala index 30ee7fc885..88a9b5e18b 100644 --- a/src/compiler/scala/tools/nsc/ast/Trees.scala +++ b/src/compiler/scala/tools/nsc/ast/Trees.scala @@ -44,6 +44,15 @@ trait Trees extends reflect.internal.Trees { self: Global => /** emitted by typer, eliminated by refchecks */ case class TypeTreeWithDeferredRefCheck()(val check: () => TypeTree) extends TypTree + + /** Marks underlying reference to id as boxed. + * @pre: id must refer to a captured variable + * A reference such marked will refer to the boxed entity, no dereferencing + * with `.elem` is done on it. + * This tree node can be emitted by macros such as reify that call markBoxedReference. + * It is eliminated in LambdaLift, where the boxing conversion takes place. + */ + case class ReferenceToBoxed(idt: Ident) extends TermTree // --- factory methods ---------------------------------------------------------- @@ -152,6 +161,8 @@ trait Trees extends reflect.internal.Trees { self: Global => traverser.traverse(lhs); traverser.traverse(rhs) case SelectFromArray(qualifier, selector, erasure) => traverser.traverse(qualifier) + case ReferenceToBoxed(idt) => + traverser.traverse(idt) case TypeTreeWithDeferredRefCheck() => // TODO: should we traverse the wrapped tree? // (and rewrap the result? how to update the deferred check? would need to store wrapped tree instead of returning it from check) case _ => super.xtraverse(traverser, tree) @@ -161,6 +172,7 @@ trait Trees extends reflect.internal.Trees { self: Global => def DocDef(tree: Tree, comment: DocComment, definition: Tree): DocDef def AssignOrNamedArg(tree: Tree, lhs: Tree, rhs: Tree): AssignOrNamedArg def SelectFromArray(tree: Tree, qualifier: Tree, selector: Name, erasure: Type): SelectFromArray + def ReferenceToBoxed(tree: Tree, idt: Ident): ReferenceToBoxed def TypeTreeWithDeferredRefCheck(tree: Tree): TypeTreeWithDeferredRefCheck } @@ -174,6 +186,8 @@ trait Trees extends reflect.internal.Trees { self: Global => new AssignOrNamedArg(lhs, rhs).copyAttrs(tree) def SelectFromArray(tree: Tree, qualifier: Tree, selector: Name, erasure: Type) = new SelectFromArray(qualifier, selector, erasure).copyAttrs(tree) + def ReferenceToBoxed(tree: Tree, idt: Ident) = + new ReferenceToBoxed(idt).copyAttrs(tree) def TypeTreeWithDeferredRefCheck(tree: Tree) = tree match { case dc@TypeTreeWithDeferredRefCheck() => new TypeTreeWithDeferredRefCheck()(dc.check).copyAttrs(tree) } @@ -195,6 +209,11 @@ trait Trees extends reflect.internal.Trees { self: Global => if (qualifier0 == qualifier) && (selector0 == selector) => t case _ => this.treeCopy.SelectFromArray(tree, qualifier, selector, erasure) } + def ReferenceToBoxed(tree: Tree, idt: Ident) = tree match { + case t @ ReferenceToBoxed(idt0) + if (idt0 == idt) => t + case _ => this.treeCopy.ReferenceToBoxed(tree, idt) + } def TypeTreeWithDeferredRefCheck(tree: Tree) = tree match { case t @ TypeTreeWithDeferredRefCheck() => t case _ => this.treeCopy.TypeTreeWithDeferredRefCheck(tree) @@ -220,6 +239,9 @@ trait Trees extends reflect.internal.Trees { self: Global => case SelectFromArray(qualifier, selector, erasure) => transformer.treeCopy.SelectFromArray( tree, transformer.transform(qualifier), selector, erasure) + case ReferenceToBoxed(idt) => + transformer.treeCopy.ReferenceToBoxed( + tree, transformer.transform(idt) match { case idt1: Ident => idt1 }) case TypeTreeWithDeferredRefCheck() => transformer.treeCopy.TypeTreeWithDeferredRefCheck(tree) } diff --git a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala index 2310eae9bb..2180fd4f3a 100644 --- a/src/compiler/scala/tools/nsc/transform/LambdaLift.scala +++ b/src/compiler/scala/tools/nsc/transform/LambdaLift.scala @@ -17,6 +17,19 @@ abstract class LambdaLift extends InfoTransform { /** the following two members override abstract members in Transform */ val phaseName: String = "lambdalift" + + /** Converts types of captured variables to *Ref types. + */ + def boxIfCaptured(sym: Symbol, tpe: Type, erasedTypes: Boolean) = + if (sym.isCapturedVariable) { + val symClass = tpe.typeSymbol + def refType(valueRef: Map[Symbol, Symbol], objectRefClass: Symbol) = + if (isValueClass(symClass) && symClass != UnitClass) valueRef(symClass).tpe + else if (erasedTypes) objectRefClass.tpe + else appliedType(objectRefClass.typeConstructor, List(tpe)) + if (sym.hasAnnotation(VolatileAttr)) refType(volatileRefClass, VolatileObjectRefClass) + else refType(refClass, ObjectRefClass) + } else tpe private val lifted = new TypeMap { def apply(tp: Type): Type = tp match { @@ -31,7 +44,8 @@ abstract class LambdaLift extends InfoTransform { } } - def transformInfo(sym: Symbol, tp: Type): Type = lifted(tp) + def transformInfo(sym: Symbol, tp: Type): Type = + boxIfCaptured(sym, lifted(tp), erasedTypes = true) protected def newTransformer(unit: CompilationUnit): Transformer = new LambdaLifter(unit) @@ -55,7 +69,10 @@ abstract class LambdaLift extends InfoTransform { /** Buffers for lifted out classes and methods */ private val liftedDefs = new LinkedHashMap[Symbol, List[Tree]] - + + /** True if we are transforming under a ReferenceToBoxed node */ + private var isBoxedRef = false + private type SymSet = TreeSet[Symbol] private def newSymSet = new TreeSet[Symbol](_ isLess _) @@ -116,22 +133,7 @@ abstract class LambdaLift extends InfoTransform { } changedFreeVars = true debuglog("" + sym + " is free in " + enclosure); - if (sym.isVariable && !sym.hasFlag(CAPTURED)) { - // todo: We should merge this with the lifting done in liftCode. - // We do have to lift twice: in liftCode, because Code[T] needs to see the lifted version - // and here again because lazy bitmaps are introduced later and get lifted here. - // But we should factor out the code and run it twice. - sym setFlag CAPTURED - val symClass = sym.tpe.typeSymbol - atPhase(phase.next) { - sym updateInfo ( - if (sym.hasAnnotation(VolatileAttr)) - if (isValueClass(symClass)) volatileRefClass(symClass).tpe else VolatileObjectRefClass.tpe - else - if (isValueClass(symClass)) refClass(symClass).tpe else ObjectRefClass.tpe - ) - } - } + if (sym.isVariable) sym setFlag CAPTURED } !enclosure.isClass } @@ -340,7 +342,7 @@ abstract class LambdaLift extends InfoTransform { EmptyTree } - private def postTransform(tree: Tree): Tree = { + private def postTransform(tree: Tree, isBoxedRef: Boolean = false): Tree = { val sym = tree.symbol tree match { case ClassDef(_, _, _, _) => @@ -363,8 +365,19 @@ abstract class LambdaLift extends InfoTransform { } case arg => arg } + /** Wrap expr argument in new *Ref(..) constructor, but make + * sure that Try expressions stay at toplevel. + */ + def refConstr(expr: Tree): Tree = expr match { + case Try(block, catches, finalizer) => + Try(refConstr(block), catches map refConstrCase, finalizer) + case _ => + Apply(Select(New(TypeTree(sym.tpe)), nme.CONSTRUCTOR), List(expr)) + } + def refConstrCase(cdef: CaseDef): CaseDef = + CaseDef(cdef.pat, cdef.guard, refConstr(cdef.body)) treeCopy.ValDef(tree, mods, name, tpt1, typer.typedPos(rhs.pos) { - Apply(Select(New(TypeTree(sym.tpe)), nme.CONSTRUCTOR), List(constructorArg)) + refConstr(constructorArg) }) } else tree case Return(Block(stats, value)) => @@ -388,7 +401,7 @@ abstract class LambdaLift extends InfoTransform { atPos(tree.pos)(proxyRef(sym)) else tree else tree - if (sym.isCapturedVariable) + if (sym.isCapturedVariable && !isBoxedRef) atPos(tree.pos) { val tp = tree.tpe val elemTree = typer typed Select(tree1 setType sym.tpe, nme.elem) @@ -406,10 +419,16 @@ abstract class LambdaLift extends InfoTransform { tree } } + + private def preTransform(tree: Tree) = super.transform(tree) setType lifted(tree.tpe) - override def transform(tree: Tree): Tree = - postTransform(super.transform(tree) setType lifted(tree.tpe)) - + override def transform(tree: Tree): Tree = tree match { + case ReferenceToBoxed(idt) => + postTransform(preTransform(idt), isBoxedRef = true) + case _ => + postTransform(preTransform(tree)) + } + /** Transform statements and add lifted definitions to them. */ override def transformStats(stats: List[Tree], exprOwner: Symbol): List[Tree] = { def addLifted(stat: Tree): Tree = stat match { diff --git a/src/compiler/scala/tools/nsc/transform/LiftCode.scala b/src/compiler/scala/tools/nsc/transform/LiftCode.scala index 720509644b..171d1df975 100644 --- a/src/compiler/scala/tools/nsc/transform/LiftCode.scala +++ b/src/compiler/scala/tools/nsc/transform/LiftCode.scala @@ -110,18 +110,10 @@ abstract class LiftCode extends Transform with TypingTransformers { } } - /** Set of mutable local variables that are free in some inner method. */ - private val freeMutableVars: mutable.Set[Symbol] = new mutable.HashSet - private val converted: mutable.Set[Symbol] = new mutable.HashSet // debug - override def transformUnit(unit: CompilationUnit) { - freeMutableVars.clear() - freeLocalsTraverser(unit.body) atPhase(phase.next) { super.transformUnit(unit) } - for (v <- freeMutableVars) //!!! remove - assert(converted contains v, "unconverted: " + v + " in " + v.owner + " in unit " + unit) } override def transform(tree: Tree): Tree = { @@ -137,24 +129,6 @@ abstract class LiftCode extends Transform with TypingTransformers { result } } finally printTypings = saved - case ValDef(mods, name, tpt, rhs) if (freeMutableVars(sym)) => // box mutable variables that are accessed from a local closure - val tpt1 = TypeTree(sym.tpe) setPos tpt.pos - /* Creating a constructor argument if one isn't present. */ - val constructorArg = rhs match { - case EmptyTree => gen.mkZero(atPhase(phase.prev)(sym.tpe)) - case _ => transform(rhs) - } - val rhs1 = typer.typedPos(rhs.pos) { - Apply(Select(New(TypeTree(sym.tpe)), nme.CONSTRUCTOR), List(constructorArg)) - } - sym resetFlag MUTABLE - sym removeAnnotation VolatileAttr - converted += sym // dereference boxed variables - treeCopy.ValDef(tree, mods &~ MUTABLE, name, tpt1, rhs1) - case Ident(name) if freeMutableVars(sym) => - localTyper.typedPos(tree.pos) { - Select(tree setType sym.tpe, nme.elem) - } case _ => super.transform(tree) } @@ -170,74 +144,6 @@ abstract class LiftCode extends Transform with TypingTransformers { New(TypeTree(appliedType(definitions.CodeClass.typeConstructor, List(treetpe.widen))), List(List(arg))) } - - /** - * PP: There is apparently some degree of overlap between the CAPTURED - * flag and the role being filled here. I think this is how this was able - * to go for so long looking only at DefDef and Ident nodes, as bugs - * would only emerge under more complicated conditions such as #3855. - * I'll try to figure it all out, but if someone who already knows the - * whole story wants to fill it in, that too would be great. - * - * XXX I found this had been cut and pasted between LiftCode and UnCurry, - * and seems to be running in both. - */ - private val freeLocalsTraverser = new Traverser { - var currentMethod: Symbol = NoSymbol - var maybeEscaping = false - - def withEscaping(body: => Unit) { - val saved = maybeEscaping - maybeEscaping = true - try body - finally maybeEscaping = saved - } - - override def traverse(tree: Tree) = tree match { - case DefDef(_, _, _, _, _, _) => - val lastMethod = currentMethod - currentMethod = tree.symbol - try super.traverse(tree) - finally currentMethod = lastMethod - /** A method call with a by-name parameter represents escape. */ - case Apply(fn, args) if fn.symbol.paramss.nonEmpty => - traverse(fn) - treeInfo.foreachMethodParamAndArg(tree) { (param, arg) => - if (param.tpe != null && isByNameParamType(param.tpe)) - withEscaping(traverse(arg)) - else - traverse(arg) - } - - /** The rhs of a closure represents escape. */ - case Function(vparams, body) => - vparams foreach traverse - withEscaping(traverse(body)) - - /** - * The appearance of an ident outside the method where it was defined or - * anytime maybeEscaping is true implies escape. - */ - case Ident(_) => - val sym = tree.symbol - if (sym.isVariable && sym.owner.isMethod && (maybeEscaping || sym.owner != currentMethod)) { - freeMutableVars += sym - val symTpe = sym.tpe - val symClass = symTpe.typeSymbol - atPhase(phase.next) { - def refType(valueRef: Map[Symbol, Symbol], objectRefClass: Symbol) = - if (isValueClass(symClass) && symClass != UnitClass) valueRef(symClass).tpe - else appliedType(objectRefClass.typeConstructor, List(symTpe)) - - sym updateInfo ( - if (sym.hasAnnotation(VolatileAttr)) refType(volatileRefClass, VolatileObjectRefClass) - else refType(refClass, ObjectRefClass)) - } - } - case _ => - super.traverse(tree) - } - } } /** @@ -385,7 +291,10 @@ abstract class LiftCode extends Transform with TypingTransformers { else { if (sym.isTerm) { if (reifyDebug) println("Free: " + sym) - mirrorCall("freeVar", reify(sym.name.toString), reify(sym.tpe), Ident(sym)) + val symtpe = lambdaLift.boxIfCaptured(sym, sym.tpe, erasedTypes = false) + def markIfCaptured(arg: Ident): Tree = + if (sym.isCapturedVariable) referenceCapturedVariable(arg) else arg + mirrorCall("freeVar", reify(sym.name.toString), reify(symtpe), markIfCaptured(Ident(sym))) } else { if (reifyDebug) println("Late local: " + sym) registerReifiableSymbol(sym) @@ -471,7 +380,10 @@ abstract class LiftCode extends Transform with TypingTransformers { case This(_) if !(boundSyms contains tree.symbol) => reifyFree(tree) case Ident(_) if !(boundSyms contains tree.symbol) => - reifyFree(tree) + if (tree.symbol.isVariable && tree.symbol.owner.isTerm) { + captureVariable(tree.symbol) // Note order dependency: captureVariable needs to come before reifyTree here. + mirrorCall("Select", reifyFree(tree), reifyName(nme.elem)) + } else reifyFree(tree) case tt: TypeTree if (tt.tpe != null) => if (!(boundSyms exists (tt.tpe contains _))) mirrorCall("TypeTree", reifyType(tt.tpe)) else if (tt.original != null) reify(tt.original) diff --git a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala index 2bed5bffd4..3536608efd 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Duplicators.scala @@ -248,7 +248,7 @@ abstract class Duplicators extends Analyzer { case vdef @ ValDef(mods, name, tpt, rhs) => // log("vdef fixing tpe: " + tree.tpe + " with sym: " + tree.tpe.typeSymbol + " and " + invalidSyms) - if (mods.hasFlag(Flags.LAZY)) vdef.symbol.resetFlag(Flags.MUTABLE) + //if (mods.hasFlag(Flags.LAZY)) vdef.symbol.resetFlag(Flags.MUTABLE) // Martin to Iulian: lazy vars can now appear because they are no longer boxed; Please check that deleting this statement is OK. vdef.tpt.tpe = fixType(vdef.tpt.tpe) vdef.tpe = null super.typed(vdef, mode, pt) diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 6b6b905e16..9991836344 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -4271,6 +4271,10 @@ trait Typers extends Modes with Adaptations with PatMatVirtualiser { else typedIdent(name) + case ReferenceToBoxed(idt @ Ident(_)) => + val id1 = typed1(idt, mode, pt) match { case id: Ident => id } + treeCopy.ReferenceToBoxed(tree, id1) setType AnyRefClass.tpe + case Literal(value) => tree setType ( if (value.tag == UnitTag) UnitClass.tpe diff --git a/src/library/scala/reflect/api/MacroContext.scala b/src/library/scala/reflect/api/MacroContext.scala new file mode 100644 index 0000000000..e23357d26e --- /dev/null +++ b/src/library/scala/reflect/api/MacroContext.scala @@ -0,0 +1,15 @@ +package scala.reflect +package api + +trait MacroContext extends Universe { + + /** Mark a variable as captured; i.e. force boxing in a *Ref type. + */ + def captureVariable(vble: Symbol): Unit + + /** Mark given identifier as a reference to a captured variable itself + * suppressing dereferencing with the `elem` field. + */ + def referenceCapturedVariable(id: Ident): Tree + +} \ No newline at end of file -- cgit v1.2.3 From fafbffc2950aa3f25f91575786093e044f9af549 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 4 Jan 2012 07:05:19 +0100 Subject: Closes SI-5354. The reason why the test case compiled without error is pretty devious: When checking the `Foo.x' implicit, a CyclicReference error occurs which causes the alternative to be discarded. Why a CylicReference? Because the inferencer tries to decide whether the owner of `z` is a subclass of the owner od `x`. To do this, it computed the info of the owner of `z1`, which is not complete because no result type for `f1` was given. Hence a CyclicReference error. The fix is twofold: (1) We make isNonBottomSubClass smarter so that it always returns false if the symbol in question is not a type; hence the info need not be computed. (2) It's dubious to swallow CyclicReference errors anywhere, but I deemed it too risky to propagate them. But at least the CyclicReference is now logged if -Ylog-implicit is true. This hopefully spares future maintainers the same detective work I had to go through when digging this out. --- src/compiler/scala/reflect/internal/Symbols.scala | 15 +++++++++------ src/compiler/scala/tools/nsc/typechecker/Implicits.scala | 9 ++++++++- test/files/neg/t5354.check | 7 +++++++ test/files/neg/t5354.scala | 15 +++++++++++++++ 4 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 test/files/neg/t5354.check create mode 100644 test/files/neg/t5354.scala diff --git a/src/compiler/scala/reflect/internal/Symbols.scala b/src/compiler/scala/reflect/internal/Symbols.scala index 0c57f0c43a..e629b0ed43 100644 --- a/src/compiler/scala/reflect/internal/Symbols.scala +++ b/src/compiler/scala/reflect/internal/Symbols.scala @@ -1244,12 +1244,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => */ final def isNestedIn(that: Symbol): Boolean = owner == that || owner != NoSymbol && (owner isNestedIn that) - - /** Is this class symbol a subclass of that symbol? */ - final def isNonBottomSubClass(that: Symbol): Boolean = ( - (this eq that) || this.isError || that.isError || - info.baseTypeIndex(that) >= 0 - ) + + /** Is this class symbol a subclass of that symbol, + * and is this class symbol also different from Null or Nothing? */ + def isNonBottomSubClass(that: Symbol): Boolean = false /** Overridden in NullClass and NothingClass for custom behavior. */ @@ -2226,6 +2224,11 @@ trait Symbols extends api.Symbols { self: SymbolTable => super.info_=(tp) } + final override def isNonBottomSubClass(that: Symbol): Boolean = ( + (this eq that) || this.isError || that.isError || + info.baseTypeIndex(that) >= 0 + ) + override def reset(completer: Type) { super.reset(completer) tpePeriod = NoPeriod diff --git a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala index d54cb248cf..77dde88a80 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Implicits.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Implicits.scala @@ -816,7 +816,14 @@ trait Implicits { val newPending = undoLog undo { is filterNot (alt => alt == i || { try improves(i, alt) - catch { case e: CyclicReference => true } + catch { + case e: CyclicReference => + if (printInfers) { + println(i+" discarded because cyclic reference occurred") + e.printStackTrace() + } + true + } }) } rankImplicits(newPending, i :: acc) diff --git a/test/files/neg/t5354.check b/test/files/neg/t5354.check new file mode 100644 index 0000000000..e47cecb5fe --- /dev/null +++ b/test/files/neg/t5354.check @@ -0,0 +1,7 @@ +t5354.scala:9: error: ambiguous implicit values: + both method x123 in package foo of type => foo.Bippy + and method z of type => foo.Bippy + match expected type foo.Bippy + implicitly[Bippy] + ^ +one error found diff --git a/test/files/neg/t5354.scala b/test/files/neg/t5354.scala new file mode 100644 index 0000000000..99b5650155 --- /dev/null +++ b/test/files/neg/t5354.scala @@ -0,0 +1,15 @@ +package object foo { + implicit def x123: Bippy = new Bippy("x") +} +package foo { + class Bippy(override val toString: String){ } + class Dingus { + def f1 = { + implicit def z: Bippy = new Bippy("z") + implicitly[Bippy] + } + } + object Test extends App { + println(new Dingus().f1) + } +} -- cgit v1.2.3 From 4787f883604d1344257c0b40c15790c3dde477f2 Mon Sep 17 00:00:00 2001 From: Szabolcs Berecz Date: Sat, 7 Jan 2012 17:40:00 +0100 Subject: Fixed equality and string representation of xml attributes with null value Prior to this patch was not equal to and it's string representation was "" instead of "" This includes changing MetaData.normalize() so that it doesn't reverse the chain. On the downside, the iterate function in MetaData.normalize() is not tail-recursive now. --- src/library/scala/xml/Elem.scala | 4 +++- src/library/scala/xml/MetaData.scala | 4 ++-- src/library/scala/xml/UnprefixedAttribute.scala | 2 +- src/library/scala/xml/Utility.scala | 2 +- test/files/jvm/xml03syntax.check | 2 +- test/files/run/xml-attribute.scala | 14 ++++++++++++++ 6 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 test/files/run/xml-attribute.scala diff --git a/src/library/scala/xml/Elem.scala b/src/library/scala/xml/Elem.scala index 127e6e0ab7..df52b34f87 100644 --- a/src/library/scala/xml/Elem.scala +++ b/src/library/scala/xml/Elem.scala @@ -41,7 +41,7 @@ object Elem { class Elem( override val prefix: String, val label: String, - override val attributes: MetaData, + attributes1: MetaData, override val scope: NamespaceBinding, val child: Node*) extends Node with Serializable @@ -49,6 +49,8 @@ extends Node with Serializable final override def doCollectNamespaces = true final override def doTransform = true + override val attributes = MetaData.normalize(attributes1, scope) + if (prefix == "") throw new IllegalArgumentException("prefix of zero length, use null instead") diff --git a/src/library/scala/xml/MetaData.scala b/src/library/scala/xml/MetaData.scala index 98e863eb37..c516747bae 100644 --- a/src/library/scala/xml/MetaData.scala +++ b/src/library/scala/xml/MetaData.scala @@ -38,8 +38,8 @@ object MetaData { def iterate(md: MetaData, normalized_attribs: MetaData, set: Set[String]): MetaData = { lazy val key = getUniversalKey(md, scope) if (md eq Null) normalized_attribs - else if (set(key)) iterate(md.next, normalized_attribs, set) - else iterate(md.next, md copy normalized_attribs, set + key) + else if ((md.value eq null) || set(key)) iterate(md.next, normalized_attribs, set) + else md copy iterate(md.next, normalized_attribs, set + key) } iterate(attribs, Null, Set()) } diff --git a/src/library/scala/xml/UnprefixedAttribute.scala b/src/library/scala/xml/UnprefixedAttribute.scala index c56fba1e6c..b6800d5ed1 100644 --- a/src/library/scala/xml/UnprefixedAttribute.scala +++ b/src/library/scala/xml/UnprefixedAttribute.scala @@ -22,7 +22,7 @@ extends Attribute final val pre = null val next = if (value ne null) next1 else next1.remove(key) - /** same as this(key, Text(value), next) */ + /** same as this(key, Text(value), next), or no attribute if value is null */ def this(key: String, value: String, next: MetaData) = this(key, if (value ne null) Text(value) else null: NodeSeq, next) diff --git a/src/library/scala/xml/Utility.scala b/src/library/scala/xml/Utility.scala index 9b48f4e1bb..fc20b892b9 100644 --- a/src/library/scala/xml/Utility.scala +++ b/src/library/scala/xml/Utility.scala @@ -61,7 +61,7 @@ object Utility extends AnyRef with parsing.TokenTests { val key = md.key val smaller = sort(md.filter { m => m.key < key }) val greater = sort(md.filter { m => m.key > key }) - smaller.append( Null ).append(md.copy ( greater )) + smaller.copy(md.copy ( greater )) } /** Return the node with its attribute list sorted alphabetically diff --git a/test/files/jvm/xml03syntax.check b/test/files/jvm/xml03syntax.check index 75dc539137..9fbedc2ae6 100644 --- a/test/files/jvm/xml03syntax.check +++ b/test/files/jvm/xml03syntax.check @@ -23,4 +23,4 @@ true 4 node=, key=Some(hello) -node=, key=None +node=, key=None diff --git a/test/files/run/xml-attribute.scala b/test/files/run/xml-attribute.scala new file mode 100644 index 0000000000..2b83f70b22 --- /dev/null +++ b/test/files/run/xml-attribute.scala @@ -0,0 +1,14 @@ +import xml.Node + +object Test { + def main(args: Array[String]): Unit = { + val noAttr = + val attrNull = + val attrNone = + assert(noAttr == attrNull) + assert(noAttr == attrNone) + assert(noAttr.toString() == "") + assert(attrNull.toString() == "") + assert(attrNone.toString() == "") + } +} -- cgit v1.2.3 From 51089b34a7a535498dee42e6465d4d577d65b7d5 Mon Sep 17 00:00:00 2001 From: Szabolcs Berecz Date: Sat, 7 Jan 2012 18:23:21 +0100 Subject: Accept prefixed xml attributes with null value This changes makes PrefixedAttribute work the same way as UnprefixedAttribute with respect to null values: is accepted and results in --- src/library/scala/xml/PrefixedAttribute.scala | 15 +++++++++------ test/files/run/xml-attribute.scala | 25 ++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/library/scala/xml/PrefixedAttribute.scala b/src/library/scala/xml/PrefixedAttribute.scala index 436dfcda43..b80d6a1c73 100644 --- a/src/library/scala/xml/PrefixedAttribute.scala +++ b/src/library/scala/xml/PrefixedAttribute.scala @@ -13,22 +13,25 @@ package scala.xml * * @param pre ... * @param key ... - * @param value the attribute value, which may not be null + * @param value the attribute value * @param next ... */ class PrefixedAttribute( val pre: String, val key: String, val value: Seq[Node], - val next: MetaData) + val next1: MetaData) extends Attribute { - if (value eq null) - throw new UnsupportedOperationException("value is null") + val next = if (value ne null) next1 else next1.remove(key) - /** same as this(key, Utility.parseAttributeValue(value), next) */ + /** same as this(pre, key, Text(value), next), or no attribute if value is null */ def this(pre: String, key: String, value: String, next: MetaData) = - this(pre, key, Text(value), next) + this(pre, key, if (value ne null) Text(value) else null: NodeSeq, next) + + /** same as this(pre, key, value.get, next), or no attribute if value is None */ + def this(pre: String, key: String, value: Option[Seq[Node]], next: MetaData) = + this(pre, key, value.orNull, next) /** Returns a copy of this unprefixed attribute with the given * next field. diff --git a/test/files/run/xml-attribute.scala b/test/files/run/xml-attribute.scala index 2b83f70b22..8b261acc94 100644 --- a/test/files/run/xml-attribute.scala +++ b/test/files/run/xml-attribute.scala @@ -5,10 +5,29 @@ object Test { val noAttr = val attrNull = val attrNone = + val preAttrNull = + val preAttrNone = assert(noAttr == attrNull) assert(noAttr == attrNone) - assert(noAttr.toString() == "") - assert(attrNull.toString() == "") - assert(attrNone.toString() == "") + assert(noAttr == preAttrNull) + assert(noAttr == preAttrNone) + + val noAttrStr = "" + assert(noAttr.toString() == noAttrStr) + assert(attrNull.toString() == noAttrStr) + assert(attrNone.toString() == noAttrStr) + assert(preAttrNull.toString() == noAttrStr) + assert(preAttrNone.toString() == noAttrStr) + + val xml1 = + val xml2 = + val xml3 = + assert(xml1 == xml2) + assert(xml1 == xml3) + + val xml1Str = "" + assert(xml1.toString() == xml1Str) + assert(xml2.toString() == xml1Str) + assert(xml3.toString() == xml1Str) } } -- cgit v1.2.3