Fix #3853 and #4151: Compile code in the macro definition and remove interpreter

[nicolasstucki]

The only interpreted code remaining is a call to a static method
containing a lambda which can compute the resulting inlined quoted
expression. Arguments to the macro are trivially computed from the
inlined call and bindings.

[nicolasstucki]

test performance please

[dottybot]

performance test scheduled: 1 job(s) in queue, 0 running.

[dottybot]

performance test failed:

Error line number: 24

[check /data/workspace/bench/logs/pull-4155-03-22-15.36.out for more information]

[nicolasstucki]

@liufengyun do you know what failed in the benchmark server?

[liufengyun]

Sorry, I have no access to computer now, but you should be able to login into lamppc37 with your account and check the log file. Or, I can check in one hour. Le ven. 23 mars 2018 17:22, Nicolas Stucki <[email protected]> a écrit :

…

@liufengyun <https://github.com/liufengyun> do you know what failed in the benchmark server? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4155 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAuDyfk94KClMJ8U2FbFbNIMl0YPt4KJks5thSEsgaJpZM4S3KY9> .

[liufengyun]

Cause diagnosed: in #4149 , an empty file Foo.scala is incorrectly added to the repo.

That error should not cause the benchmarks to fail, but the following find command failed due to the existence of Foo.scala in current directory. If I remove Foo.scala, everything works correctly.

find /data/workspace/bench/tests/scala-library -name *.scala

[nicolasstucki]

@liufengyun thanks :)

[nicolasstucki]

test performance please

[dottybot]

performance test scheduled: 1 job(s) in queue, 0 running.

[dottybot]

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/4155/ to see the changes.

Benchmarks is based on merging with master (c89f8fa)

[Blaisorblade]

That error should not cause the benchmarks to fail, but the following find command failed due to the existence of Foo.scala in current directory. If I remove Foo.scala, everything works correctly.

find /data/workspace/bench/tests/scala-library -name *.scala

As a rule, it seems good to prevent such issues by adding quotes around *.scala, hence writing find /data/workspace/bench/tests/scala-library -name "*.scala".

[odersky]

I think it's a pity that we do not allow general interpretation in an inline macro. Why do we have to remove that possibility?

[odersky]

Why make the exception for macros? Since there is a reasoning in the comment above why we rewrite calls to callTraces, we need to add to the comment a reasoning why the same does not apply to macro calls.

[nicolasstucki]

I will add a comment

[nicolasstucki]

Added the comment and making this optimization at ReifyQuotes when the call is processed.

[nicolasstucki]

Removing the interpreter does not really limit what we could already write in terms of macros. @odersky do you have some precise examples where we would need interpretation?

This transformation can also be extended to support multiple spliced in a single macro by making the macro return a list of lambdas. We would only need to find the additional definitions in the body of the inline method (such as y) and also pass them to the lambdas.

def foo(inline x: Int): Int = {
  val y  = 3
  ~impl2(x) + ~impl3('(y))
}

This could also be performed by the interpreter, but is currently not supported as well.

One thing is not possible anymore with the changes in this PR. We cannot write inline macros as extension methods in implicit classes. Those were only working by an abuse of the inline bindings which were wrongly moved to be interpreted to allow this to work. The failed if the implicit class was instantiated explicitly. The correct way to would be using the new extension methods, where we might additionally need to require the this parameter to be an inline parameter of the resulting method (this would need a direct encoding of the extensions without value classes).

[odersky]

I think I get it now after @nicolasstucki explained it to me. LGTM.