From 64138082a4f1bb4c864d85660d350f61c81e7fd7 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Fri, 3 Aug 2012 17:13:18 +0200 Subject: SI-5940 impls are no longer in macro def pickles The first officially released version of macros persisted macro def -> impl bindings across compilation runs using a neat trick. The right-hand side of macro definition (which contains a reference to an impl) was typechecked and then put verbatim into an annotation on macro definition. This solution is very simple, but unfortunately it's also lacking. If we use it then signatures of macro defs become transitively dependent on scala-reflect.jar (because they refer to macro impls, and macro impls refer to scala.reflect.macros.Context defined in scala-reflect.jar). More details can be found in https://issues.scala-lang.org/browse/SI-5940. Therefore we have to avoid putting macro impls into binding pickles and come up with our own serialization format. Situation is further complicated by the fact that it's not enough to just pickle impl's class and method names, because macro expansion needs knowledge about the shape of impl's signature (which we can't pickle). Hence we precompute necessary stuff (e.g. the layout of type parameters) when compiling macro defs. --- test/files/run/t5940.scala | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/files/run/t5940.scala (limited to 'test/files') diff --git a/test/files/run/t5940.scala b/test/files/run/t5940.scala new file mode 100644 index 0000000000..147ff38256 --- /dev/null +++ b/test/files/run/t5940.scala @@ -0,0 +1,41 @@ +import scala.tools.partest._ + +object Test extends DirectTest { + def code = ??? + + def macros_1 = """ + import scala.reflect.macros.Context + + object Impls { + def impl(c: Context) = c.literalUnit + } + + object Macros { + //import Impls._ + def impl(c: Context) = c.literalUnit + def foo = macro impl + } + """ + def compileMacros() = { + val classpath = List(sys.props("partest.lib"), sys.props("partest.reflect")) mkString sys.props("path.separator") + compileString(newCompiler("-language:experimental.macros", "-cp", classpath, "-d", testOutput.path))(macros_1) + } + + def test_2 = """ + object Test extends App { + println(Macros.foo) + } + """ + def compileTest() = { + val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") + compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(test_2) + } + + def show(): Unit = { + log("Compiling Macros_1...") + if (compileMacros()) { + log("Compiling Test_2...") + if (compileTest()) log("Success!") else log("Failed...") + } + } +} \ No newline at end of file -- cgit v1.2.3 From 46aab7b8d77d275aad0686b11214ee1a0d62e941 Mon Sep 17 00:00:00 2001 From: Eugene Burmako Date: Fri, 10 Aug 2012 00:33:30 +0200 Subject: macro implementations must be public The immediate motivator for this change was the desire to keep things simple during macro expansion. Here's the story. When you expand a macro, you need to reflectively load a macro implementation method and invoke it. However private methods in Scala can sometime have their names mangled, so the reflector has to check multiple variants of the same name (the normal one and the mangled one). The problem is that since the previous commit we don't have an access to a symbol of the macro impl, hence we cannot just use `impl.expandedName` like we did before. Sure I could duplicate the logic of expandedName, but I have a better suggestion. Let's prohibit non-public macro implementations. This doesn't seem to hurt, and it lets us avoid extra bit of complexity in Macros.scala. If this measure turns out to be a hassle during the trial period of macros, non-public macros can always be allowed. In fact, we can even have this feature back for free when we migrate from Java reflection to Scala reflection for invoking macro implementations (because Scala reflection knows how to account for mangled private names). But that's the 2.10.x business. --- src/compiler/scala/tools/nsc/typechecker/Macros.scala | 2 ++ test/files/neg/macro-invalidimpl-i.check | 4 ++++ test/files/neg/macro-invalidimpl-i.flags | 1 + test/files/neg/macro-invalidimpl-i/Impls_1.scala | 7 +++++++ test/files/neg/macro-invalidimpl-i/Macros_Test_2.scala | 5 +++++ 5 files changed, 19 insertions(+) create mode 100644 test/files/neg/macro-invalidimpl-i.check create mode 100644 test/files/neg/macro-invalidimpl-i.flags create mode 100644 test/files/neg/macro-invalidimpl-i/Impls_1.scala create mode 100644 test/files/neg/macro-invalidimpl-i/Macros_Test_2.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/typechecker/Macros.scala b/src/compiler/scala/tools/nsc/typechecker/Macros.scala index e48a95fab0..7d5ab25235 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Macros.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Macros.scala @@ -492,6 +492,8 @@ trait Macros extends scala.tools.reflect.FastTrack with Traces { } else { if (!macroImpl.isMethod) invalidBodyError() + if (!macroImpl.isPublic) + reportError(implpos, "macro implementation must be public") if (macroImpl.isOverloaded) reportError(implpos, "macro implementation cannot be overloaded") if (!macroImpl.typeParams.isEmpty && (!rhs1.isInstanceOf[TypeApply])) diff --git a/test/files/neg/macro-invalidimpl-i.check b/test/files/neg/macro-invalidimpl-i.check new file mode 100644 index 0000000000..b6277809a3 --- /dev/null +++ b/test/files/neg/macro-invalidimpl-i.check @@ -0,0 +1,4 @@ +Macros_Test_2.scala:4: error: macro implementation must be public + def foo = macro Impls.impl + ^ +one error found diff --git a/test/files/neg/macro-invalidimpl-i.flags b/test/files/neg/macro-invalidimpl-i.flags new file mode 100644 index 0000000000..cd66464f2f --- /dev/null +++ b/test/files/neg/macro-invalidimpl-i.flags @@ -0,0 +1 @@ +-language:experimental.macros \ No newline at end of file diff --git a/test/files/neg/macro-invalidimpl-i/Impls_1.scala b/test/files/neg/macro-invalidimpl-i/Impls_1.scala new file mode 100644 index 0000000000..c35d8ab3c1 --- /dev/null +++ b/test/files/neg/macro-invalidimpl-i/Impls_1.scala @@ -0,0 +1,7 @@ +package foo + +import scala.reflect.macros.Context + +object Impls { + private[foo] def impl(c: Context) = ??? +} \ No newline at end of file diff --git a/test/files/neg/macro-invalidimpl-i/Macros_Test_2.scala b/test/files/neg/macro-invalidimpl-i/Macros_Test_2.scala new file mode 100644 index 0000000000..fb129c70be --- /dev/null +++ b/test/files/neg/macro-invalidimpl-i/Macros_Test_2.scala @@ -0,0 +1,5 @@ +package foo + +object Test extends App { + def foo = macro Impls.impl +} -- cgit v1.2.3