Skip to content

passing generate lambda as macro parameter produce tree with incorrect owner #11401

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.

Sign up for GitHub

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

Jump to bottom
Closed
rssh opened this issue Feb 13, 2021 · 1 comment · Fixed by #11552
Closed

passing generate lambda as macro parameter produce tree with incorrect owner #11401

rssh opened this issue Feb 13, 2021 · 1 comment · Fixed by #11552
Labels
itype:bug itype:crash
Milestone
3.0.0

Comments

@rssh
Copy link
Contributor

rssh commented Feb 13, 2021

Compiler version

Latest Dotty nightly build version: 3.0.0-M4-bin-20210212-0273336-NIGHTLY

Minimized code

X.scala

package x

import scala.quoted._

import scala.concurrent.Future

def await[T](x:Future[T]):T = ???

class CIFReader[A](a:A)


class SLSelect[S]:

  def onRead[A](ch: CIFReader[A])(f: A=> S): this.type =
      ???

  def fold[S](s0:S)(step: (S,SLSelect[S])=> S): S = {
     ???
  }

  def fold_async[S](s0:S)(step: (S,SLSelect[S])=> Future[S]): Future[S] = {
     ???
  }

  inline def apply1[A](inline ch: CIFReader[A], f: A=>S): S =
      val s0 = new SLSelect[S]
      await(s0.onRead(ch)(f).runAsync())

  def runAsync(): Future[S] = ???


