Skip to content

Can't use Symbol.tree to access DefDef body with blackbox macro using Tasty reflect #7592

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

Closed
fhackett opened this issue Nov 20, 2019 · 14 comments · Fixed by #11030
Closed

Can't use Symbol.tree to access DefDef body with blackbox macro using Tasty reflect #7592

fhackett opened this issue Nov 20, 2019 · 14 comments · Fixed by #11030

Comments

@fhackett
Copy link
Contributor

fhackett commented Nov 20, 2019

(you may recognise this from the Gitter chat, bumping because it seems to have gone unaddressed) never mind

minimized code

The macro:

  def compileImpl[T](expr : Expr[T])(given qctx : QuoteContext) : Expr[T] = {
    import qctx.tasty.{_,given}

    def proc(term : Term)(given outerCtx : Context) : Term = {
      given ctx : Context = term.symbol(given outerCtx).localContext(given outerCtx)
      term match {
        case Inlined(call, bindings, body) => Inlined(call, bindings, proc(body))
        case l : Literal => l
        case Block(statements, expr) =>
          proc(expr) // FIXME: very wrong
        case s : (Select|Ident) => {
          if( s.symbol.isDefDef ) {
            s.symbol.tree match {
              case DefDef(name, typeParams, params, returnTp, Some(rhs)) => proc(rhs)
            }
          } else {
            ???
          }
        }
      }
    }

    proc(expr.unseal).seal.asInstanceOf[Expr[T]]
  }

  inline def compile[T](expr : =>T) : T =
    ${ compileImpl('{ expr }) }

The usage (in a different package):

  def return1 = 1

  def testReturn1 = {
    assert(1 == compile(return1))
  }

expectation

The compile macro should be able to inspect the body of return1, allowing it to inline the method body into the macro expansion.

Currently the DefDef body is matched as None, preventing this. I know some DefDefs don't have bodies, but this one does. I'm at a loss as to how to achieve the effect I want.

Is there some undocumented thing I should be doing or is this a bug?

@fhackett
Copy link
Contributor Author

Tagging @nicolasstucki after some further discussion on Gitter

@fhackett
Copy link
Contributor Author

Also, for further reference adding that the MatchError I get when I run this code shows this tree: DefDef(return1,List(),List(),TypeTree[TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>),module scala),Int)],EmptyTree)

@nicolasstucki
Copy link
Contributor

This is a limitation of expanding macros during typed and not after. We have discussed the idea of only expanding white box macros in typed and delay the expansion of black box. That would fix this issue.

@nicolasstucki
Copy link
Contributor

When a tree is not available for a symbol we synthesize it with an empty body.

@fhackett
Copy link
Contributor Author

fhackett commented Nov 21, 2019

@nicolasstucki Thanks for the explanation.

This is a significant blocker for me - I would label current behaviour as very counter-intuitive, and it makes it effectively impossible to write interprocedural code folding macros. I am prepared to patch the compiler / implement the change you describe for black box macros myself, since I can't meaningfully move forward on my project without this kind of functionality. Let me know how you think it would be best to proceed.

@fhackett
Copy link
Contributor Author

fhackett commented Nov 21, 2019

Turns out there's a workaround available in the form of the -Yretain-trees option. Thanks to @smarter for the hint.

For those like me who didn't know how to pass options to Dotty in sbt:

scalacOptions += "-Yretain-trees"

@nicolasstucki
Copy link
Contributor

Even with that flag there might be local definitions that do not have a tree yet. @fhackett does that pose a blocker for you?

Doing the change I proposed might be a bit involved and might require some design decisions.

@fhackett
Copy link
Contributor Author

@nicolasstucki luckily I can live without the local definitions - generally I would expect expr to hold a very simple expression that refers to the actually important (non-local) code, similar to the example I give in this issue.

As it is, it seems I can keep going with my project with that little caveat in mind. That said, let me know if I can be of any help with design/test/etc. I'll also hopefully be able to link my full-scale use case soon (currently doing some design decisions of my own in that area).

@nicolasstucki
Copy link
Contributor

This is a starting point. There are still a few tests that need to be fixed. I will not have time to look at it this week. https://github.com/dotty-staging/dotty/tree/move-blackbox-macro-exapnsion-to-reify-quotes

@fhackett
Copy link
Contributor Author

@nicolasstucki thanks for the link - I have looked at it superficially, but will likewise have to get back to it.

meanwhile, here's a link to my work-in-progress that uses this feature (with the compiler flag)
https://github.com/fhackett/towers

@fhackett
Copy link
Contributor Author

fhackett commented Dec 19, 2019

@nicolasstucki with the current state of #7626 it seems I am once again at an impasse. I also have a much more precise idea of what I need that Dotty doesn't have. Pushing forward on my own project's design was worth it in that sense. Since I am not (yet) an official contributor, where should I start? I know about the CLA etc, I mean what would be the best use of my time for the issues I'm focusing on? I'm confident in my independent learning, but it would help a lot to have "go read this file/class/function" pointers or a description of what you've already done + what is still TODO.

Your thoughts are appreciated.

@nicolasstucki
Copy link
Contributor

A simple place to start which would have a high value would be #6280.

@fhackett
Copy link
Contributor Author

@nicolasstucki that like a good place to start. I see there is existing work - is there a specific branch I should start with, or do I just fork+branch from master and work from there?

@nicolasstucki
Copy link
Contributor

Just fork+branch from master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants