Merge pull request #5846 from adriaanm/t10261-2.11

Actually retract clashing synthetic apply/unapply

8 files changed, 42 insertions, 3 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
index b755ee3ebd..81299dc425 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala
@@ -691,8 +691,21 @@ trait Namers extends MethodSynthesis {
 
           if (suppress) {
             sym setInfo ErrorType
+            // There are two ways in which we exclude the symbol from being added in typedStats::addSynthetics,
+            // because we don't know when the completer runs with respect to this loop in addSynthetics
+            //  for (sym <- scope)
+            //    for (tree <- context.unit.synthetics.get(sym) if shouldAdd(sym)) {
+            //      if (!sym.initialize.hasFlag(IS_ERROR))
+            //        newStats += typedStat(tree)
+            //  (1) If we're already in the loop, set the IS_ERROR flag and trigger the condition
+            //      `sym.initialize.hasFlag(IS_ERROR)` in typedStats::addSynthetics,
+            //  (2) Or, if we are not yet in the addSynthetics loop (and we're not going to emit an error anyway),
+            //      we unlink the symbol from its scope.
             sym setFlag IS_ERROR
 
+            // For good measure. Removing it from its owner's scope and setting the IS_ERROR flag is enough to exclude it from addSynthetics
+            companionContext.unit.synthetics -= sym
+
             // 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
@@ -702,7 +715,7 @@ trait Namers extends MethodSynthesis {
             // 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)
+              companionContext.scope.unlink(sym) // (2)
           }
         }
       }
@@ -770,7 +783,7 @@ trait Namers extends MethodSynthesis {
         val completer =
           if (sym hasFlag SYNTHETIC) {
             if (name == nme.copy) copyMethodCompleter(tree)
-            else if (sym hasFlag CASE) applyUnapplyMethodCompleter(tree, context)
+            else if (settings.isScala212 && (sym hasFlag CASE)) applyUnapplyMethodCompleter(tree, context)
             else completerOf(tree)
           } else completerOf(tree)
 
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 00e0517df6..ac0a653626 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -3093,6 +3093,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
         val scope = if (inBlock) context.scope else context.owner.info.decls
         var newStats = new ListBuffer[Tree]
         var moreToAdd = true
+        val retractErroneousSynthetics = settings.isScala212
+
         while (moreToAdd) {
           val initElems = scope.elems
           // SI-5877 The decls of a package include decls of the package object. But we don't want to add
@@ -3101,7 +3103,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
             inBlock || !context.isInPackageObject(sym, context.owner)
           for (sym <- scope)
             for (tree <- context.unit.synthetics get sym if shouldAdd(sym)) { // OPT: shouldAdd is usually true. Call it here, rather than in the outer loop
-              newStats += typedStat(tree) // might add even more synthetics to the scope
+              // if the completer set the IS_ERROR flag, retract the stat (currently only used by applyUnapplyMethodCompleter)
+              if (!(retractErroneousSynthetics && sym.initialize.hasFlag(IS_ERROR)))
+                newStats += typedStat(tree) // might add even more synthetics to the scope
               context.unit.synthetics -= sym
             }
           // the type completer of a synthetic might add more synthetics. example: if the
diff --git a/test/files/neg/userdefined_apply.flags b/test/files/neg/userdefined_apply.flags
new file mode 100644
index 0000000000..0acce1e7ce
--- /dev/null
+++ b/test/files/neg/userdefined_apply.flags
@@ -0,0 +1 @@
+-Xsource:2.12
diff --git a/test/files/pos/userdefined_apply.flags b/test/files/pos/userdefined_apply.flags
new file mode 100644
index 0000000000..0acce1e7ce
--- /dev/null
+++ b/test/files/pos/userdefined_apply.flags
@@ -0,0 +1 @@
+-Xsource:2.12
diff --git a/test/files/pos/userdefined_apply_poly_overload.flags b/test/files/pos/userdefined_apply_poly_overload.flags
new file mode 100644
index 0000000000..0acce1e7ce
--- /dev/null
+++ b/test/files/pos/userdefined_apply_poly_overload.flags
@@ -0,0 +1 @@
+-Xsource:2.12
diff --git a/test/files/run/t10261.flags b/test/files/run/t10261.flags
new file mode 100644
index 0000000000..0acce1e7ce
--- /dev/null
+++ b/test/files/run/t10261.flags
@@ -0,0 +1 @@
+-Xsource:2.12
diff --git a/test/files/run/t10261/Companion_1.scala b/test/files/run/t10261/Companion_1.scala
new file mode 100644
index 0000000000..9b8e2c73b2
--- /dev/null
+++ b/test/files/run/t10261/Companion_1.scala
@@ -0,0 +1,4 @@
+trait Companion[T] {
+  def parse(value: String): Option[T]
+  def apply(value: String): T = parse(value).get
+}
diff --git a/test/files/run/t10261/Test_2.scala b/test/files/run/t10261/Test_2.scala
new file mode 100644
index 0000000000..d7d9fe9a0e
--- /dev/null
+++ b/test/files/run/t10261/Test_2.scala
@@ -0,0 +1,14 @@
+import scala.util.Try
+
+object C extends Companion[C] {
+  def parse(v: String) = if (v.nonEmpty) Some(new C(v)) else None
+}
+
+case class C(value: String)
+
+object Test {
+  def main(args: Array[String]): Unit = {
+    assert(Try{C("")}.isFailure, "Empty value should fail to parse") // check that parse is used to validate input
+    assert(C("a").value == "a", "Unexpected value")
+  }
+}