From c58647f5f25e9d99cb1890a32af4621c4d4645fa Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Sun, 6 Jan 2013 12:25:03 -0800 Subject: SI-6928, VerifyError with self reference to super. A bug in typers mishandled varargs. We should get more aggressive about eliminating all the ad hoc parameter/argument handling code spread everywhere. For varargs especially: any code which tries to make an adjustment based on a repeated parameter is more likely to be wrong than right. In aggregate these reinventions are a huge source of bugs. --- .../scala/tools/nsc/typechecker/Typers.scala | 91 +++++++++++----------- test/files/neg/t6928.check | 7 ++ test/files/neg/t6928.scala | 10 +++ test/files/run/t6928-run.check | 1 + test/files/run/t6928-run.scala | 10 +++ 5 files changed, 75 insertions(+), 44 deletions(-) create mode 100644 test/files/neg/t6928.check create mode 100644 test/files/neg/t6928.scala create mode 100644 test/files/run/t6928-run.check create mode 100644 test/files/run/t6928-run.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 9d390476db..1dce2c633d 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -2070,57 +2070,60 @@ trait Typers extends Modes with Adaptations with Tags { */ def computeParamAliases(clazz: Symbol, vparamss: List[List[ValDef]], rhs: Tree) { debuglog(s"computing param aliases for $clazz:${clazz.primaryConstructor.tpe}:$rhs") + val pending = ListBuffer[AbsTypeError]() + + // !!! This method is redundant with other, less buggy ones. def decompose(call: Tree): (Tree, List[Tree]) = call match { case Apply(fn, args) => - val (superConstr, args1) = decompose(fn) + // an object cannot be allowed to pass a reference to itself to a superconstructor + // because of initialization issues; SI-473, SI-3913, SI-6928. + foreachSubTreeBoundTo(args, clazz) { tree => + if (tree.symbol.isModule) + pending += SuperConstrReferenceError(tree) + tree match { + case This(qual) => + pending += SuperConstrArgsThisReferenceError(tree) + case _ => () + } + } + val (superConstr, preArgs) = decompose(fn) val params = fn.tpe.params - val args2 = if (params.isEmpty || !isRepeatedParamType(params.last.tpe)) args - else args.take(params.length - 1) :+ EmptyTree - assert(sameLength(args2, params) || call.isErrorTyped, "mismatch " + clazz + " " + (params map (_.tpe)) + " " + args2)//debug - (superConstr, args1 ::: args2) - case Block(stats, expr) if !stats.isEmpty => - decompose(stats.last) + // appending a dummy tree to represent Nil for an empty varargs (is this really necessary?) + val applyArgs = if (args.length < params.length) args :+ EmptyTree else args take params.length + + assert(sameLength(applyArgs, params) || call.isErrorTyped, + s"arity mismatch but call is not error typed: $clazz (params=$params, args=$applyArgs)") + + (superConstr, preArgs ::: applyArgs) + case Block(_ :+ superCall, _) => + decompose(superCall) case _ => - (call, List()) + (call, Nil) } val (superConstr, superArgs) = decompose(rhs) assert(superConstr.symbol ne null, superConstr)//debug - - val pending = ListBuffer[AbsTypeError]() - // an object cannot be allowed to pass a reference to itself to a superconstructor - // because of initialization issues; bug #473 - foreachSubTreeBoundTo(superArgs, clazz) { tree => - if (tree.symbol.isModule) - pending += SuperConstrReferenceError(tree) - tree match { - case This(qual) => - pending += SuperConstrArgsThisReferenceError(tree) - case _ => () - } - } - - if (superConstr.symbol.isPrimaryConstructor) { - val superClazz = superConstr.symbol.owner - if (!superClazz.isJavaDefined) { - val superParamAccessors = superClazz.constrParamAccessors - if (sameLength(superParamAccessors, superArgs)) { - for ((superAcc, superArg @ Ident(name)) <- superParamAccessors zip superArgs) { - if (vparamss.exists(_.exists(_.symbol == superArg.symbol))) { - var alias = superAcc.initialize.alias - if (alias == NoSymbol) - alias = superAcc.getter(superAcc.owner) - if (alias != NoSymbol && - superClazz.info.nonPrivateMember(alias.name) != alias) - alias = NoSymbol - if (alias != NoSymbol) { - var ownAcc = clazz.info.decl(name).suchThat(_.isParamAccessor) - if ((ownAcc hasFlag ACCESSOR) && !ownAcc.isDeferred) - ownAcc = ownAcc.accessed - if (!ownAcc.isVariable && !alias.accessed.isVariable) { - debuglog("" + ownAcc + " has alias "+alias.fullLocationString) //debug - ownAcc.asInstanceOf[TermSymbol].setAlias(alias) - } - } + def superClazz = superConstr.symbol.owner + def superParamAccessors = superClazz.constrParamAccessors + + // associate superclass paramaccessors with their aliases + if (superConstr.symbol.isPrimaryConstructor && !superClazz.isJavaDefined && sameLength(superParamAccessors, superArgs)) { + for ((superAcc, superArg @ Ident(name)) <- superParamAccessors zip superArgs) { + if (mexists(vparamss)(_.symbol == superArg.symbol)) { + val alias = ( + superAcc.initialize.alias + orElse (superAcc getter superAcc.owner) + filter (alias => superClazz.info.nonPrivateMember(alias.name) != alias) + ) + if (alias.exists && !alias.accessed.isVariable) { + val ownAcc = clazz.info decl name suchThat (_.isParamAccessor) match { + case acc if !acc.isDeferred && acc.hasAccessorFlag => acc.accessed + case acc => acc + } + ownAcc match { + case acc: TermSymbol if !acc.isVariable => + debuglog(s"$acc has alias ${alias.fullLocationString}") + acc setAlias alias + case _ => } } } diff --git a/test/files/neg/t6928.check b/test/files/neg/t6928.check new file mode 100644 index 0000000000..28b8e382dc --- /dev/null +++ b/test/files/neg/t6928.check @@ -0,0 +1,7 @@ +t6928.scala:2: error: super constructor cannot be passed a self reference unless parameter is declared by-name +object B extends A(B) + ^ +t6928.scala:3: error: super constructor cannot be passed a self reference unless parameter is declared by-name +object C extends A(null, null, C) + ^ +two errors found diff --git a/test/files/neg/t6928.scala b/test/files/neg/t6928.scala new file mode 100644 index 0000000000..84bdcde45a --- /dev/null +++ b/test/files/neg/t6928.scala @@ -0,0 +1,10 @@ +abstract class A( val someAs: A* ) +object B extends A(B) +object C extends A(null, null, C) + +object Test { + def main(args: Array[String]): Unit = { + println(B.someAs) + println(C.someAs) + } +} diff --git a/test/files/run/t6928-run.check b/test/files/run/t6928-run.check new file mode 100644 index 0000000000..a640c3e5fd --- /dev/null +++ b/test/files/run/t6928-run.check @@ -0,0 +1 @@ +3 As diff --git a/test/files/run/t6928-run.scala b/test/files/run/t6928-run.scala new file mode 100644 index 0000000000..87a8884d60 --- /dev/null +++ b/test/files/run/t6928-run.scala @@ -0,0 +1,10 @@ +abstract class A( val someAs: A* ) { + override def toString = someAs.length + " As" +} +object B extends A(null, null, null) + +object Test { + def main(args: Array[String]): Unit = { + println(B) + } +} -- cgit v1.2.3