Fix #3912: Account for units and blocks in inline and macros #4012
Conversation
| 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.
Can you give an example of when this happen?
There was a problem hiding this comment.
Yes, this is related to the implicit conversion related to value discarding (simply put when an e returns a value and it's called from a function of expected type Unit and it is converted into { e; () }). This manifested in quoted code as well.
There was a problem hiding this comment.
For example inline foo: Unit = 4 will become inline foo: Unit = { 4; () } and then be inlined as { 4; () }. In our case we have a ~xyz instead of 4.
There was a problem hiding this comment.
Got it! I would add a comment explaining why this is needed (i.e. the compiler can insert ()).
nicolasstucki
changed the title
| /** 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.
I don't know how performance sensitive this code is but you could return a Tree instead of Option[Select] to avoid boxing (and EmptyTree instead of None)
There was a problem hiding this comment.
We need that .get method on unapply's return type so isn't Option unavoidable?
There was a problem hiding this comment.
I believe Tree qualifies for both product pattern and name based pattern. And scalac supports name based pattern
|
What's the relationship with #4019? |
|
No relationship. |
|
Needs rebase |
| // 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) |
There was a problem hiding this comment.
This should be atree2
val tree2 = transform(tree1)
| 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.
And use tree2 here and in next line
|
Approve @nicolasstucki? ⭐️ |
No description provided.