-
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 all 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) | ||
val tree2 = transform(tree1) | ||
|
||
// due to value-discarding which converts an { e } into { e; () }) | ||
if (tree.tpe =:= defn.UnitType) Block(tree2 :: Nil, Literal(Constant(()))) | ||
else tree2 | ||
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 { | ||
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.
I don't know how performance sensitive this code is but you could return a
Tree
instead ofOption[Select]
to avoid boxing (andEmptyTree
instead ofNone
)Uh oh!
There was an error while loading. Please reload this page.
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.
We need that
.get
method onunapply
's return type so isn'tOption
unavoidable?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.
I believe
Tree
qualifies for both product pattern and name based pattern. And scalac supports name based pattern