Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change top-level ~ evaluation scheme #4822

Merged
merged 3 commits into from Jul 27, 2018

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Jul 21, 2018

The new scheme consists in only allowing simple interpretable expressions in top-level splices. Then, the inlined splice can be evaluated directly.

Changes:

  • top-level ~ must contain a call to a static method
    • arguments must be quoted
    • inline arguments can be passed directly
  • removed dependency on the original inlined code (removed in PostTyper)
  • ReifyQuotes is no longer an InfoTransformer
  • Splicer implements both checking and evaluation of splices in a single abstraction
  • Addapted some tests
  • Fix Macros fail Ycheck when called in inline #4773
  • Fix Macro inside inline fails Ycheck #4735

PR followed by #4823

@nicolasstucki
Copy link
Contributor Author

Rebased on #4616

@nicolasstucki nicolasstucki force-pushed the interpret-macros branch 2 times, most recently from c98ec3e to 7ea2810 Compare July 27, 2018 08:36
Copy link
Contributor

@biboudis biboudis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

// replace macro code by lambda used to evaluate the macro expansion
cpy.DefDef(tree)(tpt = TypeTree(macroReturnType), rhs = lambda)
reifier.transform(tree) // Ignore output, only check PCP
cpy.DefDef(tree)(rhs = defaultValue(tree.rhs.tpe))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the comment that was here before apply here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Now we just remove the RHS to prune the tree earlier in the pipeline. Maybe this could be done later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic will be simplified later in #4826.

* top-level ~ must contain a call to a static method
  * arguments must be quoted
  * inline arguments can be passed directly
* removed dependency on the original inlined code (removed in PostTyper)
* ReifyQuotes is no longer an InfoTransformer
* Splicer implements both checking and evaluation of splices in a
  single abstraction
* Fix scala#4773
* Fix scala#4735
* previously compiled and is present in the classpath of the current context.
*/
private class Interpreter(pos: Position, classLoader: ClassLoader)(implicit ctx: Context) {
type Res = Object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Res for "result"? I'd use the full name.

@@ -172,6 +158,15 @@ object Splicer {
ex.printStackTrace(new PrintWriter(sw))
sw.write("\n")
throw new StopInterpretation(sw.toString, pos)
case ex: InvocationTargetException =>
val sw = new StringWriter()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use a low-level method to compose the output? Is it to avoid a stackoverflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was just to pass it simply to the ex. printStackTrace method of Exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later I will need to improve the format of the output, then we will switch to a high level abstraction.

@nicolasstucki nicolasstucki merged commit 2e64369 into scala:master Jul 27, 2018
@Blaisorblade Blaisorblade deleted the interpret-macros branch July 30, 2018 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants