Printer improvements to fuse JS emitting and printing #4920
Conversation
|
Full diff on helloworld: https://gist.github.com/gzm0/c8602ac8b4f928d6bc4598d04e18dee9 |
205b4a3 to
3370329
Compare
a0f0f72 to
a2706cd
Compare
| indent() | ||
| println() | ||
| indent() |
There was a problem hiding this comment.
This swap does not seem needed, and makes this spot inconsistent with other places that increase the indentation (for example printBlock). Consider reverting.
There was a problem hiding this comment.
The swap is indeed not needed (indent() and println() are now independent of each other). I originally did the swap since it seemed more logical (what a human would write, newline, then indent). However, it is indeed less consistent with the rest of the class (where the swap cannot be so easily done due to pre element println). So I think sticking with the existing order is fine.
linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala
Show resolved
Hide resolved
| fullLinkGz = 35000 to 36000, | ||
| )) | ||
|
|
||
| case `default213Version` => | ||
| Some(ExpectedSizes( | ||
| fastLink = 481000 to 483000, | ||
| fastLink = 479000 to 480000, |
There was a problem hiding this comment.
I'm surprised not to see changes in LibrarySizeTest. Weren't there any /*<skip>*/ there?
There was a problem hiding this comment.
They were within tolerance, but I have adjusted / tightened them.
We're going to do modifications, so we add some tests.
This is necesary for a subsequent change, where we pre-print statements into byte buffers. When we later assemble the statement into the surrounding block, we do not have type information anymore (and hence cannot judge whether a semicolon is necessary). Note that this now prints a semicolon after the last statement in a block (which we didn't previously). This does increase fastopt size (somewhat artificially), but more closely corresponds: - to what a human would write - the ECMAScript spec (e.g. https://tc39.es/ecma262/#sec-expression-statement)
This was forgotten in 1982a6b.
This was forgotten in 1982a6b.
Because it does not need to flatten blocks anymore, we can simply print it as a statement and add a trailing newline.
They were unnecessarily protected.
This is necessary so we can partially print substatements more easily (which we'll do in a subsequent PR). It will also allow us to fully remove the concept of "top-level" tree.
| fullLinkGz = 35000 to 36000, | ||
| )) | ||
|
|
||
| case `default213Version` => | ||
| Some(ExpectedSizes( | ||
| fastLink = 481000 to 483000, | ||
| fastLink = 479000 to 480000, |
There was a problem hiding this comment.
They were within tolerance, but I have adjusted / tightened them.
linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala
Show resolved
Hide resolved
| protected def printBlock(tree: Tree): Unit = { | ||
| print('{'); indent(); | ||
| private def printBlock(tree: Tree): Unit = { | ||
| print('{'); indent(); println() |
There was a problem hiding this comment.
Note: This change should be in the next commit. I've moved it.
| indent() | ||
| println() | ||
| indent() |
There was a problem hiding this comment.
The swap is indeed not needed (indent() and println() are now independent of each other). I originally did the swap since it seemed more logical (what a human would write, newline, then indent). However, it is indeed less consistent with the rest of the class (where the swap cannot be so easily done due to pre element println). So I think sticking with the existing order is fine.
The core of the change is to print semicolons and newlines as part of a statement.
This is necessary for a subsequent change, where we pre-print
statements into byte buffers. When we later assemble the statement
into the surrounding block, we do not have type information
anymore (and hence cannot judge whether a semicolon is necessary).
Note that this now prints a semicolon after the last statement in a
block (which we didn't previously). This does increase fastopt
size (somewhat artificially), but more closely corresponds: