Skip to content

DefDef.copy does not allow changing anything other than function body #7626

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 27, 2019 · 4 comments
Closed
Assignees

Comments

@fhackett
Copy link
Contributor

minimized code

import quoted._

def fooImpl(given qctx : QuoteContext) : Expr[Int] = {
  import qctx.tasty.{_,given}
  val fn = '{
    def shouldBeHidden = ???
  }.unseal match {
    case Inlined(_, _, Block(List(theDefDef), _)) =>
      DefDef.copy(theDefDef)("realName", Nil, Nil, '[Int].unseal, Some('{1}.unseal))
  }
  Block(List(fn), Ref(fn.symbol)).seal.asInstanceOf[Expr[Int]]
}

inline def foo = ${fooImpl}

expectation

Referencing foo should produce the value 1 by generating both the definition of and a call to an inner function named realName that returns the corresponding value.

Instead, we get a ClassCastError at runtime because Dotty's codegen keeps using shouldBeHidden's return type of Nothing instead of the copy's updated Int type. Inspecting the bytecode shows that the name of the generated method is shouldBeHidden, not realName.

Similarly, attempting to change anything else (other than the definition body) about shouldBeHidden has little or no effect, leading to either compiler errors or incorrect codegen.

DefDef.copy seems like the easiest way to generate computed-arity (local) functions, so it's unfortunate that transforming things other than the function body does not work.

@fhackett
Copy link
Contributor Author

I have some follow-up thoughts from trying to work around this issue.

To properly generate a function with arbitrary arity you also need to make the parameters. If not, more internal compiler errors happen (Tasty pickler sees same symbol twice, is unhappy...).

If you have a function you want to replicate, I figured out you can call DefDef.apply instead and get fresh parameter symbols. That's nice and fixes half the problem.

The remaining problem is: how do we ask for a fresh function symbol? There are some useful transformations / refactors (think domain-specific inlining) you just can't write if macros are not allowed to properly make local copies of existing definitions.

That is, if we just had a variant of DefDef.apply that also generated a new Symbol (for local use only) then a lot of this issue would be resolved.

@nicolasstucki
Copy link
Contributor

Copy should not change the symbol. Indeed, what we need is DefDef.apply which requires the creation of a new symbol. This will be possible after #8090

@fhackett
Copy link
Contributor Author

@nicolasstucki Since this is evolving into more "making an API for generating new symbols", I've posted a new "Other" issue #8114 for discussing that and tracking further efforts.

Let me know what you think of the ideas going forward.

@nicolasstucki
Copy link
Contributor

Replaced by #8114. Now possible using DefDef.apply and Symbol.newMethod

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

No branches or pull requests

2 participants