SI-7657 clarifies the "macro overrides method" rule

Currently we allow macros to override non-abstract methods (in order
to provide performance enhancements such as foreach for collections),
and we also disallow macros to override abstract methods (otherwise
downcasting might lead to AbstractMethodErrors).

This patch fixes an oversight in the disallowing rule that prohibited
macros from overriding a concrete method if that concrete method itself
overrides an abstract method. RefCheck entertains all overriding pairs,
not only the immediate ones, so the disallowing rule was triggered.

Now macros can override abstract methods if and only if either the base
type or the self type contain a matching non-abstract method.

7 files changed, 40 insertions, 20 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index 03ce710700..9c374e85ea 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -225,7 +225,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
      *    1.8.1  M's type is a subtype of O's type, or
      *    1.8.2  M is of type []S, O is of type ()T and S <: T, or
      *    1.8.3  M is of type ()S, O is of type []T and S <: T, or
-     *    1.9.  If M is a macro def, O cannot be deferred.
+     *    1.9.  If M is a macro def, O cannot be deferred unless there's a concrete method overriding O.
      *    1.10. If M is not a macro def, O cannot be a macro def.
      *  2. Check that only abstract classes have deferred members
      *  3. Check that concrete classes do not have deferred definitions
@@ -417,7 +417,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
           } else if (other.isValue && other.isLazy && !other.isSourceMethod && !other.isDeferred &&
                      member.isValue && !member.isLazy) {
             overrideError("must be declared lazy to override a concrete lazy value")
-          } else if (other.isDeferred && member.isTermMacro) { // (1.9)
+          } else if (other.isDeferred && member.isTermMacro && member.extendedOverriddenSymbols.forall(_.isDeferred)) { // (1.9)
             overrideError("cannot override an abstract method")
           } else if (other.isTermMacro && !member.isTermMacro) { // (1.10)
             overrideError("cannot override a macro")
diff --git a/test/files/neg/macro-override-macro-overrides-abstract-method-b.check b/test/files/neg/macro-override-macro-overrides-abstract-method-b.check
index 895e0dca50..cde3dbdbe8 100644
--- a/test/files/neg/macro-override-macro-overrides-abstract-method-b.check
+++ b/test/files/neg/macro-override-macro-overrides-abstract-method-b.check
@@ -1,5 +1,11 @@
-Impls_Macros_1.scala:12: error: overriding method foo in trait Foo of type (x: Int)Int;
- macro method foo cannot override an abstract method
-  def foo(x: Int) = macro Impls.impl
-      ^
-one error found
+Test_2.scala:3: error: anonymous class $anon inherits conflicting members:
+  macro method t in trait C of type ()Unit  and
+  method t in trait A of type ()Unit
+(Note: this can be resolved by declaring an override in anonymous class $anon.)
+  val c2 = new C with A {}
+               ^
+Test_2.scala:5: error: overriding macro method t in trait C of type ()Unit;
+ method t cannot override a macro
+  val c4 = new C with A { override def t(): Unit = () }
+                                       ^
+two errors found
diff --git a/test/files/neg/macro-override-macro-overrides-abstract-method-b/Impls_Macros_1.scala b/test/files/neg/macro-override-macro-overrides-abstract-method-b/Impls_Macros_1.scala
index e43264f52f..f5b2555aa5 100644
--- a/test/files/neg/macro-override-macro-overrides-abstract-method-b/Impls_Macros_1.scala
+++ b/test/files/neg/macro-override-macro-overrides-abstract-method-b/Impls_Macros_1.scala
@@ -1,13 +1,8 @@
-import scala.reflect.macros.{Context => Ctx}
+import scala.reflect.macros.Context
+import language.experimental.macros
 
-object Impls {
-  def impl(c: Ctx)(x: c.Expr[Int]) = x
-}
+trait T { def t(): Unit }
+trait A { def t(): Unit = () }
 
-trait Foo {
-  def foo(x: Int): Int
-}
-
-object Macros extends Foo {
-  def foo(x: Int) = macro Impls.impl
-}
+object Macro { def t(c: Context)(): c.Expr[Unit] = c.universe.reify(()) }
+trait C extends T { self: A => override def t(): Unit = macro Macro.t }
diff --git a/test/files/neg/macro-override-macro-overrides-abstract-method-b/Test_2.scala b/test/files/neg/macro-override-macro-overrides-abstract-method-b/Test_2.scala
index 08fff30baf..9b4c8e35f0 100644
--- a/test/files/neg/macro-override-macro-overrides-abstract-method-b/Test_2.scala
+++ b/test/files/neg/macro-override-macro-overrides-abstract-method-b/Test_2.scala
@@ -1,4 +1,6 @@
 object Test extends App {
-  val designator: Foo = Macros
-  designator.foo(42)
+  val c1 = new A with C {}
+  val c2 = new C with A {}
+  val c3 = new C with A { override def t(): Unit = macro Macro.t }
+  val c4 = new C with A { override def t(): Unit = () }
 }
\ No newline at end of file
diff --git a/test/files/run/t7657.check b/test/files/run/t7657.check
new file mode 100644
index 0000000000..c25d8d1c1b
--- /dev/null
+++ b/test/files/run/t7657.check
@@ -0,0 +1,3 @@
+()
+()
+()
diff --git a/test/files/run/t7657/Macros_1.scala b/test/files/run/t7657/Macros_1.scala
new file mode 100644
index 0000000000..b1e31aa2dd
--- /dev/null
+++ b/test/files/run/t7657/Macros_1.scala
@@ -0,0 +1,8 @@
+import scala.reflect.macros.Context
+import language.experimental.macros
+
+trait T { def t(): Unit }
+abstract class A extends T { override def t(): Unit = () }
+
+object Macro { def t(c: Context)(): c.Expr[Unit] = c.universe.reify(()) }
+class C extends A { override def t(): Unit = macro Macro.t }
diff --git a/test/files/run/t7657/Test_2.scala b/test/files/run/t7657/Test_2.scala
new file mode 100644
index 0000000000..5cc46b6aa1
--- /dev/null
+++ b/test/files/run/t7657/Test_2.scala
@@ -0,0 +1,6 @@
+object Test extends App {
+  val c = new C()
+  println(c.t())
+  println((c: T).t())
+  println((c: A).t())
+}
\ No newline at end of file