Skip to content

Avoid printing extra blocks and inline nodes #5510

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,8 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
case SeqLiteral(elems, elemtpt) =>
"[" ~ toTextGlobal(elems, ",") ~ " : " ~ toText(elemtpt) ~ "]"
case tree @ Inlined(call, bindings, body) =>
(("/* inlined from " ~ toText(call) ~ " */ ") `provided`
!call.isEmpty && !homogenizedView && !ctx.settings.YshowNoInline.value) ~
(("/* inlined from " ~ (if (call.isEmpty) "outside" else toText(call)) ~ " */ ") `provided`
!homogenizedView && !ctx.settings.YshowNoInline.value) ~
blockText(bindings :+ body)
case tpt: untpd.DerivedTypeTree =>
"<derived typetree watching " ~ summarized(toText(tpt.watched)) ~ ">"
Expand Down
4 changes: 0 additions & 4 deletions compiler/test/dotc/pos-recompilation.whitelist
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ CoderTrait
collectGenericCC
collections_1
comp-rec-test
companions
compile1
conforms
conformsWild
Expand Down Expand Up @@ -317,7 +316,6 @@ i831
i864
i877
i878
i880
i903
i938
i939
Expand Down Expand Up @@ -438,7 +436,6 @@ propagate
quote-1
quote-lift-inline-params-b
quote-non-static-macro
quote-this
range
rangepos
rangepos-anonapply
Expand Down Expand Up @@ -883,7 +880,6 @@ t613
t6145
t6146
t615
t6157
t616
t6184
t6201
Expand Down
115 changes: 85 additions & 30 deletions library/src/scala/tasty/reflect/Printers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,17 @@ trait Printers
printTree(body)
}

case IsDefDef(ddef @ DefDef(name, targs, argss, _, rhsOpt)) if name.startsWith("$anonfun") =>
// Decompile lambda definition
assert(targs.isEmpty)
val args :: Nil = argss
val Some(rhs) = rhsOpt
inParens {
printArgsDefs(args)
this += " => "
printTree(rhs)
}

case IsDefDef(ddef @ DefDef(name, targs, argss, tpt, rhs)) =>
printDefAnnotations(ddef)

Expand Down Expand Up @@ -777,34 +788,13 @@ trait Printers
case IsValDef(tree) => !tree.symbol.flags.isObject
case _ => true
}
printFlatBlock(stats, expr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code seems to have forgotten the case implicit object A.


expr match {
case Term.Lambda(_, _) =>
// Decompile lambda from { def annon$(...) = ...; closure(annon$, ...)}
assert(stats.size == 1)
val DefDef(_, _, args :: Nil, _, Some(rhs)) :: Nil = stats
inParens {
printArgsDefs(args)
this += " => "
printTree(rhs)
}
case _ =>
this += "{"
indented {
printStats(stats, expr)
}
this += lineBreak() += "}"
}

case Term.Inlined(call, bindings, expansion) => // FIXME: Don't print Inlined with empty calls?
this += "{ // inlined"
indented {
printStats(bindings, expansion)
}
this += lineBreak() += "}"
case Term.Inlined(_, bindings, expansion) =>
printFlatBlock(bindings, expansion)

case Term.Lambda(meth, tpt) =>
// Printed in Term.Block branch
// Printed in by it's DefDef
this

case Term.If(cond, thenp, elsep) =>
Expand Down Expand Up @@ -847,15 +837,80 @@ trait Printers

}

