Skip to content

Enable a compiler plugin to use the async transform after patmat #141

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

Merged
merged 2 commits into from
Sep 24, 2015

Conversation

retronym
Copy link
Member

Currently, the async transformation is performed during the typer
phase, like all other macros.

We have to levy a few artificial restrictions on whern an async
boundary may be: for instance we don't support await within a
pattern guard. A more natural home for the transform would be
after patterns have been translated.

The test case in this commit shows how to use the async transform
from a custom compiler phase after patmat.

The remainder of the commit updates the implementation to handle
the new tree shapes.

For states that correspond to a label definition, we use -symbol.id
as the state ID. This made it easier to emit the forward jumps to when
processing the label application before we had seen the label
definition.

I've also made the transformation more efficient in the way it checks
whether a given tree encloses an await call: we traverse the input
tree at the start of the macro, and decorate it with tree attachments
containig the answer to this question. Even after the ANF and state
machine transforms introduce new layers of synthetic trees, the
containsAwait code need only traverse shallowly through those
trees to find a child that has the cached answer from the original
traversal.

I had to special case the ANF transform for expressions that always
lead to a label jump: we avoids trying to push an assignment to a result
variable into if (cond) jump1() else jump2(), in trees of the form:

% cat sandbox/jump.scala
class Test {
  def test = {
    (null: Any) match {
      case _: String => ""
      case _ => ""
    }
  }
}

% qscalac -Xprint:patmat -Xprint-types sandbox/jump.scala
    def test: String = {
      case <synthetic> val x1: Any = (null{Null(null)}: Any){Any};
      case5(){
        if (x1.isInstanceOf{[T0]=> Boolean}[String]{Boolean})
          matchEnd4{(x: String)String}(""{String("")}){String}
        else
          case6{()String}(){String}{String}
      }{String};
      case6(){
        matchEnd4{(x: String)String}(""{String("")}){String}
      }{String};
      matchEnd4(x: String){
        x{String}
      }{String}
    }{String}

Currently, the async transformation is performed during the typer
phase, like all other macros.

We have to levy a few artificial restrictions on whern an async
boundary may be: for instance we don't support await within a
pattern guard. A more natural home for the transform would be
after patterns have been translated.

The test case in this commit shows how to use the async transform
from a custom compiler phase after patmat.

The remainder of the commit updates the implementation to handle
the new tree shapes.

For states that correspond to a label definition, we use `-symbol.id`
as the state ID. This made it easier to emit the forward jumps to when
processing the label application before we had seen the label
definition.

I've also made the transformation more efficient in the way it checks
whether a given tree encloses an `await` call: we traverse the input
tree at the start of the macro, and decorate it with tree attachments
containig the answer to this question. Even after the ANF and state
machine transforms introduce new layers of synthetic trees, the
`containsAwait` code need only traverse shallowly through those
trees to find a child that has the cached answer from the original
traversal.

I had to special case the ANF transform for expressions that always
lead to a label jump: we avoids trying to push an assignment to a result
variable into `if (cond) jump1() else jump2()`, in trees of the form:

```
% cat sandbox/jump.scala
class Test {
  def test = {
    (null: Any) match {
      case _: String => ""
      case _ => ""
    }
  }
}

% qscalac -Xprint:patmat -Xprint-types sandbox/jump.scala
    def test: String = {
      case <synthetic> val x1: Any = (null{Null(null)}: Any){Any};
      case5(){
        if (x1.isInstanceOf{[T0]=> Boolean}[String]{Boolean})
          matchEnd4{(x: String)String}(""{String("")}){String}
        else
          case6{()String}(){String}{String}
      }{String};
      case6(){
        matchEnd4{(x: String)String}(""{String("")}){String}
      }{String};
      matchEnd4(x: String){
        x{String}
      }{String}
    }{String}
```
@@ -259,6 +272,44 @@ private[async] trait AnfTransform {
}
}

// Replace the label parameters on `matchEnd` with use of a `matchRes` temporary variable
Copy link
Member

Choose a reason for hiding this comment

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

maybe a bit of clarification here

  • the method name suggests that it eliminates multiple label parameters, but it expects there to be one
  • quickly recap the structure generated by patmat. is the following correct?
    • normal case defs are label defs without parameters.
    • the single matchEnd label def has excatly one (zero or one? it seems to have one also for unit-typed matches) parameter.

@lrytz
Copy link
Member

lrytz commented Sep 23, 2015

great work, looks very good!

 - More internal docs
 - Be more frugal with the `NoAwait` attachment, for some AST node
   types this is implied.
 - Just use `x`, rather than what was effectively
   `x.reverseMap(identity).reverse`
@retronym retronym force-pushed the ticket/await-extractor branch from 7265132 to 168e10c Compare September 23, 2015 12:15
@retronym
Copy link
Member Author

Thanks for the review, @lrytz. I've incorporated your suggestions.

retronym added a commit that referenced this pull request Sep 24, 2015
Enable a compiler plugin to use the async transform after patmat
@retronym retronym merged commit 7263aaa into scala:master Sep 24, 2015
@retronym retronym mentioned this pull request Jan 19, 2016
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.

2 participants