object X:

 inline def process[T](inline f:T) = ${
   processImpl[T]('f)
 }

 def processImpl[T:Type](t:Expr[T])(using Quotes):Expr[Future[T]] =
   import quotes.reflect._
   val r = processTree[T](t.asTerm)
   r.asExprOf[Future[T]]


 def processTree[T:Type](using Quotes)(t: quotes.reflect.Term):quotes.reflect.Term =
   import quotes.reflect._
   val r: Term = t match
     case Inlined(_,List(),body) => processTree(body)
     case Inlined(d,bindings,body) =>
       Inlined(d,bindings,processTree[T](body))
     case Block(stats,expr) => Block(stats,processTree(expr))
     case Apply(Apply(TypeApply(Select(x,"fold"),targs),List(state)),List(fun)) =>
       val nFun = processLambda[T](fun)
       Apply(Apply(TypeApply(Select.unique(x,"fold_async"),targs),List(state)),List(nFun))
     case Apply(TypeApply(Ident("await"),targs),List(body)) => body
     case Typed(x,tp) => Typed(processTree(x), Inferred(TypeRepr.of[Future].appliedTo(tp.tpe)) )
     case _ => throw new RuntimeException(s"tree not recoginized: $t")
   val checker = new TreeMap() {}
   checker.transformTerm(r)(Symbol.spliceOwner)
   r


 def processLambda[T:Type](using Quotes)(fun: quotes.reflect.Term):quotes.reflect.Term =
   import quotes.reflect._

   def changeArgs(oldArgs:List[Tree], newArgs:List[Tree], body:Term, owner: Symbol):Term =
         val association: Map[Symbol, Term] = (oldArgs zip newArgs).foldLeft(Map.empty){
             case (m, (oldParam, newParam: Term)) => m.updated(oldParam.symbol, newParam)
             case (m, (oldParam, newParam: Tree)) => throw RuntimeException("Term expected")
         }
         val changes = new TreeMap() {
             override def transformTerm(tree:Term)(owner: Symbol): Term =
               tree match
                 case ident@Ident(name) => association.getOrElse(ident.symbol, super.transformTerm(tree)(owner))
                 case _ => super.transformTerm(tree)(owner)
         }
         changes.transformTerm(body)(owner)

   val r = fun match
     case Lambda(params, body) =>
            val nBody = processTree[T](body)
            val paramTypes = params.map(_.tpt.tpe)
            val paramNames = params.map(_.name)
            val mt = MethodType(paramNames)(_ => paramTypes, _ => TypeRepr.of[Future].appliedTo(body.tpe.widen) )
            val r = Lambda(Symbol.spliceOwner, mt, (owner,args) => changeArgs(params,args,nBody,owner) )
            r
     case Block(List(),expr) => processLambda(expr)
     case _ =>
          throw new RuntimeException(s"Lambda expected")
   r

Main.scala

package x

object Main {

 def main(args:Array[String]):Unit =
     val in = new CIFReader[Boolean](true)
     val select = new SLSelect[Unit]()

     val generator = X.process {
         select.fold(in){ (ch,s) =>
            s.apply1(ch, v=>ch)
         }
     }
     assert(true)

}

Compile options should include "-Ycheck:macros"

Output (click arrow to expand)

[error] -- Error: /Users/rssh/tests/dotty/crash-fold/src/main/scala/x/Main.scala:11:31 -
[error] 11 |     val generator = X.process {
[error]    |                     ^
[error]    |Exception occurred while executing macro expansion.
[error]    |java.lang.AssertionError: assertion failed: Tree had an unexpected owner for method $anonfun
[error]    |Expected: val f$proxy3 (x.Main$._$_$_$_$f$proxy3)
[error]    |But was: method $anonfun (x.Main$._$_$_$$anonfun)
[error]    |
[error]    |
[error]    |The code of the definition of method $anonfun is
[error]    |def $anonfun(v: scala.Boolean): x.CIFReader[scala.Boolean] = ch
[error]    |
[error]    |which was found in the code
[error]    |((v: scala.Boolean) => ch)
[error]    |
[error]    |which has the AST representation
[error]    |Block(List(DefDef("$anonfun", List(TermParamClause(List(ValDef("v", Inferred(), None)))), Inferred(), Some(Ident("ch")))), Closure(Ident("$anonfun"), None))
[error]    |
[error]    |
[error]    |	at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
[error]    |	at scala.quoted.runtime.impl.QuotesImpl$$anon$9.traverse(QuotesImpl.scala:2827)
[error]    |	at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1571)
[error]    |	at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1571)
[error]    |	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.fold$1(Trees.scala:1444)
[error]    |	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.apply(Trees.scala:1446)
[error]    |	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1479)
[error]    |	at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1572)
[error]    |	at scala.quoted.runtime.impl.QuotesImpl$$anon$9.traverse(QuotesImpl.scala:2830)
[error]    |	at scala.quoted.runtime.impl.QuotesImpl$reflect$.yCheckOwners(QuotesImpl.scala:2831)
[error]    |	at scala.quoted.runtime.impl.QuotesImpl$reflect$.scala$quoted$runtime$impl$QuotesImpl$reflect$$$yCheckedOwners(QuotesImpl.scala:2793)
[error]    |	at scala.quoted.runtime.impl.QuotesImpl$reflect$ValDef$.copy(QuotesImpl.scala:298)
[error]    |	at scala.quoted.runtime.impl.QuotesImpl$reflect$ValDef$.copy(QuotesImpl.scala:297)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformStatement(Quotes.scala:4279)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformStatement$(Quotes.scala:4242)
[error]    |	at x.X$$anon$1.transformStatement(X.scala:58)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformTree(Quotes.scala:4253)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformTree$(Quotes.scala:4242)
[error]    |	at x.X$$anon$1.transformTree(X.scala:58)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformTrees$$anonfun$1(Quotes.scala:4383)
[error]    |	at scala.collection.immutable.List.mapConserve(List.scala:472)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformTrees(Quotes.scala:4383)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformTrees$(Quotes.scala:4242)
[error]    |	at x.X$$anon$1.transformTrees(X.scala:58)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformSubTrees(Quotes.scala:4398)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformSubTrees$(Quotes.scala:4242)
[error]    |	at x.X$$anon$1.transformSubTrees(X.scala:58)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformTerm(Quotes.scala:4340)
[error]    |	at scala.quoted.Quotes$reflectModule$TreeMap.transformTerm$(Quotes.scala:4242)
[error]    |	at x.X$$anon$1.transformTerm(X.scala:58)
[error]    |	at x.X$.processTree(X.scala:59)
[error]    |	at x.X$.processTree(X.scala:51)
[error]    |	at x.X$.processLambda(X.scala:80)
[error]    |	at x.X$.processLambda(X.scala:86)
[error]    |	at x.X$.processTree(X.scala:53)
[error]    |	at x.X$.processTree(X.scala:51)
[error]    |	at x.X$.processTree(X.scala:48)
[error]    |	at x.X$.processImpl(X.scala:41)
[error]    |
[error]    | This location contains code that was inlined from Main.scala:11
[error] 12 |         select.fold(in){ (ch,s) =>
[error] 13 |            s.apply1(ch, v=>ch)
[error] 14 |         }
[error] 15 |     }
[error] one error found
[error] one error found
[error] (Compile / compileIncremental) Compilation failed
[error] Total time: 4 s, completed 13 Feb 2021, 11:06:59
@rssh rssh changed the title passing generate lambda as parameter produce tree with incorrect owner passing generate lambda as macro parameter produce tree with incorrect owner Feb 13, 2021
@rssh
Copy link
Contributor Author

rssh commented Feb 27, 2021