def flatBlock(stats: List[Statement], expr: Term): (List[Statement], Term) = {
val flatStats = List.newBuilder[Statement]
def extractFlatStats(stat: Statement): Unit = stat match {
case Term.Block(stats1, expr1) =>
val it = stats1.iterator
while (it.hasNext)
extractFlatStats(it.next())
extractFlatStats(expr1)
case Term.Inlined(_, bindings, expansion) =>
val it = bindings.iterator
while (it.hasNext)
extractFlatStats(it.next())
extractFlatStats(expansion)
case Term.Literal(Constant.Unit()) => // ignore
case stat => flatStats += stat
}
def extractFlatExpr(term: Term): Term = term match {
case Term.Block(stats1, expr1) =>
val it = stats1.iterator
while (it.hasNext)
extractFlatStats(it.next())
extractFlatExpr(expr1)
case Term.Inlined(_, bindings, expansion) =>
val it = bindings.iterator
while (it.hasNext)
extractFlatStats(it.next())
extractFlatExpr(expansion)
case term => term
}
val it = stats.iterator
while (it.hasNext)
extractFlatStats(it.next())
val flatExpr = extractFlatExpr(expr)
(flatStats.result(), flatExpr)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code smell for flatBlock: is there a way to simplify the code and logic?


def printFlatBlock(stats: List[Statement], expr: Term): Buffer = {
val (stats1, expr1) = flatBlock(stats, expr)
// Remove Term.Lambda nodes, lambdas are printed by their definition
val stats2 = stats1.filter { case Term.Lambda(_, _) => false; case _ => true }
val (stats3, expr3) = expr1 match {
case Term.Lambda(_, _) =>
val init :+ last = stats2
(init, last)
case _ => (stats2, expr1)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Remove Term.Lambda logic seems to belong to somewhere else .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the decompiler, Term.Lambda are no-op/no-print trees that must be removed from the blocks. We need to remove them here because they might affect the block flattening.

if (stats3.isEmpty) {
printTree(expr3)
} else {
this += "{"
indented {
printStats(stats3, expr3)
}
this += lineBreak() += "}"
}
}

def printStats(stats: List[Tree], expr: Tree): Unit = {
def printSeparator(next: Tree): Unit = {
// Avoid accidental application of opening `{` on next line with a double break
def rec(next: Tree): Unit = next match {
case Term.Block(stats, _) if stats.nonEmpty => this += doubleLineBreak()
case Term.Inlined(_, bindings, _) if bindings.nonEmpty => this += doubleLineBreak()
case Term.Select(qual, _) => rec(qual)
case Term.Apply(fn, _) => rec(fn)
case Term.TypeApply(fn, _) => rec(fn)
case _ => this += lineBreak()
}
next match {
case Term.Block(_, _) => this += doubleLineBreak()
case Term.Inlined(_, _, _) => this += doubleLineBreak()
case Term.Select(qual, _) => printSeparator(qual)
case Term.Apply(fn, _) => printSeparator(fn)
case Term.TypeApply(fn, _) => printSeparator(fn)
case IsTerm(term) =>
flatBlock(Nil, term) match {
case (next :: _, _) => rec(next)
case (Nil, next) => rec(next)
}
case _ => this += lineBreak()
}
}
Expand Down
10 changes: 4 additions & 6 deletions tests/pos/i2104b.decompiled
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,8 @@ case class Pair[A, B](_1: A, _2: B) {
object Pair extends scala.AnyRef()
/** Decompiled from out/posTestFromTasty/pos/i2104b/Test.class */
object Test {
def main(args: scala.Array[scala.Predef.String]): scala.Unit = {
Cons.apply[scala.Option[scala.Int], scala.None.type](scala.Option.apply[scala.Int](1), scala.None) match {
case Cons(scala.Some(i), scala.None) =>
()
}
def main(args: scala.Array[scala.Predef.String]): scala.Unit = Cons.apply[scala.Option[scala.Int], scala.None.type](scala.Option.apply[scala.Int](1), scala.None) match {
case Cons(scala.Some(i), scala.None) =>
()
}
}
}
12 changes: 5 additions & 7 deletions tests/pos/i4526b.decompiled
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
/** Decompiled from out/posTestFromTasty/pos/i4526b/Foo.class */
class Foo() {
def justdoit(f: scala.Either[scala.Int, scala.Predef.String]): scala.Predef.String = {
f match {
case scala.Left(i) =>
i.toString()
case scala.Right(s) =>
(s: java.lang.String)
}
def justdoit(f: scala.Either[scala.Int, scala.Predef.String]): scala.Predef.String = f match {
case scala.Left(i) =>
i.toString()
case scala.Right(s) =>
(s: java.lang.String)
}
}
6 changes: 1 addition & 5 deletions tests/pos/lambda.decompiled
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
/** Decompiled from out/posTestFromTasty/pos/lambda/foo/Foo.class */
package foo {
class Foo() {
{
((x: scala.Int) => {
2
})
}
((x: scala.Int) => 2)
val a: scala.Function1[scala.Int, scala.Int] = ((x: scala.Int) => x.*(x))
}
}
4 changes: 1 addition & 3 deletions tests/pos/simpleDoWhile.decompiled
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
class Foo() {
def foo: scala.Unit = {
var i: scala.Int = 1
do {
i = 0
} while (i.!=(0))
do i = 0 while (i.!=(0))
}
}
4 changes: 1 addition & 3 deletions tests/pos/simpleInline.decompiled
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
/** Decompiled from out/posTestFromTasty/pos/simpleInline/Foo.class */
class Foo() {
inline def foo: scala.Int = (9: scala.Int)
def bar: scala.Int = { // inlined
(9: scala.Int)
}
def bar: scala.Int = (9: scala.Int)
}
10 changes: 4 additions & 6 deletions tests/pos/simpleMatchCase.decompiled
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
/** Decompiled from out/posTestFromTasty/pos/simpleMatchCase/Foo.class */
class Foo() {
def foo: scala.Unit = {
"c" match {
case x =>
scala.Predef.println("a")
scala.Predef.println("b")
}
def foo: scala.Unit = "c" match {
case x =>
scala.Predef.println("a")
scala.Predef.println("b")
}
}
4 changes: 1 addition & 3 deletions tests/pos/simpleWhile.decompiled
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
class Foo() {
def foo: scala.Unit = {
var i: scala.Int = 1
while (i.!=(0)) {
i = 0
}
while (i.!=(0)) i = 0
}
}
4 changes: 1 addition & 3 deletions tests/pos/t3869.decompiled
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
/** Decompiled from out/posTestFromTasty/pos/t3869/Test.class */
object Test {
def f: scala.Unit = try return () finally while (true) ()
def main(args: scala.Array[scala.Predef.String]): scala.Unit = {
Test.f
}
def main(args: scala.Array[scala.Predef.String]): scala.Unit = Test.f
}
7 changes: 2 additions & 5 deletions tests/pos/t704.decompiled
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,8 @@ trait E() extends java.lang.Object with D {
val x1: scala.AnyRef = E.this.get_xxxx
scala.Console.println(y)
}

{
yyyy
()
}
yyyy
()
}
}
/** Decompiled from out/posTestFromTasty/pos/t704/Go.class */
Expand Down
4 changes: 1 addition & 3 deletions tests/run-with-compiler/quote-impure-by-name.check
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
1 + {{ // inlined
Index.zero["bar", scala.Tuple2["baz", scala.Unit]]
}}
1 + {Index.zero["bar", scala.Tuple2["baz", scala.Unit]]}
1 change: 0 additions & 1 deletion tests/run-with-compiler/quote-nested-3.check
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,5 @@
type T = scala.Predef.String
val x: java.lang.String = "foo"
val z: T = x
()
(x: java.lang.String)
}
36 changes: 0 additions & 36 deletions tests/run-with-compiler/quote-show-blocks-raw.check

This file was deleted.

25 changes: 0 additions & 25 deletions tests/run-with-compiler/quote-show-blocks-raw.scala

This file was deleted.

Loading