-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #3912: Account for units and blocks in inline and macros #4012
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -321,16 +321,22 @@ class ReifyQuotes extends MacroTransformWithImplicits { | |
val last = enteredSyms | ||
stats.foreach(markDef) | ||
mapOverTree(last) | ||
case Inlined(call, bindings, expansion @ Select(body, name)) if expansion.symbol.isSplice => | ||
|
||
case Inlined(call, bindings, InlineSplice(expansion @ Select(body, name))) => | ||
// To maintain phase consistency, we move the binding of the this parameter into the spliced code | ||
val (splicedBindings, stagedBindings) = bindings.partition { | ||
case vdef: ValDef => vdef.symbol.is(Synthetic) // Assume that only _this bindings are tagged with Synthetic | ||
case _ => false | ||
} | ||
|
||
val tree1 = | ||
if (level == 0) cpy.Inlined(tree)(call, stagedBindings, Splicer.splice(seq(splicedBindings, body))) | ||
else seq(stagedBindings, cpy.Select(expansion)(cpy.Inlined(tree)(call, splicedBindings, body), name)) | ||
transform(tree1) | ||
|
||
// due to value-discarding which converts an { e } into { e; () }) | ||
if (tree.tpe =:= defn.UnitType) Block(tree1 :: Nil, Literal(Constant(()))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And use |
||
else tree1 | ||
case _: Import => | ||
tree | ||
case tree: DefDef if tree.symbol.is(Macro) && level == 0 => | ||
|
@@ -343,5 +349,20 @@ class ReifyQuotes extends MacroTransformWithImplicits { | |
checkLevel(mapOverTree(enteredSyms)) | ||
} | ||
} | ||
|
||
/** InlineSplice is used to detect cases where the expansion | ||
* consists of a (possibly multiple & nested) block or a sole expression. | ||
*/ | ||
object InlineSplice { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know how performance sensitive this code is but you could return a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe |
||
def unapply(tree: Tree)(implicit ctx: Context): Option[Select] = { | ||
tree match { | ||
case expansion: Select if expansion.symbol.isSplice => | ||
Some(expansion) | ||
case Block(List(stat), Literal(Constant(()))) => unapply(stat) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you give an example of when this happen? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is related to the implicit conversion related to value discarding (simply put when an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it! I would add a comment explaining why this is needed (i.e. the compiler can insert |
||
case Block(Nil, expr) => unapply(expr) | ||
case _ => None | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import scala.quoted._ | ||
|
||
object Macros { | ||
inline def foo(): Int = { ~impl() } | ||
|
||
def impl(): Expr[Int] = '(1) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import scala.quoted._ | ||
import Macros._ | ||
|
||
class Test { | ||
val a: Unit = foo() | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import scala.quoted._ | ||
|
||
object Macros { | ||
inline def foo2(): Unit = ~impl() | ||
|
||
def impl(): Expr[Int] = '(1) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import scala.quoted._ | ||
import Macros._ | ||
|
||
class Test { | ||
val a2: Unit = foo2() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import scala.quoted._ | ||
|
||
object Macros { | ||
inline def foo3(): Int = { | ||
{ | ||
~impl() | ||
} | ||
} | ||
|
||
def impl(): Expr[Int] = '(1) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import scala.quoted._ | ||
import Macros._ | ||
|
||
class Test { | ||
val a3: Unit = foo3() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a
tree2
val tree2 = transform(tree1)