diff options
| -rw-r--r-- | src/compiler/scala/reflect/internal/Symbols.scala | 43 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/Namers.scala | 35 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala | 136 | ||||
| -rw-r--r-- | test/files/neg/t1422.check | 2 | ||||
| -rw-r--r-- | test/files/neg/t1422.scala | 2 |
5 files changed, 106 insertions, 112 deletions
diff --git a/src/compiler/scala/reflect/internal/Symbols.scala b/src/compiler/scala/reflect/internal/Symbols.scala index fb6d512243..82706d7265 100644 --- a/src/compiler/scala/reflect/internal/Symbols.scala +++ b/src/compiler/scala/reflect/internal/Symbols.scala @@ -487,6 +487,10 @@ trait Symbols extends api.Symbols { self: SymbolTable => def isLiftedMethod = isMethod && hasFlag(LIFTED) def isCaseClass = isClass && isCase + // unfortunately having the CASEACCESSOR flag does not actually mean you + // are a case accessor (you can also be a field.) + def isCaseAccessorMethod = isMethod && isCaseAccessor + /** Does this symbol denote the primary constructor of its enclosing class? */ final def isPrimaryConstructor = isConstructor && owner.primaryConstructor == this @@ -1275,39 +1279,12 @@ trait Symbols extends api.Symbols { self: SymbolTable => */ def thisType: Type = NoPrefix - /** 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] = { - val allWithFlag = info.decls.toList filter (_.isCaseAccessor) - 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 - - // This fails more gracefully rather than throw an Error as it used to because - // as seen in #2625, we can reach this point with an already erroneous tree. - accessorName getOrElse NoSymbol - // throw new Error("Could not find case accessor for %s in %s".format(field, this)) - } - - fields map findAccessor - } + /** For a case class, the symbols of the accessor methods, one for each + * argument in the first parameter list of the primary constructor. + * The empty list for all other classes. + */ + final def caseFieldAccessors: List[Symbol] = + info.decls filter (_.isCaseAccessorMethod) toList final def constrParamAccessors: List[Symbol] = info.decls.toList filter (sym => !sym.isMethod && sym.isParamAccessor) diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index d0f1722b1c..5073cd02ae 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -497,25 +497,30 @@ trait Namers { self: Analyzer => finish case vd @ ValDef(mods, name, tp, rhs) => - if ((!context.owner.isClass || - (mods.isPrivateLocal && !mods.isCaseAccessor) || - name.startsWith(nme.OUTER) || - context.unit.isJava) && - !mods.isLazy) { - val vsym = owner.newValue(tree.pos, name).setFlag(mods.flags); - if(context.unit.isJava) setPrivateWithin(tree, vsym, mods) // #3663 -- for Scala fields we assume private[this] + val needsNoAccessors = !mods.isLazy && ( + !context.owner.isClass + || (mods.isPrivateLocal && !mods.isCaseAccessor) + || name.startsWith(nme.OUTER) + || context.unit.isJava + ) + + if (needsNoAccessors) { + val vsym = owner.newValue(tree.pos, name).setFlag(mods.flags) + if (context.unit.isJava) setPrivateWithin(tree, vsym, mods) // #3663 -- for Scala fields we assume private[this] tree.symbol = enterInScope(vsym) finish - } else { - val mods1 = - if (mods.isPrivateLocal && !mods.isLazy) { - context.error(tree.pos, "private[this] not allowed for case class parameters") - mods &~ LOCAL - } else mods + } + else { + val mods1 = ( + if (mods.isPrivateLocal && !mods.isLazy) { + context.error(tree.pos, "private[this] not allowed for case class parameters") + mods &~ LOCAL + } + else mods + ) // add getter and possibly also setter if (nme.isSetterName(name)) context.error(tree.pos, "Names of vals or vars may not end in `_='") - // .isInstanceOf[..]: probably for (old) IDE hook. is this obsolete? val getter = enterAccessorMethod(tree, name, getterFlags(mods1.flags), mods1) setInfo(getter)(namerOf(getter).getterTypeCompleter(vd)) if (mods1.isMutable) { @@ -529,7 +534,7 @@ trait Namers { self: Analyzer => } else { val vsym = if (!context.owner.isClass) { - assert(mods1.isLazy) // if not a field, it has to be a lazy val + assert(mods1.isLazy, mods1) // if not a field, it has to be a lazy val owner.newValue(tree.pos, name + "$lzy" ).setFlag((mods1.flags | MUTABLE) & ~IMPLICIT) } else { val mFlag = if (mods1.isLazy) MUTABLE else 0 diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala index 89b6eb9759..95fb104f5f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala +++ b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala @@ -124,7 +124,6 @@ trait SyntheticMethods extends ast.TreeDSL { m setInfo infoFn(m) finishMethod(m, f) } - private def cloneInternal(original: Symbol, f: Symbol => Tree, name: Name): Tree = { val m = original.cloneSymbol(clazz) setPos clazz.pos.focus m.name = name @@ -199,8 +198,15 @@ trait SyntheticMethods extends ast.TreeDSL { ) import synthesizer._ - def accessors = clazz.caseFieldAccessors - def arity = accessors.size + val originalAccessors = clazz.caseFieldAccessors + // private ones will have been renamed -- make sure they are entered + // in the original order. + def accessors = clazz.caseFieldAccessors sortBy { acc => + originalAccessors indexWhere { orig => + (acc.name == orig.name) || (acc.name startsWith (orig.name + "$").toTermName) + } + } + val arity = accessors.size // If this is ProductN[T1, T2, ...], accessorLub is the lub of T1, T2, ..., . // !!! Hidden behind -Xexperimental due to bummer type inference bugs. // Refining from Iterator[Any] leads to types like @@ -299,28 +305,6 @@ trait SyntheticMethods extends ast.TreeDSL { argsBody } - def newAccessorMethod(ddef: DefDef): Tree = { - deriveMethod(ddef.symbol, name => context.unit.freshTermName(name + "$")) { newAcc => - makeMethodPublic(newAcc) - newAcc resetFlag (ACCESSOR | PARAMACCESSOR) - ddef.rhs.duplicate - } - } - - // A buffer collecting additional methods for the template body - val ts = new ListBuffer[Tree] - - def makeAccessorsPublic() { - // 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.isCaseAccessor && !s.isPublic - for (ddef @ DefDef(_, _, _, _, _, _) <- templ.body; if needsService(ddef.symbol)) { - ts += newAccessorMethod(ddef) - ddef.symbol resetFlag CASEACCESSOR - } - } /** The _1, _2, etc. methods to implement ProductN. */ def productNMethods = { @@ -355,48 +339,76 @@ trait SyntheticMethods extends ast.TreeDSL { // Object_equals -> (() => createMethod(Object_equals)(m => This(clazz) ANY_EQ methodArg(m, 0))) ) - def addReadResolve() { - /** If you serialize a singleton and then deserialize it twice, - * you will have two instances of your singleton, unless you implement - * the readResolve() method (see http://www.javaworld.com/javaworld/ - * jw-04-2003/jw-0425-designpatterns_p.html) - */ - if (clazz.isSerializable && !hasConcreteImpl(nme.readResolve)) { - // Aha, I finally decoded the original comment. - // This method should be generated as private, but apparently if it is, then - // it is name mangled afterward. (Wonder why that is.) So it's only protected. - // For sure special methods like "readResolve" should not be mangled. - ts += createMethod(nme.readResolve, Nil, ObjectClass.tpe) { m => - m setFlag PRIVATE - REF(clazz.sourceModule) - } - } - } + /** If you serialize a singleton and then deserialize it twice, + * you will have two instances of your singleton, unless you implement + * the readResolve() method (see http://www.javaworld.com/javaworld/ + * jw-04-2003/jw-0425-designpatterns_p.html) + */ - def synthesize() = { - if (clazz.isCase) { - makeAccessorsPublic() - val methods = if (clazz.isModuleClass) caseObjectMethods else caseClassMethods + // Only nested objects inside objects should get readResolve automatically. + // Otherwise, after de-serialization we get null references for lazy accessors + // (nested object -> lazy val + class def) since the bitmap gets serialized but + // the moduleVar not. + def needsReadResolve = ( + clazz.isModuleClass + && clazz.owner.isModuleClass + && clazz.isSerializable + && !hasConcreteImpl(nme.readResolve) + ) - for ((m, impl) <- methods ; if !hasOverridingImplementation(m)) - ts += impl() - } - // Only nested objects inside objects should get readResolve automatically. - // Otherwise, after de-serialization we get null references for lazy accessors - // (nested object -> lazy val + class def) since the bitmap gets serialized but - // the moduleVar not. - if (clazz.isModuleClass && clazz.owner.isModuleClass) - addReadResolve() + def synthesize(): List[Tree] = { + val methods = ( + if (!clazz.isCase) Nil + else if (clazz.isModuleClass) caseObjectMethods + else caseClassMethods + ) + def impls = for ((m, impl) <- methods ; if !hasOverridingImplementation(m)) yield impl() + def extras = ( + if (needsReadResolve) { + // Aha, I finally decoded the original comment. + // This method should be generated as private, but apparently if it is, then + // it is name mangled afterward. (Wonder why that is.) So it's only protected. + // For sure special methods like "readResolve" should not be mangled. + List(createMethod(nme.readResolve, Nil, ObjectClass.tpe)(m => { m setFlag PRIVATE ; REF(clazz.sourceModule) })) + } + else Nil + ) + + try impls ++ extras + catch { case _: TypeError if reporter.hasErrors => Nil } } - try synthesize() - catch { case _: TypeError if reporter.hasErrors => () } + /** If this case class has any less than public accessors, + * adds new accessors at the correct locations to preserve ordering. + * Note that this must be done before the other method synthesis + * because synthesized methods need refer to the new symbols. + * Care must also be taken to preserve the case accessor order. + */ + def caseTemplateBody(): List[Tree] = { + val lb = ListBuffer[Tree]() + def isRewrite(sym: Symbol) = sym.isCaseAccessorMethod && !sym.isPublic + + for (ddef @ DefDef(_, _, _, _, _, _) <- templ.body ; if isRewrite(ddef.symbol)) { + val original = ddef.symbol + val newAcc = deriveMethod(ddef.symbol, name => context.unit.freshTermName(name + "$")) { newAcc => + makeMethodPublic(newAcc) + newAcc resetFlag (ACCESSOR | PARAMACCESSOR) + ddef.rhs.duplicate + } + ddef.symbol resetFlag CASEACCESSOR + lb += logResult("case accessor new")(newAcc) + } - if (phase.id <= currentRun.typerPhase.id) { - treeCopy.Template(templ, templ.parents, templ.self, - if (ts.isEmpty) templ.body else templ.body ++ ts // avoid copying templ.body if empty - ) + lb ++= templ.body ++= synthesize() toList } - else templ + + if (phase.id > currentRun.typerPhase.id) templ + else treeCopy.Template(templ, templ.parents, templ.self, + if (clazz.isCase) caseTemplateBody() + else synthesize() match { + case Nil => templ.body // avoiding unnecessary copy + case ms => templ.body ++ ms + } + ) } } diff --git a/test/files/neg/t1422.check b/test/files/neg/t1422.check index 5931fcb049..4db64f1d49 100644 --- a/test/files/neg/t1422.check +++ b/test/files/neg/t1422.check @@ -1,4 +1,4 @@ t1422.scala:1: error: private[this] not allowed for case class parameters -case class A(private[this] val foo:String) +case class A(private[this] val foo:String) { } ^ one error found diff --git a/test/files/neg/t1422.scala b/test/files/neg/t1422.scala index 751f05a764..af308244cd 100644 --- a/test/files/neg/t1422.scala +++ b/test/files/neg/t1422.scala @@ -1 +1 @@ -case class A(private[this] val foo:String) +case class A(private[this] val foo:String) { } |