It was a combination of bug in compiler and bug in test-case.
The correct X_1.scala should looks like:

object X:

 inline def process[T](inline f:T) = ${
   processImpl[T]('f)
 }

 def processImpl[T:Type](t:Expr[T])(using Quotes):Expr[Future[T]] =
   import quotes.reflect._
   val r = processTree[T](t.asTerm)
   r.asExprOf[Future[T]]


 def processTree[T:Type](using Quotes)(t: quotes.reflect.Term):quotes.reflect.Term =
   import quotes.reflect._
   val r: Term = t match
     case Inlined(_,List(),body) => processTree(body)
     case Inlined(d,bindings,body) =>
       Inlined(d,bindings,processTree[T](body))
     case Block(stats,expr) => Block(stats,processTree(expr))
     case Apply(Apply(TypeApply(Select(x,"fold"),targs),List(state)),List(fun)) =>
       val nFun = processLambda[T](fun)
       Apply(Apply(TypeApply(Select.unique(x,"fold_async"),targs),List(state)),List(nFun))
     case Apply(TypeApply(Ident("await"),targs),List(body)) => body
     case Typed(x,tp) => Typed(processTree(x), Inferred(TypeRepr.of[Future].appliedTo(tp.tpe)) )
     case _ => throw new RuntimeException(s"tree not recoginized: $t")
     val checker = new TreeMap() {}
     checker.transformTerm(r)(Symbol.spliceOwner)
     r

 def processLambda[T:Type](using Quotes)(fun: quotes.reflect.Term):quotes.reflect.Term =
   import quotes.reflect._

   def changeArgs(oldArgs:List[Tree], newArgs:List[Tree], body:Term, owner: Symbol):Term =
         val association: Map[Symbol, Term] = (oldArgs zip newArgs).foldLeft(Map.empty){
             case (m, (oldParam, newParam: Term)) => m.updated(oldParam.symbol, newParam)
             case (m, (oldParam, newParam: Tree)) => throw RuntimeException("Term expected")
         }
         val changes = new TreeMap() {
             override def transformTerm(tree:Term)(owner: Symbol): Term =
               tree match
                 case ident@Ident(name) => association.getOrElse(ident.symbol, super.transformTerm(tree)(owner))
                 case _ => super.transformTerm(tree)(owner)
         }
         changes.transformTerm(body)(owner)

   val r = fun match
     case Lambda(params, body) =>
            val nBody = processTree[T](body)
            val paramTypes = params.map(_.tpt.tpe)
            val paramNames = params.map(_.name)
            val mt = MethodType(paramNames)(_ => paramTypes, _ => TypeRepr.of[Future].appliedTo(body.tpe.widen) )
            val r = Lambda(Symbol.spliceOwner, mt, (owner,args) => changeArgs(params,args,nBody,owner).changeOwner(owner) )
            r
     case Block(List(),expr) => processLambda(expr)
     case _ =>
          throw new RuntimeException(s"Lambda expected")
   r

(difference is additional changeOwner after changeArgs)

will submit fix with correct test-case

rssh added a commit to rssh/dotty that referenced this issue Feb 27, 2021
@rssh
fix scala#11401 (proxy parameter should be owned by proxy)
1412897
rssh added a commit to rssh/dotty that referenced this issue Feb 27, 2021
@rssh
fix scala#11401 (proxy parameter should be owned by proxy)
3f8abf9
rssh added a commit to rssh/dotty that referenced this issue Feb 27, 2021
@rssh
fix scala#11401 (proxy parameter should be owned by proxy)
0241150 
 (since boundSym is new, no need to use check in QuoteUtils.changeOwner)
@nicolasstucki nicolasstucki linked a pull request Mar 1, 2021 that will close this issue
fix #11401 (proxy parameter should be owned by proxy) #11552
Merged
nicolasstucki added a commit that referenced this issue Mar 1, 2021
@nicolasstucki
Merge pull request #11552 from rssh/fix-11401
7bef33b 
 fix #11401 (proxy parameter should be owned by proxy)
JakkuSakura added a commit to JakkuSakura/dotty that referenced this issue Feb 14, 2022
@JakkuSakura
Revert " fix scala#11401 (proxy parameter should be owned by proxy) (…
17cf73d 
…since boundSym is new, no need to use check in QuoteUtils.changeOwner)"

This reverts commit 0241150
This might help fix scala#12508
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
itype:bug itype:crash
Projects
None yet
Milestone
3.0.0
Development

Successfully merging a pull request may close this issue.

fix #11401 (proxy parameter should be owned by proxy) rssh/dotty
2 participants
@rssh @Kordyjan