From 5eeb2a3b439419e1f6b17925c439346a3cab017d Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Mon, 19 Oct 2009 18:40:17 +0000 Subject: A new and better fix for ticket #1373. --- src/compiler/scala/tools/nsc/ast/Trees.scala | 1 + src/compiler/scala/tools/nsc/symtab/Symbols.scala | 42 ++++++++++++++++++---- .../tools/nsc/transform/SpecializeTypes.scala | 8 ++--- .../scala/tools/nsc/typechecker/Namers.scala | 36 ++++++++++--------- .../tools/nsc/typechecker/SyntheticMethods.scala | 15 ++++---- test/files/run/bug1373.scala | 6 ++++ 6 files changed, 73 insertions(+), 35 deletions(-) create mode 100644 test/files/run/bug1373.scala diff --git a/src/compiler/scala/tools/nsc/ast/Trees.scala b/src/compiler/scala/tools/nsc/ast/Trees.scala index 7da71eaed6..3337366269 100644 --- a/src/compiler/scala/tools/nsc/ast/Trees.scala +++ b/src/compiler/scala/tools/nsc/ast/Trees.scala @@ -64,6 +64,7 @@ trait Trees { def isAbstract = hasFlag(ABSTRACT ) def isDeferred = hasFlag(DEFERRED ) def isCase = hasFlag(CASE ) + def isLazy = hasFlag(LAZY ) def isSealed = hasFlag(SEALED ) def isFinal = hasFlag(FINAL ) def isTrait = hasFlag(TRAIT | notDEFERRED) // (part of DEVIRTUALIZE) diff --git a/src/compiler/scala/tools/nsc/symtab/Symbols.scala b/src/compiler/scala/tools/nsc/symtab/Symbols.scala index 0003a50823..94d1764d8c 100644 --- a/src/compiler/scala/tools/nsc/symtab/Symbols.scala +++ b/src/compiler/scala/tools/nsc/symtab/Symbols.scala @@ -1078,10 +1078,38 @@ trait Symbols { */ def thisType: Type = NoPrefix - /** Return every accessor of a primary constructor parameter in this case class + /** Return every accessor of a primary constructor parameter in this case class. + * The scope declarations may be out of order because fields with less than private + * access are first given a regular getter, then a new renamed getter which comes + * later in the declaration list. For this reason we have to pinpoint the + * right accessors by starting with the original fields (which will be in the right + * order) and looking for getters with applicable names. The getters may have the + * standard name "foo" or may have been renamed to "foo$\d+" in SyntheticMethods. + * See ticket #1373. */ - final def caseFieldAccessors: List[Symbol] = - info.decls.toList filter (sym => !(sym hasFlag PRIVATE) && sym.hasFlag(CASEACCESSOR)) + final def caseFieldAccessors: List[Symbol] = { + val allWithFlag = info.decls.toList filter (_ hasFlag CASEACCESSOR) + val (accessors, fields) = allWithFlag partition (_.isMethod) + + def findAccessor(field: Symbol): Symbol = { + // There is another renaming the field may have undergone, for instance as in + // ticket #2175: case class Property[T](private var t: T), t becomes Property$$t. + // So we use the original name everywhere. + val getterName = nme.getterName(field.originalName) + + // Note this is done in two passes intentionally, to ensure we pick up the original + // getter if present before looking for the renamed getter. + def origGetter = accessors find (_.originalName == getterName) + def renamedGetter = accessors find (_.originalName startsWith (getterName + "$")) + val accessorName = origGetter orElse renamedGetter + + accessorName getOrElse { + throw new Error("Could not find case accessor for %s in %s".format(field, this)) + } + } + + fields map findAccessor + } final def constrParamAccessors: List[Symbol] = info.decls.toList filter (sym => !sym.isMethod && sym.hasFlag(PARAMACCESSOR)) @@ -1117,7 +1145,7 @@ trait Symbols { def defaultGetter_=(getter: Symbol): Unit = throw new Error("defaultGetter cannot be set for " + this) - /** For a lazy value, it's lazy accessor. NoSymbol for all others */ + /** For a lazy value, its lazy accessor. NoSymbol for all others */ def lazyAccessor: Symbol = NoSymbol /** For an outer accessor: The class from which the outer originates. @@ -1586,15 +1614,17 @@ trait Symbols { clone } + private val validAliasFlags = SUPERACCESSOR | PARAMACCESSOR | MIXEDIN | SPECIALIZED + override def alias: Symbol = - if (hasFlag(SUPERACCESSOR | PARAMACCESSOR | MIXEDIN | SPECIALIZED)) initialize.referenced + if (hasFlag(validAliasFlags)) initialize.referenced else NoSymbol def setAlias(alias: Symbol): TermSymbol = { assert(alias != NoSymbol, this) assert(!(alias hasFlag OVERLOADED), alias) - assert(hasFlag(SUPERACCESSOR | PARAMACCESSOR | MIXEDIN | SPECIALIZED), this) + assert(hasFlag(validAliasFlags), this) referenced = alias this } diff --git a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala index 2ee768b592..aeb54ea02d 100644 --- a/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala +++ b/src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala @@ -430,11 +430,9 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers { log(" * looking at: " + m) if (!m.isDeferred) concreteSpecMethods += m - // specialized members have to be overridable. Fields should not - // have the field CASEACCESSOR (messes up patmatch) - if (m.hasFlag(PRIVATE)) { - m.resetFlag(PRIVATE | CASEACCESSOR).setFlag(PROTECTED) - } + // specialized members have to be overridable. + if (m.hasFlag(PRIVATE)) + m.resetFlag(PRIVATE).setFlag(PROTECTED) if (m.isConstructor) { val specCtor = enterMember(m.cloneSymbol(cls).setFlag(SPECIALIZED)) diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index f31394407e..91b0facd92 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -127,13 +127,13 @@ trait Namers { self: Analyzer => unsafeTypeParams foreach(sym => paramContext.scope.enter(sym)) newNamer(paramContext) } - if (sym.isTerm) { - if (sym.hasFlag(PARAM) && sym.owner.isPrimaryConstructor) - primaryConstructorParamNamer - else if (sym.hasFlag(PARAMACCESSOR)) - primaryConstructorParamNamer - else innerNamer - } else innerNamer + def usePrimary = sym.isTerm && ( + (sym hasFlag PARAMACCESSOR) || + ((sym hasFlag PARAM) && sym.owner.isPrimaryConstructor) + ) + + if (usePrimary) primaryConstructorParamNamer + else innerNamer } protected def conflict(newS : Symbol, oldS : Symbol) : Boolean = { @@ -339,7 +339,7 @@ trait Namers { self: Analyzer => case tree @ ClassDef(mods, name, tparams, impl) => tree.symbol = enterClassSymbol(tree) finishWith(tparams) - if ((mods.flags & CASE) != 0L) { + if (mods.isCase) { val m = ensureCompanionObject(tree, caseModuleDef(tree)) caseClassOfModuleClass(m.moduleClass) = tree } @@ -368,7 +368,7 @@ trait Namers { self: Analyzer => (mods.flags & (PRIVATE | LOCAL)) == (PRIVATE | LOCAL).toLong || name.endsWith(nme.OUTER, nme.OUTER.length) || context.unit.isJava) && - (mods.flags & LAZY) == 0L) { + !mods.isLazy) { tree.symbol = enterInScope(owner.newValue(tree.pos, name) .setFlag(mods.flags)) finish @@ -389,22 +389,24 @@ trait Namers { self: Analyzer => setInfo(setter)(namerOf(setter).setterTypeCompleter(vd)) } tree.symbol = - if (mods.hasFlag(DEFERRED)) { + if (mods.isDeferred) { getter setPos tree.pos // unfocus getter position, because there won't be a separate value } else { var vsym = if (!context.owner.isClass) { - assert((mods.flags & LAZY) != 0L) // if not a field, it has to be a lazy val + assert(mods.isLazy) // if not a field, it has to be a lazy val owner.newValue(tree.pos, name + "$lzy" ).setFlag(mods.flags | MUTABLE) } else { - owner.newValue(tree.pos, nme.getterToLocal(name)) - .setFlag(mods.flags & FieldFlags | PRIVATE | LOCAL | - (if (mods.hasFlag(LAZY)) MUTABLE else 0)) + val mflag = if (mods.isLazy) MUTABLE else 0 + val newflags = mods.flags & FieldFlags | PRIVATE | LOCAL | mflag + + owner.newValue(tree.pos, nme.getterToLocal(name)) setFlag newflags } enterInScope(vsym) setInfo(vsym)(namerOf(vsym).typeCompleter(tree)) - if ((mods.flags & LAZY) != 0L) + if (mods.isLazy) vsym.setLazyAccessor(getter) + vsym } addBeanGetterSetter(vd, getter) @@ -472,7 +474,7 @@ trait Namers { self: Analyzer => if (!name(0).isLetter) context.error(vd.pos, "`BeanProperty' annotation can be applied "+ "only to fields that start with a letter") - else if (mods hasFlag PRIVATE) + else if (mods.isPrivate) // avoids name clashes with private fields in traits context.error(vd.pos, "`BeanProperty' annotation can only be applied "+ "to non-private fields") @@ -485,7 +487,7 @@ trait Namers { self: Analyzer => val getterMods = Modifiers(flags, mods.privateWithin, Nil, mods.positions) val beanGetterDef = atPos(vd.pos.focus) { DefDef(getterMods, getterName, Nil, List(Nil), tpt.duplicate, - if (mods hasFlag DEFERRED) EmptyTree + if (mods.isDeferred) EmptyTree else Select(This(getter.owner.name), name)) } enterSyntheticSym(beanGetterDef) diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala index a1c1a7b661..4fa66a242b 100644 --- a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala +++ b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala @@ -204,8 +204,6 @@ trait SyntheticMethods extends ast.TreeDSL { def hasSerializableAnnotation(clazz: Symbol) = clazz hasAnnotation SerializableAttr - def isPublic(sym: Symbol) = - !sym.hasFlag(PRIVATE | PROTECTED) && sym.privateWithin == NoSymbol def readResolveMethod: Tree = { // !!! the synthetic method "readResolve" should be private, but then it is renamed !!! @@ -233,11 +231,14 @@ trait SyntheticMethods extends ast.TreeDSL { clazz addAnnotation AnnotationInfo(SerializableAttr.tpe, Nil, Nil) if (isTop) { - for (stat <- templ.body) { - if (stat.isDef && stat.symbol.isMethod && stat.symbol.hasFlag(CASEACCESSOR) && !isPublic(stat.symbol)) { - ts += newAccessorMethod(stat) - stat.symbol resetFlag CASEACCESSOR - } + // If this case class has fields with less than public visibility, their getter at this + // point also has those permissions. In that case we create a new, public accessor method + // with a new name and remove the CASEACCESSOR flag from the existing getter. This complicates + // the retrieval of the case field accessors (see def caseFieldAccessors in Symbols.) + def needsService(s: Symbol) = s.isMethod && (s hasFlag CASEACCESSOR) && !s.isPublic + for (stat <- templ.body ; if stat.isDef && needsService(stat.symbol)) { + ts += newAccessorMethod(stat) + stat.symbol resetFlag CASEACCESSOR } } diff --git a/test/files/run/bug1373.scala b/test/files/run/bug1373.scala new file mode 100644 index 0000000000..537421c788 --- /dev/null +++ b/test/files/run/bug1373.scala @@ -0,0 +1,6 @@ +// Testing whether case class params come back in the right order. +object Test extends Application { + case class Foo(private val a: String, b: String, private val c: String, d: String, private val e: String) + val x = Foo("a", "b", "c", "d", "e") + assert(x.toString == """Foo(a,b,c,d,e)""") +} \ No newline at end of file -- cgit v1.2.3