summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAdriaan Moors <adriaan@lightbend.com>2017-02-28 14:14:11 -0800
committerAdriaan Moors <adriaan@lightbend.com>2017-04-06 08:00:13 -0700
commit31a56077af5c5b35049fec456204e12a19bb6701 (patch)
treeb1e40f24fc34d87e117d2e7a0fdcccdeb4dc254c
parent276434b4af2c2d244d1b5e596867041b36e7b920 (diff)
downloadscala-31a56077af5c5b35049fec456204e12a19bb6701.tar.gz
scala-31a56077af5c5b35049fec456204e12a19bb6701.tar.bz2
scala-31a56077af5c5b35049fec456204e12a19bb6701.zip
Improvements based on reviews by Lukas & Jason
-rw-r--r--spec/05-classes-and-objects.md5
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/Namers.scala48
-rw-r--r--src/reflect/scala/reflect/internal/tpe/FindMembers.scala14
-rw-r--r--test/files/neg/userdefined_apply.check20
-rw-r--r--test/files/neg/userdefined_apply.scala26
-rw-r--r--test/files/pos/userdefined_apply.scala18
6 files changed, 107 insertions, 24 deletions
diff --git a/spec/05-classes-and-objects.md b/spec/05-classes-and-objects.md
index 246e579c94..ffb65979f7 100644
--- a/spec/05-classes-and-objects.md
+++ b/spec/05-classes-and-objects.md
@@ -876,10 +876,9 @@ If a type parameter section is missing in the class, it is also missing in the `
If the companion object $c$ is already defined,
the `apply` and `unapply` methods are added to the existing object.
+If the object $c$ already has a [matching](#definition-matching)
+`apply` (or `unapply`) member, no new definition is added.
The definition of `apply` is omitted if class $c$ is `abstract`.
-If the object $c$ already defines a [matching](#definition-matching) member of the
-same name as the synthetic member to be added, the synthetic member
-is not added (overloading or mutual recursion is allowed, however).
If the case class definition contains an empty value parameter list, the
`unapply` method returns a `Boolean` instead of an `Option` type and
diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index 8c5f4590b9..51df750951 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -650,6 +650,8 @@ trait Namers extends MethodSynthesis {
def applyUnapplyMethodCompleter(un_applyDef: DefDef, companionContext: Context): TypeCompleter =
new CompleterWrapper(completerOf(un_applyDef)) {
override def complete(sym: Symbol): Unit = {
+ assert(sym hasAllFlags CASE | SYNTHETIC, sym.defString)
+
super.complete(sym)
// don't propagate e.g. @volatile annot to apply's argument
@@ -658,23 +660,37 @@ trait Namers extends MethodSynthesis {
sym.info.paramss.foreach(_.foreach(retainOnlyParamAnnots))
+ // owner won't be locked
+ val ownerInfo = companionContext.owner.info
+
// 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)
+ // If `scopePartiallyCompleted`, the program is known to have a type error, since
+ // this means a user-defined method is missing a result type while its rhs refers to `sym` or an overload.
+ // This is an error because overloaded/recursive methods must have a result type.
+ // The method would be overloaded if its signature, once completed, would not match the synthetic method's,
+ // or recursive if it turned out we should unlink our synthetic method (matching sig).
+ // In any case, error out. We don't unlink the symbol so that `symWasOverloaded` says yes,
+ // which would be wrong if the method is in fact recursive, but it seems less confusing.
+ val scopePartiallyCompleted = new HasMember(ownerInfo, sym.name, BridgeFlags | SYNTHETIC, LOCKED).apply()
+
+ // Check `scopePartiallyCompleted` first to rule out locked symbols from the owner.info.member call,
+ // as FindMember will call info on a locked symbol (while checking type matching to assemble an overloaded type),
+ // and throw a TypeError, so that we are aborted.
+ // Do not consider deferred symbols, as suppressing our concrete implementation would be an error regardless
+ // of whether the signature matches (if it matches, we omitted a valid implementation, if it doesn't,
+ // we would get an error for the missing implementation it isn't implemented by some overload other than our synthetic one)
+ val suppress = scopePartiallyCompleted || {
+ // can't exclude deferred members using DEFERRED flag here (TODO: why?)
+ val userDefined = ownerInfo.memberBasedOnName(sym.name, BridgeFlags | SYNTHETIC)
+
(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)
+ assert(userDefined != sym)
+ val alts = userDefined.alternatives // could be just the one, if this member isn't overloaded
+ // don't compute any further `memberInfo`s if there's an error somewhere
+ alts.exists(_.isErroneous) || {
+ val self = companionContext.owner.thisType
+ val memberInfo = self.memberInfo(sym)
+ alts.exists(alt => !alt.isDeferred && (self.memberInfo(alt) matches memberInfo))
}
}
}
@@ -744,8 +760,6 @@ trait Namers extends MethodSynthesis {
val bridgeFlag = if (mods hasAnnotationNamed tpnme.bridgeAnnot) BRIDGE | ARTIFACT else 0
val sym = assignAndEnterSymbol(tree) setFlag bridgeFlag
- // 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)
diff --git a/src/reflect/scala/reflect/internal/tpe/FindMembers.scala b/src/reflect/scala/reflect/internal/tpe/FindMembers.scala
index 6ba48cb44d..510d76793e 100644
--- a/src/reflect/scala/reflect/internal/tpe/FindMembers.scala
+++ b/src/reflect/scala/reflect/internal/tpe/FindMembers.scala
@@ -285,4 +285,18 @@ trait FindMembers {
initBaseClasses.head.newOverloaded(tpe, members)
}
}
+
+ private[scala] final class HasMember(tpe: Type, name: Name, excludedFlags: Long, requiredFlags: Long) extends FindMemberBase[Boolean](tpe, name, excludedFlags, requiredFlags) {
+ private[this] var _result = false
+ override protected def result: Boolean = _result
+
+ protected def shortCircuit(sym: Symbol): Boolean = {
+ _result = true
+ true // prevents call to addMemberIfNew
+ }
+
+ // Not used
+ protected def addMemberIfNew(sym: Symbol): Unit = {}
+ }
+
}
diff --git a/test/files/neg/userdefined_apply.check b/test/files/neg/userdefined_apply.check
index ca0154885d..c8c8976f5f 100644
--- a/test/files/neg/userdefined_apply.check
+++ b/test/files/neg/userdefined_apply.check
@@ -1,13 +1,25 @@
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
+userdefined_apply.scala:14: 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
+userdefined_apply.scala:21: 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
+userdefined_apply.scala:28: error: overloaded method apply needs result type
private def apply(x: Boolean) = if (x) NoClashOverload(1) else apply("")
^
-four errors found
+userdefined_apply.scala:45: error: recursive method apply needs result type
+case class NoClashNoSigPoly private(x: Int)
+ ^
+userdefined_apply.scala:39: error: NoClashNoSigPoly.type does not take parameters
+ def apply(x: T) = if (???) NoClashNoSigPoly(1) else ???
+ ^
+userdefined_apply.scala:57: error: recursive method apply needs result type
+case class ClashNoSigPoly private(x: Int)
+ ^
+userdefined_apply.scala:51: error: ClashNoSigPoly.type does not take parameters
+ def apply(x: T) = if (???) ClashNoSigPoly(1) else ???
+ ^
+8 errors found
diff --git a/test/files/neg/userdefined_apply.scala b/test/files/neg/userdefined_apply.scala
index 1f2aff6e82..0a0d960b39 100644
--- a/test/files/neg/userdefined_apply.scala
+++ b/test/files/neg/userdefined_apply.scala
@@ -8,6 +8,8 @@ object ClashOverloadNoSig {
case class ClashOverloadNoSig private(x: Int)
object ClashRecNoSig {
+ // TODO: status quo is that the error refers to an overloaded method, which is actually recursive
+ // (we should have unlinked the symbol in the `if(suppress)` part of `applyUnapplyMethodCompleter`)
// error: recursive method apply needs result type
private def apply(x: Int) = if (x > 0) ClashRecNoSig(1) else ???
}
@@ -29,3 +31,27 @@ object NoClashOverload {
}
case class NoClashOverload private(x: Int)
+
+
+class BaseNCNSP[T] {
+ // TODO: suppress the following error
+ // error: NoClashNoSigPoly.type does not take parameters
+ def apply(x: T) = if (???) NoClashNoSigPoly(1) else ???
+}
+
+object NoClashNoSigPoly extends BaseNCNSP[Boolean]
+// TODO: position error at definition of apply in superclass instead of on case clss
+// error: recursive method apply needs result type
+case class NoClashNoSigPoly private(x: Int)
+
+
+class BaseCNSP[T] {
+ // TODO: suppress the following error
+ // error: ClashNoSigPoly.type does not take parameters
+ def apply(x: T) = if (???) ClashNoSigPoly(1) else ???
+}
+
+object ClashNoSigPoly extends BaseCNSP[Int]
+// TODO: position error at definition of apply in superclass instead of on case clss
+// error: recursive method apply needs result type
+case class ClashNoSigPoly private(x: Int)
diff --git a/test/files/pos/userdefined_apply.scala b/test/files/pos/userdefined_apply.scala
index ca563f1dc5..e29f9f5141 100644
--- a/test/files/pos/userdefined_apply.scala
+++ b/test/files/pos/userdefined_apply.scala
@@ -34,3 +34,21 @@ object NoClashOverload {
def apply(x: String): NoClashOverload = ???
}
case class NoClashOverload private (x: Int)
+
+
+
+class BaseNCP[T] {
+ // error: overloaded method apply needs result type
+ def apply(x: T): NoClashPoly = if (???) NoClashPoly(1) else ???
+}
+
+object NoClashPoly extends BaseNCP[Boolean]
+case class NoClashPoly private(x: Int)
+
+
+class BaseCP[T] {
+ // error: overloaded method apply needs result type
+ def apply(x: T): ClashPoly = if (???) ClashPoly(1) else ???
+}
+object ClashPoly extends BaseCP[Int]
+case class ClashPoly private(x: Int)