From 9a86215c18a4eacd2ee1218fa01f458e415a8e7f Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Thu, 4 Aug 2011 02:14:06 +0000 Subject: Cleanups in Namers and AddInterfaces emerging f... Cleanups in Namers and AddInterfaces emerging from bugfixing attempts and comprehension pursuits. I appear to have accidentally fixed at least one bug, as there are new (correct) warnings when building the compiler involving permanently hidden imports. No review. --- .../scala/tools/nsc/transform/AddInterfaces.scala | 98 +++++------ .../scala/tools/nsc/typechecker/Namers.scala | 185 +++++++++++---------- .../tools/nsc/typechecker/TypeDiagnostics.scala | 26 ++- .../scala/tools/nsc/typechecker/Typers.scala | 2 +- test/files/neg/t2773.check | 2 +- 5 files changed, 149 insertions(+), 164 deletions(-) diff --git a/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala b/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala index c467ed9445..6ec55b2187 100644 --- a/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala +++ b/src/compiler/scala/tools/nsc/transform/AddInterfaces.scala @@ -15,14 +15,9 @@ abstract class AddInterfaces extends InfoTransform { import global._ // the global environment import definitions._ // standard classes and methods - /**

- * The phase sets lateINTERFACE for non-interface traits - * that now become interfaces. - *

- *

- * It sets lateDEFERRED for formerly concrete methods in - * such traits. - *

+ /** The phase sets lateINTERFACE for non-interface traits that now + * become interfaces. It sets lateDEFERRED for formerly concrete + * methods in such traits. */ override def phaseNewFlags: Long = lateDEFERRED | lateINTERFACE @@ -52,23 +47,32 @@ abstract class AddInterfaces extends InfoTransform { } /** Is given trait member symbol a member of the trait's interface - * after this transform is performed? */ - private def isInterfaceMember(sym: Symbol): Boolean = { - sym.isType || - { sym.info; // to set lateMETHOD flag if necessary - sym.isMethod && - !sym.isLabel && - !sym.isPrivate && - (!(sym hasFlag BRIDGE) || sym.hasBridgeAnnotation) && // count @_$bridge$_ annotated classes as interface members - !sym.isConstructor && - !sym.isImplOnly + * after this transform is performed? + */ + private def isInterfaceMember(sym: Symbol) = ( + sym.isType || { + // !!! Shouldn't the following code be equivalent to leaving + // out the "sym.info" call and starting with "sym.initialize.isMethod" ? + // Because, it is not, which I found a little disturbing. The compiler + // fails to bootstrap with an error somewhere. + sym.info // initialize to set lateMETHOD flag if necessary + + ( sym.isMethod + && !sym.isLabel + && !sym.isPrivate + && (!(sym hasFlag BRIDGE) || sym.hasBridgeAnnotation) // count @bridge annotated classes as interface members + && !sym.isConstructor + && !sym.isImplOnly + ) } - } + ) /** Does symbol need an implementation method? */ - private def needsImplMethod(sym: Symbol): Boolean = - sym.isMethod && isInterfaceMember(sym) && - (!(sym hasFlag (DEFERRED | SUPERACCESSOR)) || (sym hasFlag lateDEFERRED)) + private def needsImplMethod(sym: Symbol) = ( + sym.isMethod + && isInterfaceMember(sym) + && (!sym.hasFlag(DEFERRED | SUPERACCESSOR) || (sym hasFlag lateDEFERRED)) + ) def implClassPhase = currentRun.erasurePhase.next @@ -99,39 +103,20 @@ abstract class AddInterfaces extends InfoTransform { } }) - /**

- * A lazy type to set the info of an implementation class - * The parents of an implementation class for trait iface are: - *

- * - *

- * The declarations of a mixin class are: - *

- * + /** A lazy type to set the info of an implementation class + * The parents of an implementation class for trait iface are: + * + * - superclass: Object + * - mixin classes: mixin classes of iface where every non-interface + * trait is mapped to its implementation class, followed by iface itself. + * + * The declarations of a mixin class are: + * - for every interface member of iface: its implementation method, if one is needed + * - every former member of iface that is implementation only */ private class LazyImplClassType(iface: Symbol) extends LazyType { - - /** Compute the decls of implementation class implClass, - * given the decls ifaceDecls of its interface. - * - * @param implClass ... - * @param ifaceDecls ... - * @return ... + /** Compute the decls of implementation class implClass, + * given the decls ifaceDecls of its interface. */ private def implDecls(implClass: Symbol, ifaceDecls: Scope): Scope = { val decls = new Scope @@ -274,9 +259,8 @@ abstract class AddInterfaces extends InfoTransform { } /** Add calls to supermixin constructors - * 
super[mix].$init$()
- * to tree. tree which is assumed to be the body - * of a constructor of class clazz. + * `super[mix].$init$()` + * to tree, which is assumed to be the body of a constructor of class clazz. */ private def addMixinConstructorCalls(tree: Tree, clazz: Symbol): Tree = { def mixinConstructorCall(impl: Symbol): Tree = atPos(tree.pos) { @@ -291,7 +275,7 @@ abstract class AddInterfaces extends InfoTransform { case Block(stats, expr) => // needs `hasSymbol` check because `supercall` could be a block (named / default args) val (presuper, supercall :: rest) = stats span (t => t.hasSymbolWhich(_ hasFlag PRESUPER)) - //assert(supercall.symbol.isClassConstructor, supercall) + // assert(supercall.symbol.isClassConstructor, supercall) treeCopy.Block(tree, presuper ::: (supercall :: mixinConstructorCalls ::: rest), expr) } } diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index dde34284b7..d7b665e214 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -152,12 +152,11 @@ trait Namers { self: Analyzer => private def setInfo[Sym <: Symbol](sym : Sym)(tpe : LazyType) : Sym = sym.setInfo(tpe) private def doubleDefError(pos: Position, sym: Symbol) { - context.error(pos, - sym.name.toString() + " is already defined as " + - (if (sym.isSynthetic) - "(compiler-generated) "+ (if (sym.isModule) "case class companion " else "") - else "") + - (if (sym.isCase) "case class " + sym.name else sym.toString())) + val s1 = if (sym.isModule) "case class companion " else "" + val s2 = if (sym.isSynthetic) "(compiler-generated) " + s1 else "" + val s3 = if (sym.isCase) "case class " + sym.name else "" + sym + + context.error(pos, sym.name + " is already defined as " + s2 + s3) } private def inCurrentScope(m: Symbol): Boolean = { @@ -177,9 +176,9 @@ trait Namers { self: Analyzer => doubleDefError(sym.pos, prev.sym) sym setInfo ErrorType scope unlink prev.sym // let them co-exist... - scope enter sym - } else scope enter sym - } else scope enter sym + } + } + scope enter sym } def enterPackageSymbol(pos: Position, pid: RefTree, pkgOwner: Symbol): Symbol = { @@ -233,8 +232,7 @@ trait Namers { self: Analyzer => // .pos, mods.flags | MODULE | FINAL, name var m: Symbol = context.scope.lookup(tree.name) val moduleFlags = tree.mods.flags | MODULE | FINAL - if (m.isModule && !m.isPackage && inCurrentScope(m) && - (currentRun.canRedefine(m) || m.isSynthetic)) { + if (m.isModule && !m.isPackage && inCurrentScope(m) && (currentRun.canRedefine(m) || m.isSynthetic)) { updatePosFlags(m, tree.pos, moduleFlags) setPrivateWithin(tree, m, tree.mods) if (m.moduleClass != NoSymbol) @@ -274,8 +272,7 @@ trait Namers { self: Analyzer => sym setInfo sym.deSkolemize.info.substSym(tparams, tskolems) //@M the info of a skolem is the skolemized info of the actual type parameter of the skolem } } - tskolems foreach (_.setInfo(ltp)) - tskolems + tskolems map (_ setInfo ltp) } /** Replace type parameters with their TypeSkolems, which can later be deskolemized to the original type param @@ -295,13 +292,77 @@ trait Namers { self: Analyzer => * class definition tree. * @return the companion object symbol. */ - def ensureCompanionObject(tree: ClassDef, creator: => Tree): Symbol = { - val m = companionModuleOf(tree.symbol, context) - // @luc: not sure why "currentRun.compiles(m)" is needed, things breaks - // otherwise. documentation welcome. - if (m != NoSymbol && currentRun.compiles(m)) m - else enterSyntheticSym(creator) - } + def ensureCompanionObject(tree: ClassDef, creator: => Tree): Symbol = { + val m = companionModuleOf(tree.symbol, context) + // @luc: not sure why "currentRun.compiles(m)" is needed, things breaks + // otherwise. documentation welcome. + if (m != NoSymbol && currentRun.compiles(m)) m + else enterSyntheticSym(creator) + } + + private def checkSelectors(tree: Import): Unit = { + val Import(expr, selectors) = tree + val base = expr.tpe + + def isValid(from: Name) = + from.bothNames forall (x => (base nonLocalMember x) == NoSymbol) + + def checkNotRedundant(pos: Position, from: Name, to0: Name) { + def warnRedundant(sym: Symbol) = { + context.unit.warning(pos, + "imported `"+to0+"' is permanently hidden by definition of "+sym.fullLocationString + ) + } + def check(to: Name) = { + val e = context.scope.lookupEntry(to) + if (e != null && e.owner == context.scope && e.sym.exists) + warnRedundant(e.sym) + else if (context ne context.enclClass) { + val defSym = context.prefix.member(to) filter ( + sym => sym.exists && context.isAccessible(sym, context.prefix, false)) + + if (defSym != NoSymbol) + warnRedundant(defSym) + } + } + if (tree.symbol.isSynthetic || expr.symbol == null || expr.symbol.isInterpreterWrapper) () + else { + if (base.member(from) != NoSymbol) + check(to0) + if (base.member(from.toTypeName) != NoSymbol) + check(to0.toTypeName) + } + } + def checkSelector(s: ImportSelector) = { + val ImportSelector(from, _, to, _) = s + if (from != nme.WILDCARD && base != ErrorType) { + if (isValid(from)) { + if (currentRun.compileSourceFor(expr, from)) { + // side effecting, apparently + typeSig(tree) + } + // for Java code importing Scala objects + else if (!nme.isModuleName(from) || isValid(nme.stripModuleSuffix(from))) + notAMemberError(tree.pos, expr, from) + } + checkNotRedundant(tree.pos, from, to) + } + } + def noDuplicates(names: List[Name], message: String) { + def loop(xs: List[Name]): Unit = xs match { + case Nil => () + case hd :: tl => + if (hd == nme.WILDCARD || !(tl contains hd)) loop(tl) + else context.error(tree.pos, hd.decode + " " + message) + } + loop(names filterNot (x => x == null || x == nme.WILDCARD)) + } + selectors foreach checkSelector + + // checks on the whole set + noDuplicates(selectors map (_.name), "is renamed twice") + noDuplicates(selectors map (_.rename), "appears twice as a target of a renaming") + } private def enterSymFinishWith(tree: Tree, tparams: List[TypeDef]) { val sym = tree.symbol @@ -795,9 +856,8 @@ trait Namers { self: Analyzer => // if default getters (for constructor defaults) need to be added to that module, here's the namer // to use. clazz is the ModuleClass. sourceModule works also for classes defined in methods. val module = clazz.sourceModule - classAndNamerOfModule get module match { - case Some((cdef, _)) => classAndNamerOfModule(module) = (cdef, templateNamer) - case None => + classAndNamerOfModule get module foreach { + case (cdef, _) => classAndNamerOfModule(module) = (cdef, templateNamer) } if (opt.verbose) { @@ -817,6 +877,7 @@ trait Namers { self: Analyzer => private def methodSig(mods: Modifiers, tparams: List[TypeDef], vparamss: List[List[ValDef]], tpt: Tree, rhs: Tree): Type = { val meth = context.owner + val isJavaMethod = meth.isJavaDefined // enters the skolemized version into scope, returns the deSkolemized symbols val tparamSyms = typer.reenterTypeParams(tparams) @@ -829,7 +890,6 @@ trait Namers { self: Analyzer => tpt setPos meth.pos.focus } - /** Called for all value parameter lists, right to left * @param vparams the symbols of one parameter list * @param restpe the result type (possibly a MethodType) @@ -841,9 +901,9 @@ trait Namers { self: Analyzer => // check that params only depend on ones in earlier sections, not the same. (done by checkDependencies, // so re-use / adapt that) val params = vparams map (vparam => - if (meth hasFlag JAVA) vparam.setInfo(objToAny(vparam.tpe)) else vparam) + if (isJavaMethod) vparam.setInfo(objToAny(vparam.tpe)) else vparam) // TODODEPMET necessary?? new dependent types: replace symbols in restpe with the ones in vparams - if (meth hasFlag JAVA) JavaMethodType(params, restpe) + if (isJavaMethod) JavaMethodType(params, restpe) else MethodType(params, restpe) } @@ -1216,69 +1276,16 @@ trait Namers { self: Analyzer => case Import(expr, selectors) => val expr1 = typer.typedQualifier(expr) - val base = expr1.tpe - typer.checkStable(expr1) - if ((expr1.symbol ne null) && expr1.symbol.isRootPackage) context.error(tree.pos, "_root_ cannot be imported") - def checkNotRedundant(pos: Position, from: Name, to: Name): Boolean = { - if (!tree.symbol.isSynthetic && - !((expr1.symbol ne null) && expr1.symbol.isInterpreterWrapper) && - base.member(from) != NoSymbol) { - val e = context.scope.lookupEntry(to) - def warnRedundant(sym: Symbol) = - context.unit.warning(pos, "imported `"+to+ - "' is permanently hidden by definition of "+sym+ - sym.locationString) - if ((e ne null) && e.owner == context.scope && e.sym.exists) { - warnRedundant(e.sym); return false - } else if (context eq context.enclClass) { - val defSym = context.prefix.member(to) filter ( - sym => sym.exists && context.isAccessible(sym, context.prefix, false)) - if (defSym != NoSymbol) { warnRedundant(defSym); return false } - } - } - true - } - - def isValidSelector(from: Name)(fun : => Unit) { - if (from.bothNames forall (x => (base nonLocalMember x) == NoSymbol)) - fun - } - - def checkSelectors(selectors: List[ImportSelector]): Unit = selectors match { - case ImportSelector(from, _, to, _) :: rest => - if (from != nme.WILDCARD && base != ErrorType) { - isValidSelector(from) { - if (currentRun.compileSourceFor(expr, from)) { - // XXX This used to be "return typeSig(tree)" but since this method - // returns Unit, that is deceptive at best. Just in case it is side-effecting - // somehow, I left the call in before the return; if you know it is - // not side effecting, please delete the call. - typeSig(tree) - return - } - - def notMember() = context.error(tree.pos, from.decode + " is not a member of " + expr) - // for Java code importing Scala objects - if (nme.isModuleName(from)) - isValidSelector(nme.stripModuleSuffix(from))(notMember()) - else - notMember() - } - - if (checkNotRedundant(tree.pos, from, to)) - checkNotRedundant(tree.pos, from.toTypeName, to.toTypeName) - } - if (from != nme.WILDCARD && (rest.exists (sel => sel.name == from))) - context.error(tree.pos, from.decode + " is renamed twice") - if ((to ne null) && to != nme.WILDCARD && (rest exists (sel => sel.rename == to))) - context.error(tree.pos, to.decode + " appears twice as a target of a renaming") - checkSelectors(rest) - case Nil => - } - checkSelectors(selectors) - transformed(tree) = treeCopy.Import(tree, expr1, selectors) - expr.symbol = expr1.symbol // copy symbol and type attributes back into old expression - // so that the structure builder will find it. + typer checkStable expr1 + if (expr1.symbol != null && expr1.symbol.isRootPackage) + context.error(tree.pos, "_root_ cannot be imported") + + val newImport = treeCopy.Import(tree, expr1, selectors).asInstanceOf[Import] + checkSelectors(newImport) + transformed(tree) = newImport + // copy symbol and type attributes back into old expression + // so that the structure builder will find it. + expr.symbol = expr1.symbol expr.tpe = expr1.tpe ImportType(expr1) } diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index 81caf38762..513522e017 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala @@ -37,16 +37,10 @@ trait TypeDiagnostics { import global._ import definitions._ - import global.typer.infer + import global.typer.{ infer, context } private def currentUnit = currentRun.currentUnit - /** It can be quite difficult to know which of the many functions called "error" - * is being called at any given point in the compiler. To alleviate this I am - * renaming such functions inside this trait based on where it originated. - */ - def inferError(pos: Position, msg: String) = infer.error(pos, msg) - /** The common situation of making sure nothing is erroneous could be * nicer if Symbols, Types, and Trees all implemented some common interface * in which isErroneous and similar would be placed. @@ -85,9 +79,10 @@ trait TypeDiagnostics { /** Does the positioned line assigned to t1 precede that of t2? */ - def linePrecedes(t1: Tree, t2: Tree) = t1.pos.isDefined && t1.pos.isDefined && t1.pos.line < t2.pos.line + def posPrecedes(p1: Position, p2: Position) = p1.isDefined && p2.isDefined && p1.line < p2.line + def linePrecedes(t1: Tree, t2: Tree) = posPrecedes(t1.pos, t2.pos) - def notAMember(sel: Tree, qual: Tree, name: Name) = { + def notAMemberMessage(pos: Position, qual: Tree, name: Name) = { val owner = qual.tpe.typeSymbol val target = qual.tpe.widen def targetKindString = if (owner.isTypeParameterOrSkolem) "type parameter " else "" @@ -104,7 +99,7 @@ trait TypeDiagnostics { else "" } val semicolon = ( - if (linePrecedes(qual, sel)) + if (posPrecedes(qual.pos, pos)) "\npossible cause: maybe a semicolon is missing before `"+nameString+"'?" else "" @@ -112,14 +107,13 @@ trait TypeDiagnostics { companion + semicolon } - inferError( - sel.pos, - withAddendum(qual.pos)( - if (name == nme.CONSTRUCTOR) target + " does not have a constructor" - else nameString + " is not a member of " + targetKindString + target + addendum - ) + withAddendum(qual.pos)( + if (name == nme.CONSTRUCTOR) target + " does not have a constructor" + else nameString + " is not a member of " + targetKindString + target + addendum ) } + def notAMemberError(pos: Position, qual: Tree, name: Name) = + context.error(pos, notAMemberMessage(pos, qual, name)) /** Only prints the parameter names if they're not synthetic, * since "x$1: Int" does not offer any more information than "Int". diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index e0639cb6cf..d8c09cc685 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -3632,7 +3632,7 @@ trait Typers extends Modes with Adaptations { return makeErrorTree if (!qual.tpe.widen.isErroneous) - notAMember(tree, qual, name) + notAMemberError(tree.pos, qual, name) if (forInteractive) makeErrorTree else setError(tree) } else { diff --git a/test/files/neg/t2773.check b/test/files/neg/t2773.check index 6e88762144..a5ffb5fbd5 100644 --- a/test/files/neg/t2773.check +++ b/test/files/neg/t2773.check @@ -1,4 +1,4 @@ -t2773.scala:5: error: x is not a member of c +t2773.scala:5: error: value x is not a member of C import c.x ^ t2773.scala:6: error: not found: value x -- cgit v1.2.3