From e55b62448b4508a13a2f28e9b0dbc4922931d8d2 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Thu, 28 Dec 2023 16:32:42 +0100 Subject: [PATCH 1/9] Basic tests for javascript.Printers We're going to do modifications, so we add some tests. --- .../backend/javascript/PrintersTest.scala | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala diff --git a/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala new file mode 100644 index 0000000000..c43b1a28e9 --- /dev/null +++ b/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala @@ -0,0 +1,164 @@ +/* + * Scala.js (https://www.scala-js.org/) + * + * Copyright EPFL. + * + * Licensed under Apache License 2.0 + * (https://www.apache.org/licenses/LICENSE-2.0). + * + * See the NOTICE file distributed with this work for + * additional information regarding copyright ownership. + */ + +package org.scalajs.linker.backend.javascript + +import scala.language.implicitConversions + +import java.nio.charset.StandardCharsets + +import org.junit.Test +import org.junit.Assert._ + +import org.scalajs.ir + +import Trees._ + +class PrintersTest { + + private implicit val pos: ir.Position = ir.Position.NoPosition + + private implicit def str2ident(name: String): Ident = + Ident(name, ir.OriginalName.NoOriginalName) + + private def assertPrintEquals(expected: String, tree: Tree): Unit = { + val out = new ByteArrayWriter + val printer = new Printers.JSTreePrinter(out) + printer.printTopLevelTree(tree) + assertEquals(expected.stripMargin.trim + "\n", + new String(out.toByteArray(), StandardCharsets.UTF_8)) + } + + @Test def printFunctionDef(): Unit = { + assertPrintEquals( + """ + |function test() { + | const x = 2; + | return x + |} + """, + FunctionDef("test", Nil, None, Block( + Let("x", mutable = false, Some(IntLiteral(2))), + Return(VarRef("x")))) + ) + + assertPrintEquals( + """ + |function test() { + | /**/ + |} + """, + FunctionDef("test", Nil, None, Skip()) + ) + } + + @Test def printClassDef(): Unit = { + assertPrintEquals( + """ + |class MyClass extends foo.Other { + |} + """, + ClassDef(Some("MyClass"), Some(DotSelect(VarRef("foo"), "Other")), Nil) + ) + + assertPrintEquals( + """ + |class MyClass { + | foo() { + | /**/ + | }; + | get a() { + | return 1 + | }; + | set a(x) { + | /**/ + | }; + |} + """, + ClassDef(Some("MyClass"), None, List( + MethodDef(false, "foo", Nil, None, Skip()), + GetterDef(false, "a", Return(IntLiteral(1))), + SetterDef(false, "a", ParamDef("x"), Skip()) + )) + ) + } + + @Test def printDocComment(): Unit = { + assertPrintEquals( + """ + | /** test */ + """, + DocComment("test") + ) + } + + @Test def printFor(): Unit = { + assertPrintEquals( + """ + |for (let x = 1; (x < 15); x = (x + 1)) { + | /**/ + |}; + """, + For(Let("x", true, Some(IntLiteral(1))), + BinaryOp(ir.Trees.JSBinaryOp.<, VarRef("x"), IntLiteral(15)), + Assign(VarRef("x"), BinaryOp(ir.Trees.JSBinaryOp.+, VarRef("x"), IntLiteral(1))), + Skip()) + ) + } + + @Test def printForIn(): Unit = { + assertPrintEquals( + """ + |for (var x in foo) { + | /**/ + |}; + """, + ForIn(VarDef("x", None), VarRef("foo"), Skip()) + ) + } + + @Test def printIf(): Unit = { + assertPrintEquals( + """ + |if (false) { + | 1 + |}; + """, + If(BooleanLiteral(false), IntLiteral(1), Skip()) + ) + + assertPrintEquals( + """ + |if (false) { + | 1 + |} else { + | 2 + |}; + """, + If(BooleanLiteral(false), IntLiteral(1), IntLiteral(2)) + ) + + assertPrintEquals( + """ + |if (false) { + | 1 + |} else if (true) { + | 2 + |} else { + | 3 + |}; + """, + If(BooleanLiteral(false), IntLiteral(1), + If(BooleanLiteral(true), IntLiteral(2), IntLiteral(3))) + ) + } +} From c2ed2bb42f4dbe7e401c9b4ee130b251fe764aad Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Mon, 25 Dec 2023 20:20:25 +0100 Subject: [PATCH 2/9] Print semicolon as part of statement in javascript Printer 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) --- .../linker/backend/javascript/Printers.scala | 66 +++++++++++++------ .../org/scalajs/linker/LibrarySizeTest.scala | 4 +- .../backend/javascript/PrintersTest.scala | 32 ++++----- project/Build.scala | 4 +- 4 files changed, 67 insertions(+), 39 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala index 2df5acc9f9..e2c57c92eb 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala @@ -73,17 +73,10 @@ object Printers { } case _ => printStat(tree) - if (shouldPrintSepAfterTree(tree)) - print(';') println() } } - protected def shouldPrintSepAfterTree(tree: Tree): Boolean = tree match { - case _:DocComment | _:FunctionDef | _:ClassDef => false - case _ => true - } - protected def printRow(ts: List[Tree], start: Char, end: Char): Unit = { print(start) var rest = ts @@ -105,11 +98,8 @@ object Printers { val x = rest.head rest = rest.tail printStat(x) - if (rest.nonEmpty) { - if (shouldPrintSepAfterTree(x)) - print(';') + if (rest.nonEmpty) println() - } } case _ => @@ -146,6 +136,11 @@ object Printers { printTree(tree, isStat = false) def printTree(tree: Tree, isStat: Boolean): Unit = { + def printSeparatorIfStat() = { + if (isStat) + print(';') + } + tree match { // Comments @@ -178,6 +173,8 @@ object Printers { print(" = ") print(rhs) } + // VarDef is an "expr" in a "For" / "ForIn" tree + printSeparatorIfStat() case Let(ident, mutable, optRhs) => print(if (mutable) "let " else "const ") @@ -186,6 +183,8 @@ object Printers { print(" = ") print(rhs) } + // Let is an "expr" in a "For" / "ForIn" tree + printSeparatorIfStat() case ParamDef(ident) => print(ident) @@ -210,10 +209,12 @@ object Printers { print(lhs) print(" = ") print(rhs) + printSeparatorIfStat() case Return(expr) => print("return ") print(expr) + print(';') case If(cond, thenp, elsep) => if (isStat) { @@ -306,19 +307,22 @@ object Printers { case Throw(expr) => print("throw ") print(expr) + print(';') case Break(label) => - if (label.isEmpty) print("break") + if (label.isEmpty) print("break;") else { print("break ") print(label.get) + print(';') } case Continue(label) => - if (label.isEmpty) print("continue") + if (label.isEmpty) print("continue;") else { print("continue ") print(label.get) + print(';') } case Switch(selector, cases, default) => @@ -354,7 +358,7 @@ object Printers { print('}') case Debugger() => - print("debugger") + print("debugger;") // Expressions @@ -375,6 +379,7 @@ object Printers { print(')') } printArgs(args) + printSeparatorIfStat() case DotSelect(qualifier, item) => qualifier match { @@ -387,27 +392,33 @@ object Printers { } print(".") print(item) + printSeparatorIfStat() case BracketSelect(qualifier, item) => print(qualifier) print('[') print(item) print(']') + printSeparatorIfStat() case Apply(fun, args) => print(fun) printArgs(args) + printSeparatorIfStat() case ImportCall(arg) => print("import(") print(arg) print(')') + printSeparatorIfStat() case NewTarget() => print("new.target") + printSeparatorIfStat() case ImportMeta() => print("import.meta") + printSeparatorIfStat() case Spread(items) => print("...") @@ -416,6 +427,7 @@ object Printers { case Delete(prop) => print("delete ") print(prop) + printSeparatorIfStat() case UnaryOp(op, lhs) => import ir.Trees.JSUnaryOp._ @@ -433,6 +445,7 @@ object Printers { } print(lhs) print(')') + printSeparatorIfStat() case IncDec(prefix, inc, arg) => val op = if (inc) "++" else "--" @@ -443,6 +456,7 @@ object Printers { if (!prefix) print(op) print(')') + printSeparatorIfStat() case BinaryOp(op, lhs, rhs) => import ir.Trees.JSBinaryOp._ @@ -482,13 +496,15 @@ object Printers { print(' ') print(rhs) print(')') + printSeparatorIfStat() case ArrayConstr(items) => printRow(items, '[', ']') + printSeparatorIfStat() case ObjectConstr(Nil) => if (isStat) - print("({})") // force expression position for the object literal + print("({});") // force expression position for the object literal else print("{}") @@ -514,18 +530,21 @@ object Printers { println() print('}') if (isStat) - print(')') + print(");") // Literals case Undefined() => print("(void 0)") + printSeparatorIfStat() case Null() => print("null") + printSeparatorIfStat() case BooleanLiteral(value) => print(if (value) "true" else "false") + printSeparatorIfStat() case IntLiteral(value) => if (value >= 0) { @@ -535,6 +554,7 @@ object Printers { print(value.toString) print(')') } + printSeparatorIfStat() case DoubleLiteral(value) => if (value == 0 && 1 / value < 0) { @@ -546,11 +566,13 @@ object Printers { print(value.toString) print(')') } + printSeparatorIfStat() case StringLiteral(value) => print('\"') printEscapeJS(value) print('\"') + printSeparatorIfStat() case BigIntLiteral(value) => if (value >= 0) { @@ -561,14 +583,17 @@ object Printers { print(value.toString) print("n)") } + printSeparatorIfStat() // Atomic expressions case VarRef(ident) => print(ident) + printSeparatorIfStat() case This() => print("this") + printSeparatorIfStat() case Function(arrow, args, restParam, body) => if (arrow) { @@ -595,6 +620,7 @@ object Printers { printBlock(body) print(')') } + printSeparatorIfStat() // Named function definition @@ -624,8 +650,7 @@ object Printers { var rest = members while (rest.nonEmpty) { println() - print(rest.head) - print(';') + printStat(rest.head) rest = rest.tail } undent(); println(); print('}') @@ -677,12 +702,14 @@ object Printers { } print(" } from ") print(from: Tree) + print(';') case ImportNamespace(binding, from) => print("import * as ") print(binding) print(" from ") print(from: Tree) + print(';') case Export(bindings) => print("export { ") @@ -699,7 +726,7 @@ object Printers { print(binding._2) rest = rest.tail } - print(" }") + print(" };") case ExportImport(bindings, from) => print("export { ") @@ -718,6 +745,7 @@ object Printers { } print(" } from ") print(from: Tree) + print(';') case _ => throw new IllegalArgumentException( diff --git a/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala index a64f546d68..6695edfb35 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala @@ -70,8 +70,8 @@ class LibrarySizeTest { ) testLinkedSizes( - expectedFastLinkSize = 150031, - expectedFullLinkSizeWithoutClosure = 130655, + expectedFastLinkSize = 150534, + expectedFullLinkSizeWithoutClosure = 131079, expectedFullLinkSizeWithClosure = 21394, classDefs, moduleInitializers = MainTestModuleInitializers diff --git a/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala index c43b1a28e9..621910f9a7 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala @@ -43,7 +43,7 @@ class PrintersTest { """ |function test() { | const x = 2; - | return x + | return x; |} """, FunctionDef("test", Nil, None, Block( @@ -75,13 +75,13 @@ class PrintersTest { |class MyClass { | foo() { | /**/ - | }; + | } | get a() { - | return 1 - | }; + | return 1; + | } | set a(x) { | /**/ - | }; + | } |} """, ClassDef(Some("MyClass"), None, List( @@ -106,7 +106,7 @@ class PrintersTest { """ |for (let x = 1; (x < 15); x = (x + 1)) { | /**/ - |}; + |} """, For(Let("x", true, Some(IntLiteral(1))), BinaryOp(ir.Trees.JSBinaryOp.<, VarRef("x"), IntLiteral(15)), @@ -120,7 +120,7 @@ class PrintersTest { """ |for (var x in foo) { | /**/ - |}; + |} """, ForIn(VarDef("x", None), VarRef("foo"), Skip()) ) @@ -130,8 +130,8 @@ class PrintersTest { assertPrintEquals( """ |if (false) { - | 1 - |}; + | 1; + |} """, If(BooleanLiteral(false), IntLiteral(1), Skip()) ) @@ -139,10 +139,10 @@ class PrintersTest { assertPrintEquals( """ |if (false) { - | 1 + | 1; |} else { - | 2 - |}; + | 2; + |} """, If(BooleanLiteral(false), IntLiteral(1), IntLiteral(2)) ) @@ -150,12 +150,12 @@ class PrintersTest { assertPrintEquals( """ |if (false) { - | 1 + | 1; |} else if (true) { - | 2 + | 2; |} else { - | 3 - |}; + | 3; + |} """, If(BooleanLiteral(false), IntLiteral(1), If(BooleanLiteral(true), IntLiteral(2), IntLiteral(3))) diff --git a/project/Build.scala b/project/Build.scala index 93209c3ddb..da13fdd781 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -1967,7 +1967,7 @@ object Build { scalaVersion.value match { case `default212Version` => Some(ExpectedSizes( - fastLink = 772000 to 773000, + fastLink = 775000 to 776000, fullLink = 145000 to 146000, fastLinkGz = 91000 to 92000, fullLinkGz = 35000 to 36000, @@ -1975,7 +1975,7 @@ object Build { case `default213Version` => Some(ExpectedSizes( - fastLink = 480000 to 481000, + fastLink = 481000 to 483000, fullLink = 102000 to 103000, fastLinkGz = 62000 to 63000, fullLinkGz = 27000 to 28000, From 197af9a726414fc68345920fadc5bd77c419f0fd Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Thu, 28 Dec 2023 09:13:59 +0100 Subject: [PATCH 3/9] Do not print skip's in blocks --- .../scalajs/linker/backend/javascript/Printers.scala | 12 +++++++----- .../scala/org/scalajs/linker/LibrarySizeTest.scala | 4 ++-- .../linker/backend/javascript/PrintersTest.scala | 5 ----- project/Build.scala | 6 +++--- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala index e2c57c92eb..adf58ee214 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala @@ -90,19 +90,21 @@ object Printers { } protected def printBlock(tree: Tree): Unit = { - print('{'); indent(); println() + print('{'); indent(); tree match { + case Skip() => + // do not print anything + case tree: Block => var rest = tree.stats while (rest.nonEmpty) { - val x = rest.head + println() + printStat(rest.head) rest = rest.tail - printStat(x) - if (rest.nonEmpty) - println() } case _ => + println() printStat(tree) } undent(); println(); print('}') diff --git a/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala index 6695edfb35..9dcc074647 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/LibrarySizeTest.scala @@ -70,8 +70,8 @@ class LibrarySizeTest { ) testLinkedSizes( - expectedFastLinkSize = 150534, - expectedFullLinkSizeWithoutClosure = 131079, + expectedFastLinkSize = 150339, + expectedFullLinkSizeWithoutClosure = 130884, expectedFullLinkSizeWithClosure = 21394, classDefs, moduleInitializers = MainTestModuleInitializers diff --git a/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala index 621910f9a7..2397717164 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala @@ -54,7 +54,6 @@ class PrintersTest { assertPrintEquals( """ |function test() { - | /**/ |} """, FunctionDef("test", Nil, None, Skip()) @@ -74,13 +73,11 @@ class PrintersTest { """ |class MyClass { | foo() { - | /**/ | } | get a() { | return 1; | } | set a(x) { - | /**/ | } |} """, @@ -105,7 +102,6 @@ class PrintersTest { assertPrintEquals( """ |for (let x = 1; (x < 15); x = (x + 1)) { - | /**/ |} """, For(Let("x", true, Some(IntLiteral(1))), @@ -119,7 +115,6 @@ class PrintersTest { assertPrintEquals( """ |for (var x in foo) { - | /**/ |} """, ForIn(VarDef("x", None), VarRef("foo"), Skip()) diff --git a/project/Build.scala b/project/Build.scala index da13fdd781..758975e8f0 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -1967,15 +1967,15 @@ object Build { scalaVersion.value match { case `default212Version` => Some(ExpectedSizes( - fastLink = 775000 to 776000, + fastLink = 770000 to 771000, fullLink = 145000 to 146000, - fastLinkGz = 91000 to 92000, + fastLinkGz = 90000 to 91000, fullLinkGz = 35000 to 36000, )) case `default213Version` => Some(ExpectedSizes( - fastLink = 481000 to 483000, + fastLink = 479000 to 480000, fullLink = 102000 to 103000, fastLinkGz = 62000 to 63000, fullLinkGz = 27000 to 28000, From 7c3797d02d7194a051bc51c98299f09650134330 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Thu, 28 Dec 2023 18:22:15 +0100 Subject: [PATCH 4/9] Do not abuse a block to group instance tests This was forgotten in 1982a6b631a5b37ebbd4fe61655a45ad9b62bf93. --- .../org/scalajs/linker/backend/emitter/ClassEmitter.scala | 4 ++-- .../scala/org/scalajs/linker/backend/emitter/Emitter.scala | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/ClassEmitter.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/ClassEmitter.scala index 0701d2fd84..ca58616785 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/ClassEmitter.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/ClassEmitter.scala @@ -683,12 +683,12 @@ private[emitter] final class ClassEmitter(sjsGen: SJSGen) { def genInstanceTests(className: ClassName, kind: ClassKind)( implicit moduleContext: ModuleContext, - globalKnowledge: GlobalKnowledge, pos: Position): WithGlobals[js.Tree] = { + globalKnowledge: GlobalKnowledge, pos: Position): WithGlobals[List[js.Tree]] = { for { single <- genSingleInstanceTests(className, kind) array <- genArrayInstanceTests(className) } yield { - js.Block(single ::: array) + single ::: array } } diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/Emitter.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/Emitter.scala index d5b23e4525..d89de7ae4d 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/Emitter.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/Emitter.scala @@ -595,7 +595,7 @@ final class Emitter(config: Emitter.Config) { */ if (classEmitter.needInstanceTests(linkedClass)(classCache)) { - main += extractWithGlobals(classTreeCache.instanceTests.getOrElseUpdate( + main ++= extractWithGlobals(classTreeCache.instanceTests.getOrElseUpdate( classEmitter.genInstanceTests(className, kind)(moduleContext, classCache, linkedClass.pos))) } @@ -1035,7 +1035,7 @@ object Emitter { private final class DesugaredClassCache { val privateJSFields = new OneTimeCache[WithGlobals[List[js.Tree]]] - val instanceTests = new OneTimeCache[WithGlobals[js.Tree]] + val instanceTests = new OneTimeCache[WithGlobals[List[js.Tree]]] val typeData = new OneTimeCache[WithGlobals[List[js.Tree]]] val setTypeData = new OneTimeCache[js.Tree] val moduleAccessor = new OneTimeCache[WithGlobals[List[js.Tree]]] From bdea4d7ddb6ac6917bae270903c803ff5fdb3166 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Fri, 29 Dec 2023 15:55:20 +0100 Subject: [PATCH 5/9] Do not abuse a block to group const field export statements This was forgotten in 1982a6b631a5b37ebbd4fe61655a45ad9b62bf93. --- .../linker/backend/emitter/ClassEmitter.scala | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/ClassEmitter.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/ClassEmitter.scala index ca58616785..144672a471 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/ClassEmitter.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/ClassEmitter.scala @@ -1028,16 +1028,16 @@ private[emitter] final class ClassEmitter(sjsGen: SJSGen) { case e: TopLevelMethodExportDef => genTopLevelMethodExportDef(e) case e: TopLevelFieldExportDef => - genTopLevelFieldExportDef(topLevelExport.owningClass, e) + genTopLevelFieldExportDef(topLevelExport.owningClass, e).map(_ :: Nil) } } - WithGlobals.list(exportsWithGlobals) + WithGlobals.flatten(exportsWithGlobals) } private def genTopLevelMethodExportDef(tree: TopLevelMethodExportDef)( implicit moduleContext: ModuleContext, - globalKnowledge: GlobalKnowledge): WithGlobals[js.Tree] = { + globalKnowledge: GlobalKnowledge): WithGlobals[List[js.Tree]] = { import TreeDSL._ val JSMethodDef(flags, StringLiteral(exportName), args, restParam, body) = @@ -1056,22 +1056,22 @@ private[emitter] final class ClassEmitter(sjsGen: SJSGen) { private def genConstValueExportDef(exportName: String, exportedValue: js.Tree)( - implicit pos: Position): WithGlobals[js.Tree] = { + implicit pos: Position): WithGlobals[List[js.Tree]] = { moduleKind match { case ModuleKind.NoModule => - genAssignToNoModuleExportVar(exportName, exportedValue) + genAssignToNoModuleExportVar(exportName, exportedValue).map(_ :: Nil) case ModuleKind.ESModule => val field = fileLevelVar(VarField.e, exportName) val let = js.Let(field.ident, mutable = true, Some(exportedValue)) val exportStat = js.Export((field.ident -> js.ExportName(exportName)) :: Nil) - WithGlobals(js.Block(let, exportStat)) + WithGlobals(List(let, exportStat)) case ModuleKind.CommonJSModule => globalRef("exports").map { exportsVarRef => js.Assign( genBracketSelect(exportsVarRef, js.StringLiteral(exportName)), - exportedValue) + exportedValue) :: Nil } } } From 135932afa2601f9f2b914c1fdea806f4d3903c25 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Thu, 28 Dec 2023 18:24:23 +0100 Subject: [PATCH 6/9] Simplify printTopLevelTree in JS printer Because it does not need to flatten blocks anymore, we can simply print it as a statement and add a trailing newline. --- .../linker/backend/javascript/Printers.scala | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala index adf58ee214..c50426a5d8 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala @@ -62,19 +62,8 @@ object Printers { } def printTopLevelTree(tree: Tree): Unit = { - tree match { - case Skip() => - // do not print anything - case tree: Block => - var rest = tree.stats - while (rest.nonEmpty) { - printTopLevelTree(rest.head) - rest = rest.tail - } - case _ => - printStat(tree) - println() - } + printStat(tree) + println() } protected def printRow(ts: List[Tree], start: Char, end: Char): Unit = { From de0d65c6df5a51a8b024363f1382723b245b1c20 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Fri, 29 Dec 2023 16:14:18 +0100 Subject: [PATCH 7/9] Restrict visibility of JS Printer methods They were unnecessarily protected. --- .../linker/backend/javascript/Printers.scala | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala index c50426a5d8..035f21ef0a 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala @@ -66,7 +66,7 @@ object Printers { println() } - protected def printRow(ts: List[Tree], start: Char, end: Char): Unit = { + private def printRow(ts: List[Tree], start: Char, end: Char): Unit = { print(start) var rest = ts while (rest.nonEmpty) { @@ -78,7 +78,7 @@ object Printers { print(end) } - protected def printBlock(tree: Tree): Unit = { + private def printBlock(tree: Tree): Unit = { print('{'); indent(); tree match { case Skip() => @@ -99,7 +99,7 @@ object Printers { undent(); println(); print('}') } - protected def printSig(args: List[ParamDef], restParam: Option[ParamDef]): Unit = { + private def printSig(args: List[ParamDef], restParam: Option[ParamDef]): Unit = { print("(") var rem = args while (rem.nonEmpty) { @@ -117,13 +117,13 @@ object Printers { print(") ") } - protected def printArgs(args: List[Tree]): Unit = + private def printArgs(args: List[Tree]): Unit = printRow(args, '(', ')') - protected def printStat(tree: Tree): Unit = + private def printStat(tree: Tree): Unit = printTree(tree, isStat = true) - protected def print(tree: Tree): Unit = + private def print(tree: Tree): Unit = printTree(tree, isStat = false) def printTree(tree: Tree, isStat: Boolean): Unit = { @@ -760,7 +760,7 @@ object Printers { print("]") } - protected def print(exportName: ExportName): Unit = + private def print(exportName: ExportName): Unit = printEscapeJS(exportName.name) /** Prints an ASCII string -- use for syntax strings, not for user strings. */ From bd58bbbb4e2e2cf3fb5fb2c7643f0dc0dd8940e0 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Fri, 29 Dec 2023 16:16:39 +0100 Subject: [PATCH 8/9] Print trailing newline as part of statement 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. --- .../linker/backend/javascript/Printers.scala | 49 +++++++++++++------ 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala index 035f21ef0a..7d0235bed3 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala @@ -44,6 +44,9 @@ object Printers { protected def println(): Unit = { out.write('\n') + } + + protected def printIndent(): Unit = { val indentArray = this.indentArray val indentMargin = this.indentMargin val bigEnoughIndentArray = @@ -63,7 +66,6 @@ object Printers { def printTopLevelTree(tree: Tree): Unit = { printStat(tree) - println() } private def printRow(ts: List[Tree], start: Char, end: Char): Unit = { @@ -79,7 +81,7 @@ object Printers { } private def printBlock(tree: Tree): Unit = { - print('{'); indent(); + print('{'); indent(); println() tree match { case Skip() => // do not print anything @@ -87,16 +89,14 @@ object Printers { case tree: Block => var rest = tree.stats while (rest.nonEmpty) { - println() printStat(rest.head) rest = rest.tail } case _ => - println() printStat(tree) } - undent(); println(); print('}') + undent(); printIndent(); print('}') } private def printSig(args: List[ParamDef], restParam: Option[ParamDef]): Unit = { @@ -120,12 +120,22 @@ object Printers { private def printArgs(args: List[Tree]): Unit = printRow(args, '(', ')') - private def printStat(tree: Tree): Unit = + /** Prints a stat including leading indent and trailing newline. */ + private def printStat(tree: Tree): Unit = { + printIndent() printTree(tree, isStat = true) + println() + } private def print(tree: Tree): Unit = printTree(tree, isStat = false) + /** Print the "meat" of a tree. + * + * Even if it is a stat: + * - No leading indent. + * - No trailing newline. + */ def printTree(tree: Tree, isStat: Boolean): Unit = { def printSeparatorIfStat() = { if (isStat) @@ -144,12 +154,12 @@ object Printers { } else { print("/** ") print(lines.head) - println() + println(); printIndent() var rest = lines.tail while (rest.nonEmpty) { print(" * ") print(rest.head) - println() + println(); printIndent() rest = rest.tail } print(" */") @@ -326,7 +336,7 @@ object Printers { while (rest.nonEmpty) { val next = rest.head rest = rest.tail - println() + println(); printIndent() print("case ") print(next._1) print(':') @@ -339,13 +349,13 @@ object Printers { default match { case Skip() => case _ => - println() + println(); printIndent() print("default: ") printBlock(default) } undent() - println() + println(); printIndent() print('}') case Debugger() => @@ -509,16 +519,17 @@ object Printers { while (rest.nonEmpty) { val x = rest.head rest = rest.tail + printIndent() print(x._1) print(": ") print(x._2) if (rest.nonEmpty) { print(',') - println() } + println() } undent() - println() + printIndent() print('}') if (isStat) print(");") @@ -637,14 +648,13 @@ object Printers { print(" extends ") print(optParentClass.get) } - print(" {"); indent() + print(" {"); indent(); println() var rest = members while (rest.nonEmpty) { - println() printStat(rest.head) rest = rest.tail } - undent(); println(); print('}') + undent(); printIndent(); print('}') case MethodDef(static, name, params, restParam, body) => if (static) @@ -801,6 +811,13 @@ object Printers { override protected def println(): Unit = { super.println() sourceMap.nextLine() + column = 0 + } + + override protected def printIndent(): Unit = { + assert(column == 0) + + super.printIndent() column = this.getIndentMargin() } From cae909ad8d661eb6af63a6946236a5b63b9181d5 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Fri, 29 Dec 2023 16:21:36 +0100 Subject: [PATCH 9/9] Remove printTopLevelTree and replace it with printStat --- .../org/scalajs/linker/backend/BasicLinkerBackend.scala | 4 ++-- .../org/scalajs/linker/backend/javascript/Printers.scala | 6 +----- .../scalajs/linker/backend/javascript/PrintersTest.scala | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/BasicLinkerBackend.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/BasicLinkerBackend.scala index a1238e7433..564ebbb99c 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/BasicLinkerBackend.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/BasicLinkerBackend.scala @@ -283,7 +283,7 @@ private object BasicLinkerBackend { val jsCodeWriter = new ByteArrayWriter() val printer = new Printers.JSTreePrinter(jsCodeWriter) - printer.printTopLevelTree(tree) + printer.printStat(tree) new PrintedTree(jsCodeWriter.toByteArray(), SourceMapWriter.Fragment.Empty) } @@ -321,7 +321,7 @@ private object BasicLinkerBackend { val smFragmentBuilder = new SourceMapWriter.FragmentBuilder() val printer = new Printers.JSTreePrinterWithSourceMap(jsCodeWriter, smFragmentBuilder) - printer.printTopLevelTree(tree) + printer.printStat(tree) smFragmentBuilder.complete() new PrintedTree(jsCodeWriter.toByteArray(), smFragmentBuilder.result()) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala index 7d0235bed3..9690946e69 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/backend/javascript/Printers.scala @@ -64,10 +64,6 @@ object Printers { newIndentArray } - def printTopLevelTree(tree: Tree): Unit = { - printStat(tree) - } - private def printRow(ts: List[Tree], start: Char, end: Char): Unit = { print(start) var rest = ts @@ -121,7 +117,7 @@ object Printers { printRow(args, '(', ')') /** Prints a stat including leading indent and trailing newline. */ - private def printStat(tree: Tree): Unit = { + final def printStat(tree: Tree): Unit = { printIndent() printTree(tree, isStat = true) println() diff --git a/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala index 2397717164..316f2c3907 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/backend/javascript/PrintersTest.scala @@ -33,7 +33,7 @@ class PrintersTest { private def assertPrintEquals(expected: String, tree: Tree): Unit = { val out = new ByteArrayWriter val printer = new Printers.JSTreePrinter(out) - printer.printTopLevelTree(tree) + printer.printStat(tree) assertEquals(expected.stripMargin.trim + "\n", new String(out.toByteArray(), StandardCharsets.UTF_8)) }