diff options
author | Grzegorz Kossakowski <grzegorz.kossakowski@gmail.com> | 2012-09-27 03:33:37 -0700 |
---|---|---|
committer | Grzegorz Kossakowski <grzegorz.kossakowski@gmail.com> | 2012-09-27 03:33:37 -0700 |
commit | 6ec0fe522256e48f8ccc0204d7c4ed63a34d9ede (patch) | |
tree | 8ef2d71c934e4c6a43c12cf23e7cca704c6cde16 /src | |
parent | 78879c4dab79c0da8c31a4f7210c46fc0bd892c7 (diff) | |
parent | 97ede5a64a60c739f03619f9c0d7e2bf88a97207 (diff) | |
download | scala-6ec0fe522256e48f8ccc0204d7c4ed63a34d9ede.tar.gz scala-6ec0fe522256e48f8ccc0204d7c4ed63a34d9ede.tar.bz2 scala-6ec0fe522256e48f8ccc0204d7c4ed63a34d9ede.zip |
Merge pull request #1383 from paulp/issue/5604
Issue/5604
Diffstat (limited to 'src')
4 files changed, 100 insertions, 49 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index c3fca13374..18e9ae620e 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -604,11 +604,18 @@ abstract class GenASM extends SubComponent with BytecodeWriters { val internalName = cachedJN.toString() val trackedSym = jsymbol(sym) reverseJavaName.get(internalName) match { - case None => + case Some(oldsym) if oldsym.exists && trackedSym.exists => + assert( + // In contrast, neither NothingClass nor NullClass show up bytecode-level. + (oldsym == trackedSym) || (oldsym == RuntimeNothingClass) || (oldsym == RuntimeNullClass), + s"""|Different class symbols have the same bytecode-level internal name: + | name: $internalName + | oldsym: ${oldsym.fullNameString} + | tracked: ${trackedSym.fullNameString} + """.stripMargin + ) + case _ => reverseJavaName.put(internalName, trackedSym) - case Some(oldsym) => - assert((oldsym == trackedSym) || (oldsym == RuntimeNothingClass) || (oldsym == RuntimeNullClass), // In contrast, neither NothingClass nor NullClass show up bytecode-level. - "how can getCommonSuperclass() do its job if different class symbols get the same bytecode-level internal name: " + internalName) } } diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index df8eb9c6b9..55ec8bead4 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -161,6 +161,9 @@ trait Namers extends MethodSynthesis { else innerNamer } + // FIXME - this logic needs to be thoroughly explained + // and justified. I know it's wrong with repect to package + // objects, but I think it's also wrong in other ways. protected def conflict(newS: Symbol, oldS: Symbol) = ( ( !oldS.isSourceMethod || nme.isSetterName(newS.name) @@ -188,6 +191,19 @@ trait Namers extends MethodSynthesis { /** Enter symbol into given scope and return symbol itself */ def enterInScope(sym: Symbol, scope: Scope): Symbol = { + // FIXME - this is broken in a number of ways. + // + // 1) If "sym" allows overloading, that is not itself sufficient to skip + // the check, because "prev.sym" also must allow overloading. + // + // 2) There is nothing which reconciles a package's scope with + // the package object's scope. This is the source of many bugs + // with e.g. defining a case class in a package object. When + // compiling against classes, the class symbol is created in the + // package and in the package object, and the conflict is undetected. + // There is also a non-deterministic outcome for situations like + // an object with the same name as a method in the package object. + // allow for overloaded methods if (!allowsOverload(sym)) { val prev = scope.lookupEntry(sym.name) diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 2344e71883..6e222459c9 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -605,23 +605,31 @@ trait Typers extends Modes with Adaptations with Tags { } /** Is `sym` defined in package object of package `pkg`? + * Since sym may be defined in some parent of the package object, + * we cannot inspect its owner only; we have to go through the + * info of the package object. However to avoid cycles we'll check + * what other ways we can before pushing that way. */ private def isInPackageObject(sym: Symbol, pkg: Symbol) = { - def isInPkgObj(sym: Symbol) = - !sym.owner.isPackage && { - sym.owner.isPackageObjectClass && - sym.owner.owner == pkg || - pkg.isInitialized && { - // need to be careful here to not get a cyclic reference during bootstrap - val pkgobj = pkg.info.member(nme.PACKAGEkw) - pkgobj.isInitialized && - (pkgobj.info.member(sym.name).alternatives contains sym) - } + val pkgClass = if (pkg.isTerm) pkg.moduleClass else pkg + def matchesInfo = ( + pkg.isInitialized && { + // need to be careful here to not get a cyclic reference during bootstrap + val module = pkg.info member nme.PACKAGEkw + module.isInitialized && (module.info.member(sym.name).alternatives contains sym) } - pkg.isPackageClass && { + ) + def isInPkgObj(sym: Symbol) = ( + !sym.isPackage + && !sym.owner.isPackageClass + && (sym.owner ne NoSymbol) + && (sym.owner.owner == pkgClass || matchesInfo) + ) + + pkgClass.isPackageClass && ( if (sym.isOverloaded) sym.alternatives forall isInPkgObj else isInPkgObj(sym) - } + ) } /** Post-process an identifier or selection node, performing the following: @@ -4768,6 +4776,22 @@ trait Typers extends Modes with Adaptations with Tags { defSym = rootMirror.EmptyPackageClass.tpe.nonPrivateMember(name) defSym != NoSymbol } + def correctForPackageObject(sym: Symbol): Symbol = { + if (sym.isTerm && isInPackageObject(sym, pre.typeSymbol)) { + val sym1 = pre member sym.name + if ((sym1 eq NoSymbol) || (sym eq sym1)) sym else { + qual = gen.mkAttributedQualifier(pre) + log(s""" + | !!! Overloaded package object member resolved incorrectly. + | prefix: $pre + | Discarded: ${sym.defString} + | Using: ${sym1.defString} + """.stripMargin) + sym1 + } + } + else sym + } def startingIdentContext = ( // ignore current variable scope in patterns to enforce linearity if ((mode & (PATTERNmode | TYPEPATmode)) == 0) context @@ -4779,11 +4803,11 @@ trait Typers extends Modes with Adaptations with Tags { // which are methods (note: if we don't do that // case x :: xs in class List would return the :: method) // unless they are stable or are accessors (the latter exception is for better error messages). - def qualifies(sym: Symbol): Boolean = { - sym.hasRawInfo && // this condition avoids crashing on self-referential pattern variables - reallyExists(sym) && - ((mode & PATTERNmode | FUNmode) != (PATTERNmode | FUNmode) || !sym.isSourceMethod || sym.hasFlag(ACCESSOR)) - } + def qualifies(sym: Symbol): Boolean = ( + sym.hasRawInfo // this condition avoids crashing on self-referential pattern variables + && reallyExists(sym) + && ((mode & PATTERNmode | FUNmode) != (PATTERNmode | FUNmode) || !sym.isSourceMethod || sym.hasFlag(ACCESSOR)) + ) if (defSym == NoSymbol) { var defEntry: ScopeEntry = null // the scope entry of defSym, if defined in a local scope @@ -4791,32 +4815,17 @@ trait Typers extends Modes with Adaptations with Tags { var cx = startingIdentContext while (defSym == NoSymbol && cx != NoContext && (cx.scope ne null)) { // cx.scope eq null arises during FixInvalidSyms in Duplicators pre = cx.enclClass.prefix + // !!! FIXME. This call to lookupEntry is at the root of all the + // bad behavior with overloading in package objects. lookupEntry + // just takes the first symbol it finds in scope, ignoring the rest. + // When a selection on a package object arrives here, the first + // overload is always chosen. "correctForPackageObject" exists to + // undo that decision. Obviously it would be better not to do it in + // the first place; however other things seem to be tied to obtaining + // that ScopeEntry, specifically calculating the nesting depth. defEntry = cx.scope.lookupEntry(name) - if ((defEntry ne null) && qualifies(defEntry.sym)) { - // Right here is where SI-1987, overloading in package objects, can be - // seen to go wrong. There is an overloaded symbol, but when referring - // to the unqualified identifier from elsewhere in the package, only - // the last definition is visible. So overloading mis-resolves and is - // definition-order dependent, bad things. See run/t1987.scala. - // - // I assume the actual problem involves how/where these symbols are entered - // into the scope. But since I didn't figure out how to fix it that way, I - // catch it here by looking up package-object-defined symbols in the prefix. - if (isInPackageObject(defEntry.sym, pre.typeSymbol)) { - defSym = pre.member(defEntry.sym.name) - if (defSym ne defEntry.sym) { - qual = gen.mkAttributedQualifier(pre) - log(s""" - | !!! Overloaded package object member resolved incorrectly. - | prefix: $pre - | Discarded: ${defEntry.sym.defString} - | Using: ${defSym.defString} - """.stripMargin) - } - } - else - defSym = defEntry.sym - } + if ((defEntry ne null) && qualifies(defEntry.sym)) + defSym = correctForPackageObject(defEntry.sym) else { cx = cx.enclClass val foundSym = pre.member(name) filter qualifies diff --git a/src/reflect/scala/reflect/internal/TreeGen.scala b/src/reflect/scala/reflect/internal/TreeGen.scala index ebf0998573..c1753fc5a1 100644 --- a/src/reflect/scala/reflect/internal/TreeGen.scala +++ b/src/reflect/scala/reflect/internal/TreeGen.scala @@ -172,10 +172,29 @@ abstract class TreeGen extends macros.TreeBuilder { if (qual.symbol != null && (qual.symbol.isEffectiveRoot || qual.symbol.isEmptyPackage)) mkAttributedIdent(sym) else { + // Have to recognize anytime a selection is made on a package + // so it can be rewritten to foo.bar.`package`.name rather than + // foo.bar.name if name is in the package object. + // TODO - factor out the common logic between this and + // the Typers method "isInPackageObject", used in typedIdent. + val qualsym = ( + if (qual.tpe ne null) qual.tpe.typeSymbol + else if (qual.symbol ne null) qual.symbol + else NoSymbol + ) + val needsPackageQualifier = ( + (sym ne null) + && qualsym.isPackage + && !sym.isDefinedInPackage + ) val pkgQualifier = - if (sym != null && sym.owner.isPackageObjectClass && sym.effectiveOwner == qual.tpe.typeSymbol) { - val obj = sym.owner.sourceModule - Select(qual, nme.PACKAGE) setSymbol obj setType singleType(qual.tpe, obj) + if (needsPackageQualifier) { + // The owner of a symbol which requires package qualification may be the + // package object iself, but it also could be any superclass of the package + // object. In the latter case, we must go through the qualifier's info + // to obtain the right symbol. + val packageObject = if (sym.owner.isModuleClass) sym.owner.sourceModule else qual.tpe member nme.PACKAGE + Select(qual, nme.PACKAGE) setSymbol packageObject setType singleType(qual.tpe, packageObject) } else qual |