Permalink
Browse files

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.
  • Loading branch information...
xeno-by committed Jul 14, 2013
1 parent 2247593 commit ef979c02da887b7c56bc1da9c4eb888e92af570f
@@ -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")
@@ -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
@@ -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 }
@@ -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 = () }
}
@@ -0,0 +1,3 @@
+()
+()
+()
@@ -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 }
@@ -0,0 +1,6 @@
+object Test extends App {
+ val c = new C()
+ println(c.t())
+ println((c: T).t())
+ println((c: A).t())
+}

0 comments on commit ef979c0

Please sign in to comment.