From eed52216c634a6d73f737358ed6d6c5855452603 Mon Sep 17 00:00:00 2001 From: Adriaan Moors Date: Sun, 26 Feb 2017 16:01:37 -0800 Subject: Allow user-defined `[un]apply` in case companion Don't emit a synthetic `apply` (or `unapply`) when it would clash with an existing one. This allows e.g., a `private apply`, along with a `case class` with a `private` constructor. We have to retract the synthetic method in a pretty roundabout way, as we need the other methods and the owner to be completed already. Unless we have to complete the synthetic `apply` while completing the user-defined one, this should not be a problem. If this does happen, this implies there's a cycle in computing the user-defined signature and the synthetic one, which is not allowed. --- .../scala/tools/nsc/typechecker/Namers.scala | 99 ++++++++++++++++------ test/files/neg/userdefined_apply.check | 13 +++ test/files/neg/userdefined_apply.scala | 31 +++++++ test/files/pos/userdefined_apply.scala | 36 ++++++++ 4 files changed, 155 insertions(+), 24 deletions(-) create mode 100644 test/files/neg/userdefined_apply.check create mode 100644 test/files/neg/userdefined_apply.scala create mode 100644 test/files/pos/userdefined_apply.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 28169c9da1..8c5f4590b9 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -613,7 +613,15 @@ trait Namers extends MethodSynthesis { noDuplicates(selectors map (_.rename), AppearsTwice) } - def enterCopyMethod(copyDef: DefDef): Symbol = { + class CompleterWrapper(completer: TypeCompleter) extends TypeCompleter { + val tree = completer.tree + + override def complete(sym: Symbol): Unit = { + completer.complete(sym) + } + } + + def copyMethodCompleter(copyDef: DefDef): TypeCompleter = { val sym = copyDef.symbol val lazyType = completerOf(copyDef) @@ -632,14 +640,63 @@ trait Namers extends MethodSynthesis { ) } - sym setInfo { - mkTypeCompleter(copyDef) { sym => - assignParamTypes() - lazyType complete sym - } + mkTypeCompleter(copyDef) { sym => + assignParamTypes() + lazyType complete sym } } + // for apply/unapply, which may need to disappear when they clash with a user-defined method of matching signature + def applyUnapplyMethodCompleter(un_applyDef: DefDef, companionContext: Context): TypeCompleter = + new CompleterWrapper(completerOf(un_applyDef)) { + override def complete(sym: Symbol): Unit = { + super.complete(sym) + + // don't propagate e.g. @volatile annot to apply's argument + def retainOnlyParamAnnots(param: Symbol) = + param setAnnotations (param.annotations filter AnnotationInfo.mkFilter(ParamTargetClass, defaultRetention = false)) + + sym.info.paramss.foreach(_.foreach(retainOnlyParamAnnots)) + + // If there's a same-named locked symbol, we're currently completing its signature. + // This means it (may) refer to us, and is thus either overloaded or recursive without a signature. + // rule out locked symbols from the owner.info.member call + val scopePartiallyCompleted = + companionContext.scope.lookupAll(sym.name).exists(existing => existing != sym && existing.hasFlag(LOCKED)) + + val suppress = + scopePartiallyCompleted || { + val userDefined = companionContext.owner.info.member(sym.name).filter(_ != sym) + (userDefined != NoSymbol) && { + userDefined.info match { + // TODO: do we have something for this already? the synthetic symbol can't be overloaded, right? + case OverloadedType(pre, alternatives) => + // pre probably relevant because of inherited overloads? + alternatives.exists(_.isErroneous) || alternatives.exists(alt => pre.memberInfo(alt) matches pre.memberInfo(sym)) + case tp => + (tp eq ErrorType) || tp.matches(sym.info) + } + } + } + + if (suppress) { + sym setInfo ErrorType + sym setFlag IS_ERROR + + // Don't unlink in an error situation to generate less confusing error messages. + // Ideally, our error reporting would distinguish overloaded from recursive user-defined apply methods without signature, + // but this would require some form of partial-completion of method signatures, so that we can + // know what the argument types were, even though we can't complete the result type, because + // we hit a cycle while trying to compute it (when we get here with locked user-defined symbols, we + // are in the complete for that symbol, and thus the locked symbol has not yet received enough info; + // I hesitate to provide more info, because it would involve a WildCard or something for its result type, + // which could upset other code paths) + if (!scopePartiallyCompleted) + companionContext.scope.unlink(sym) + } + } + } + def completerOf(tree: MemberDef): TypeCompleter = { val mono = namerOf(tree.symbol) monoTypeCompleter tree val tparams = treeInfo.typeParameters(tree) @@ -687,13 +744,17 @@ trait Namers extends MethodSynthesis { val bridgeFlag = if (mods hasAnnotationNamed tpnme.bridgeAnnot) BRIDGE | ARTIFACT else 0 val sym = assignAndEnterSymbol(tree) setFlag bridgeFlag - if (name == nme.copy && sym.isSynthetic) - enterCopyMethod(tree) - else if (name == nme.apply && sym.hasAllFlags(SYNTHETIC | CASE)) - sym setInfo caseApplyMethodCompleter(tree, completerOf(tree).asInstanceOf[LockingTypeCompleter]) - else - sym setInfo completerOf(tree) - } + // copy/apply/unapply synthetics are added using the addIfMissing mechanism, + // which ensures the owner has its preliminary info (we may add another decl here) + val completer = + if (sym hasFlag SYNTHETIC) { + if (name == nme.copy) copyMethodCompleter(tree) + else if (sym hasFlag CASE) applyUnapplyMethodCompleter(tree, context) + else completerOf(tree) + } else completerOf(tree) + + sym setInfo completer + } def enterClassDef(tree: ClassDef) { val ClassDef(mods, _, _, impl) = tree @@ -818,16 +879,6 @@ trait Namers extends MethodSynthesis { classSym setAnnotations (annotations filter annotationFilter(ClassTargetClass, defaultRetention = true)) } - def caseApplyMethodCompleter(tree: DefDef, sigCompleter: LockingTypeCompleter) = mkTypeCompleter(tree) { methSym => - sigCompleter.completeImpl(methSym) - - // don't propagate e.g. @volatile annot to apply's argument - def retainOnlyParamAnnots(param: Symbol) = - param setAnnotations (param.annotations filter AnnotationInfo.mkFilter(ParamTargetClass, defaultRetention = false)) - - methSym.info.paramss.foreach(_.foreach(retainOnlyParamAnnots)) - } - // complete the type of a value definition (may have a method symbol, for those valdefs that never receive a field, // as specified by Field.noFieldFor) def valTypeCompleter(tree: ValDef) = mkTypeCompleter(tree) { fieldOrGetterSym => @@ -1464,7 +1515,7 @@ trait Namers extends MethodSynthesis { val defTpt = // don't mess with tpt's of case copy default getters, because assigning something other than TypeTree() - // will break the carefully orchestrated naming/typing logic that involves enterCopyMethod and caseClassCopyMeth + // will break the carefully orchestrated naming/typing logic that involves copyMethodCompleter and caseClassCopyMeth if (meth.isCaseCopy) TypeTree() else { // If the parameter type mentions any type parameter of the method, let the compiler infer the diff --git a/test/files/neg/userdefined_apply.check b/test/files/neg/userdefined_apply.check new file mode 100644 index 0000000000..ca0154885d --- /dev/null +++ b/test/files/neg/userdefined_apply.check @@ -0,0 +1,13 @@ +userdefined_apply.scala:3: error: overloaded method apply needs result type + private def apply(x: Int) = if (x > 0) new ClashOverloadNoSig(x) else apply("") + ^ +userdefined_apply.scala:12: error: overloaded method apply needs result type + private def apply(x: Int) = if (x > 0) ClashRecNoSig(1) else ??? + ^ +userdefined_apply.scala:19: error: overloaded method apply needs result type + private def apply(x: Boolean) = if (x) NoClashNoSig(1) else ??? + ^ +userdefined_apply.scala:26: error: overloaded method apply needs result type + private def apply(x: Boolean) = if (x) NoClashOverload(1) else apply("") + ^ +four errors found diff --git a/test/files/neg/userdefined_apply.scala b/test/files/neg/userdefined_apply.scala new file mode 100644 index 0000000000..1f2aff6e82 --- /dev/null +++ b/test/files/neg/userdefined_apply.scala @@ -0,0 +1,31 @@ +object ClashOverloadNoSig { + // error: overloaded method apply needs result type + private def apply(x: Int) = if (x > 0) new ClashOverloadNoSig(x) else apply("") + + def apply(x: String): ClashOverloadNoSig = ??? +} + +case class ClashOverloadNoSig private(x: Int) + +object ClashRecNoSig { + // error: recursive method apply needs result type + private def apply(x: Int) = if (x > 0) ClashRecNoSig(1) else ??? +} + +case class ClashRecNoSig private(x: Int) + +object NoClashNoSig { + // error: overloaded method apply needs result type + private def apply(x: Boolean) = if (x) NoClashNoSig(1) else ??? +} + +case class NoClashNoSig private(x: Int) + +object NoClashOverload { + // error: overloaded method apply needs result type + private def apply(x: Boolean) = if (x) NoClashOverload(1) else apply("") + + def apply(x: String): NoClashOverload = ??? +} + +case class NoClashOverload private(x: Int) diff --git a/test/files/pos/userdefined_apply.scala b/test/files/pos/userdefined_apply.scala new file mode 100644 index 0000000000..ca563f1dc5 --- /dev/null +++ b/test/files/pos/userdefined_apply.scala @@ -0,0 +1,36 @@ +// NOTE: the companion inherits a public apply method from Function1! +case class NeedsCompanion private (x: Int) + +object ClashNoSig { // ok + private def apply(x: Int) = if (x > 0) new ClashNoSig(x) else ??? +} +case class ClashNoSig private (x: Int) + + +object Clash { + private def apply(x: Int) = if (x > 0) new Clash(x) else ??? +} +case class Clash private (x: Int) + +object ClashSig { + private def apply(x: Int): ClashSig = if (x > 0) new ClashSig(x) else ??? +} +case class ClashSig private (x: Int) + +object ClashOverload { + private def apply(x: Int): ClashOverload = if (x > 0) new ClashOverload(x) else apply("") + def apply(x: String): ClashOverload = ??? +} +case class ClashOverload private (x: Int) + +object NoClashSig { + private def apply(x: Boolean): NoClashSig = if (x) NoClashSig(1) else ??? +} +case class NoClashSig private (x: Int) + +object NoClashOverload { + // needs full sig + private def apply(x: Boolean): NoClashOverload = if (x) NoClashOverload(1) else apply("") + def apply(x: String): NoClashOverload = ??? +} +case class NoClashOverload private (x: Int) -- cgit v1.2.3