Skip to content

IntroduceLinktimeProperty and get rid of JSLinkingInfo #5025

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.

Sign up for GitHub

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 3 commits into from
Oct 2, 2024
+554 −212
Merged

IntroduceLinktimeProperty and get rid of JSLinkingInfo #5025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f0d899
Bump the version to 1.18.0-SNAPSHOT for the upcoming changes.
tanishiking Oct 1, 2024
7bf410d
Introduce `LinkTimeProperty`.
tanishiking Sep 5, 2024
3763e42
Emit `LinkTimeProperty` via `LinkingInfo.linkTimeProperty` APIs
tanishiking Sep 6, 2024
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
42 changes: 35 additions & 7 deletions compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,15 @@ import scala.reflect.internal.Flags

import org.scalajs.ir
import org.scalajs.ir.{Trees => js, Types => jstpe, ClassKind, Hashers, OriginalName}
import org.scalajs.ir.Names.{LocalName, SimpleFieldName, FieldName, SimpleMethodName, MethodName, ClassName}
import org.scalajs.ir.Names.{
LocalName,
SimpleFieldName,
FieldName,
SimpleMethodName,
MethodName,
ClassName,
BoxedStringClass
}
import org.scalajs.ir.OriginalName.NoOriginalName
import org.scalajs.ir.Trees.OptimizerHints
import org.scalajs.ir.Version.Unversioned
Expand Down Expand Up @@ -5202,10 +5210,6 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
genStatOrExpr(args(1), isStat)
}

case LINKING_INFO =>
// runtime.linkingInfo
js.JSLinkingInfo()

case IDENTITY_HASH_CODE =>
// runtime.identityHashCode(arg)
val arg = genArgs1
Expand Down Expand Up @@ -5390,6 +5394,22 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
case UNWRAP_FROM_THROWABLE =>
// js.special.unwrapFromThrowable(arg)
js.UnwrapFromThrowable(genArgs1)

case LINKTIME_PROPERTY =>
// LinkingInfo.linkTimePropertyXXX("...")
val arg = genArgs1
val tpe: jstpe.Type = toIRType(tree.tpe) match {
case jstpe.ClassType(BoxedStringClass, _) => jstpe.StringType
case irType => irType
}
arg match {
case js.StringLiteral(name) =>
js.LinkTimeProperty(name)(tpe)
case _ =>
reporter.error(args.head.pos,
"The argument of linkTimePropertyXXX must be a String literal: \"...\"")
js.LinkTimeProperty("erroneous")(tpe)
}
}
}

Expand Down Expand Up @@ -5501,8 +5521,16 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
def genSelectGet(propName: js.Tree): js.Tree =
genSuperReference(propName)

def genSelectSet(propName: js.Tree, value: js.Tree): js.Tree =
js.Assign(genSuperReference(propName), value)
def genSelectSet(propName: js.Tree, value: js.Tree): js.Tree = {
val lhs = genSuperReference(propName)
lhs match {
case js.JSGlobalRef(js.JSGlobalRef.FileLevelThis) =>
reporter.error(pos,
"Illegal assignment to global this.")
case _ =>
}
js.Assign(lhs, value)
}

def genCall(methodName: js.Tree,
args: List[js.TreeOrJSSpread]): js.Tree = {
Expand Down
5 changes: 5 additions & 0 deletions compiler/src/main/scala/org/scalajs/nscplugin/JSDefinitions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ trait JSDefinitions {
lazy val Runtime_identityHashCode = getMemberMethod(RuntimePackageModule, newTermName("identityHashCode"))
lazy val Runtime_dynamicImport = getMemberMethod(RuntimePackageModule, newTermName("dynamicImport"))

lazy val LinkingInfoModule = getRequiredModule("scala.scalajs.LinkingInfo")
lazy val LinkingInfo_linkTimePropertyBoolean = getMemberMethod(LinkingInfoModule, newTermName("linkTimePropertyBoolean"))
lazy val LinkingInfo_linkTimePropertyInt = getMemberMethod(LinkingInfoModule, newTermName("linkTimePropertyInt"))
lazy val LinkingInfo_linkTimePropertyString = getMemberMethod(LinkingInfoModule, newTermName("linkTimePropertyString"))

lazy val DynamicImportThunkClass = getRequiredClass("scala.scalajs.runtime.DynamicImportThunk")
lazy val DynamicImportThunkClass_apply = getMemberMethod(DynamicImportThunkClass, nme.apply)

Expand Down
11 changes: 7 additions & 4 deletions compiler/src/main/scala/org/scalajs/nscplugin/JSPrimitives.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ abstract class JSPrimitives {
final val CREATE_INNER_JS_CLASS = CONSTRUCTOROF + 1 // runtime.createInnerJSClass
final val CREATE_LOCAL_JS_CLASS = CREATE_INNER_JS_CLASS + 1 // runtime.createLocalJSClass
final val WITH_CONTEXTUAL_JS_CLASS_VALUE = CREATE_LOCAL_JS_CLASS + 1 // runtime.withContextualJSClassValue
final val LINKING_INFO = WITH_CONTEXTUAL_JS_CLASS_VALUE + 1 // runtime.linkingInfo
final val IDENTITY_HASH_CODE = LINKING_INFO + 1 // runtime.identityHashCode
final val IDENTITY_HASH_CODE = WITH_CONTEXTUAL_JS_CLASS_VALUE + 1 // runtime.identityHashCode
final val DYNAMIC_IMPORT = IDENTITY_HASH_CODE + 1 // runtime.dynamicImport

final val STRICT_EQ = DYNAMIC_IMPORT + 1 // js.special.strictEquals
Expand All @@ -69,8 +68,9 @@ abstract class JSPrimitives {
final val WRAP_AS_THROWABLE = JS_TRY_CATCH + 1 // js.special.wrapAsThrowable
final val UNWRAP_FROM_THROWABLE = WRAP_AS_THROWABLE + 1 // js.special.unwrapFromThrowable
final val DEBUGGER = UNWRAP_FROM_THROWABLE + 1 // js.special.debugger
final val LINKTIME_PROPERTY = DEBUGGER + 1 // LinkingInfo.linkTimePropertyXXX

final val LastJSPrimitiveCode = DEBUGGER
final val LastJSPrimitiveCode = LINKTIME_PROPERTY

/** Initialize the map of primitive methods (for GenJSCode) */
def init(): Unit = initWithPrimitives(addPrimitive)
Expand Down Expand Up @@ -109,7 +109,6 @@ abstract class JSPrimitives {
addPrimitive(Runtime_createLocalJSClass, CREATE_LOCAL_JS_CLASS)
addPrimitive(Runtime_withContextualJSClassValue,
WITH_CONTEXTUAL_JS_CLASS_VALUE)
addPrimitive(Runtime_linkingInfo, LINKING_INFO)
addPrimitive(Runtime_identityHashCode, IDENTITY_HASH_CODE)
addPrimitive(Runtime_dynamicImport, DYNAMIC_IMPORT)

Expand All @@ -123,6 +122,10 @@ abstract class JSPrimitives {
addPrimitive(Special_wrapAsThrowable, WRAP_AS_THROWABLE)
addPrimitive(Special_unwrapFromThrowable, UNWRAP_FROM_THROWABLE)
addPrimitive(Special_debugger, DEBUGGER)

addPrimitive(LinkingInfo_linkTimePropertyBoolean, LINKTIME_PROPERTY)
addPrimitive(LinkingInfo_linkTimePropertyInt, LINKTIME_PROPERTY)
addPrimitive(LinkingInfo_linkTimePropertyString, LINKTIME_PROPERTY)
}

def isJavaScriptPrimitive(code: Int): Boolean =
Expand Down
32 changes: 31 additions & 1 deletion compiler/src/test/scala/org/scalajs/nscplugin/test/JSGlobalScopeTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ class JSGlobalScopeTest extends DirectTest with TestHelpers {
"extends", "false", "finally", "for", "function", "if", "implements",
"import", "in", "instanceof", "interface", "let", "new", "null",
"package", "private", "protected", "public", "return", "static",
"super", "switch", "this", "throw", "true", "try", "typeof", "var",
"super", "switch", "throw", "true", "try", "typeof", "var",
Copy link
Member

Choose a reason for hiding this comment

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

This change should be integrated in the first commit.

Copy link
Member

Choose a reason for hiding this comment

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

Or rather: the first commit should not touch this. It's the second commit that allows this in global selections in the compiler; not the first one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, the first commit shouldn't touch the test 😵‍💫 thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no this change (remove "this" from the rejectAllReservedIdentifiers) have to be in the first commit.

Indeed we allow this in global selections in the compiler on second commit, but we change the requirements of the IR in the first commit (isValidJSGlobalRefName which is referred from the requirements of JSGlobalRef)

Copy link
Member

Choose a reason for hiding this comment

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

Ah but then I think we should add an explicit rejection of "this" in the compiler in the first commit (and then remove it in the second commit).

This way we know that the first commit does not change anything user-visible. Only the second commit introduces user-level changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

force pushed the commits to mix this change into the first commit 🙇
d5c2f7f

Copy link
Contributor Author

@tanishiking tanishiking Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Oops, I commented at a same timing

Ah but then I think we should add an explicit rejection of "this" in the compiler in the first commit (and then remove it in the second commit).
This way we know that the first commit does not change anything user-visible. Only the second commit introduces user-level changes.

👍

Copy link
Contributor Author

@tanishiking tanishiking Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

So we're going to have the following changes in the first commit

diff --git a/compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala b/compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala
index ef5d20f12..c3dff7493 100644
--- a/compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala
+++ b/compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala
@@ -6758,7 +6758,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
         implicit pos: Position): js.JSGlobalRef = {
       propName match {
         case js.StringLiteral(value) =>
-          if (js.JSGlobalRef.isValidJSGlobalRefName(value)) {
+          if (js.JSGlobalRef.isValidJSGlobalRefName(value) && value != js.JSGlobalRef.FileLevelThis) {
             if (value == "await") {
               global.runReporting.warning(pos,
                   s"$actionFull of the global scope with the name '$value' is deprecated.\n" +

and remove the change + "this" from JSGlobalScopeTest in the second commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, sorry for making a mess

"void", "while", "with", "yield")

for (reservedIdentifier <- reservedIdentifiers) {
Expand Down Expand Up @@ -497,4 +497,34 @@ class JSGlobalScopeTest extends DirectTest with TestHelpers {
}
}

@Test
def rejectAssignmentToGlobalThis(): Unit = {
"""
import scala.scalajs.js
import scala.scalajs.js.annotation._

object Main {
def main(): Unit = {
js.Dynamic.global.`this` = 0
GlobalScope.globalThis = 0
}
}

@js.native
@JSGlobalScope
object GlobalScope extends js.Any {
@JSName("this")
var globalThis: Any = js.native
}
""" hasErrors
s"""
|newSource1.scala:44: error: Illegal assignment to global this.
| js.Dynamic.global.`this` = 0
| ^
|newSource1.scala:45: error: Illegal assignment to global this.
| GlobalScope.globalThis = 0
| ^
"""
}

}
6 changes: 3 additions & 3 deletions compiler/src/test/scala/org/scalajs/nscplugin/test/OptimizationTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,14 @@ class OptimizationTest extends JSASTTest {
// Verify the optimized emitted code for 'new js.Object' and 'new js.Array'
"""
import scala.scalajs.js

class A {
val o = new js.Object
val a = new js.Array
}
""".
hasNot("any reference to the global scope") {
case js.JSLinkingInfo() =>
hasNot("any reference to the global scope nor loading JS constructor") {
case js.JSGlobalRef(_) =>
case js.LoadJSConstructor(_) =>
}
}

Expand Down
8 changes: 5 additions & 3 deletions ir/shared/src/main/scala/org/scalajs/ir/Hashers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,9 +478,6 @@ object Hashers {
mixTag(TagJSTypeOfGlobalRef)
mixTree(globalRef)

case JSLinkingInfo() =>
mixTag(TagJSLinkingInfo)

case Undefined() =>
mixTag(TagUndefined)

Expand Down Expand Up @@ -550,6 +547,11 @@ object Hashers {
mixName(className)
mixTrees(captureValues)

case LinkTimeProperty(name) =>
mixTag(TagLinkTimeProperty)
mixString(name)
mixType(tree.tpe)

case Transient(value) =>
throw new InvalidIRException(tree,
"Cannot hash a transient IR node (its value is of class " +
Expand Down
8 changes: 5 additions & 3 deletions ir/shared/src/main/scala/org/scalajs/ir/Printers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,9 +778,6 @@ object Printers {
print(globalRef)
print(")")

case JSLinkingInfo() =>
print("<linkinginfo>")

// Literals

case Undefined() =>
Expand Down Expand Up @@ -901,6 +898,11 @@ object Printers {
print(className)
printRow(captureValues, "](", ", ", ")")

case LinkTimeProperty(name) =>
print("<linkTimeProperty>(")
print(name)
print(")")

// Transient

case Transient(value) =>
Expand Down
4 changes: 2 additions & 2 deletions ir/shared/src/main/scala/org/scalajs/ir/ScalaJSVersions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import java.util.concurrent.ConcurrentHashMap
import scala.util.matching.Regex

object ScalaJSVersions extends VersionChecks(
current = "1.17.1-SNAPSHOT",
binaryEmitted = "1.17"
current = "1.18.0-SNAPSHOT",
binaryEmitted = "1.18-SNAPSHOT"
)

/** Helper class to allow for testing of logic. */
Expand Down
59 changes: 52 additions & 7 deletions ir/shared/src/main/scala/org/scalajs/ir/Serializers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import Names._
import OriginalName.NoOriginalName
import Position._
import Trees._
import LinkTimeProperty.{ProductionMode, ESVersion, UseECMAScript2015Semantics, IsWebAssembly, LinkerVersion}
import Types._
import Tags._
import Version.Unversioned
Expand Down Expand Up @@ -500,9 +501,6 @@ object Serializers {
writeTagAndPos(TagJSTypeOfGlobalRef)
writeTree(globalRef)

case JSLinkingInfo() =>
writeTagAndPos(TagJSLinkingInfo)

case Undefined() =>
writeTagAndPos(TagUndefined)

Expand Down Expand Up @@ -572,6 +570,11 @@ object Serializers {
writeName(className)
writeTrees(captureValues)

case LinkTimeProperty(name) =>
writeTagAndPos(TagLinkTimeProperty)
writeString(name)
writeType(tree.tpe)

case Transient(value) =>
throw new InvalidIRException(tree,
"Cannot serialize a transient IR node (its value is of class " +
Expand Down Expand Up @@ -1290,9 +1293,32 @@ object Serializers {
case TagUnwrapFromThrowable =>
UnwrapFromThrowable(readTree())

case TagJSNew => JSNew(readTree(), readTreeOrJSSpreads())
case TagJSPrivateSelect => JSPrivateSelect(readTree(), readFieldIdent())
case TagJSSelect => JSSelect(readTree(), readTree())
case TagJSNew => JSNew(readTree(), readTreeOrJSSpreads())
case TagJSPrivateSelect => JSPrivateSelect(readTree(), readFieldIdent())

case TagJSSelect =>
if (hacks.use17 && buf.get(buf.position()) == TagJSLinkingInfo) {
val jsLinkingInfo = readTree()
readTree() match {
case StringLiteral("productionMode") =>
LinkTimeProperty(ProductionMode)(BooleanType)
case StringLiteral("esVersion") =>
LinkTimeProperty(ESVersion)(IntType)
case StringLiteral("assumingES6") =>
LinkTimeProperty(UseECMAScript2015Semantics)(BooleanType)
case StringLiteral("isWebAssembly") =>
LinkTimeProperty(IsWebAssembly)(BooleanType)
case StringLiteral("linkerVersion") =>
LinkTimeProperty(LinkerVersion)(StringType)
case StringLiteral("fileLevelThis") =>
JSGlobalRef(JSGlobalRef.FileLevelThis)
case otherItem =>
JSSelect(jsLinkingInfo, otherItem)
}
} else {
JSSelect(readTree(), readTree())
}

case TagJSFunctionApply => JSFunctionApply(readTree(), readTreeOrJSSpreads())
case TagJSMethodApply => JSMethodApply(readTree(), readTree(), readTreeOrJSSpreads())
case TagJSSuperSelect => JSSuperSelect(readTree(), readTree(), readTree())
Expand All @@ -1312,7 +1338,21 @@ object Serializers {
JSObjectConstr(List.fill(readInt())((readTree(), readTree())))
case TagJSGlobalRef => JSGlobalRef(readString())
case TagJSTypeOfGlobalRef => JSTypeOfGlobalRef(readTree().asInstanceOf[JSGlobalRef])
case TagJSLinkingInfo => JSLinkingInfo()

case TagJSLinkingInfo =>
if (hacks.use17) {
JSObjectConstr(List(
(StringLiteral("productionMode"), LinkTimeProperty(ProductionMode)(BooleanType)),
(StringLiteral("esVersion"), LinkTimeProperty(ESVersion)(IntType)),
(StringLiteral("assumingES6"), LinkTimeProperty(UseECMAScript2015Semantics)(BooleanType)),
(StringLiteral("isWebAssembly"), LinkTimeProperty(IsWebAssembly)(BooleanType)),
(StringLiteral("linkerVersion"), LinkTimeProperty(LinkerVersion)(StringType)),
(StringLiteral("fileLevelThis"), JSGlobalRef(JSGlobalRef.FileLevelThis))
))
} else {
throw new IOException(
s"Found invalid pre-1.18 JSLinkingInfo def at ${pos}")
}

case TagUndefined => Undefined()
case TagNull => Null()
Expand Down Expand Up @@ -1355,6 +1395,9 @@ object Serializers {

case TagCreateJSClass =>
CreateJSClass(readClassName(), readTrees())

case TagLinkTimeProperty =>
LinkTimeProperty(readString())(readType())
}
}

Expand Down Expand Up @@ -2429,6 +2472,8 @@ object Serializers {
assert(sourceVersion != "1.15", "source version 1.15 does not exist")

val use16: Boolean = use13 || sourceVersion == "1.16"

val use17: Boolean = use16 || sourceVersion == "1.17"
}

/** Names needed for hacks. */
Expand Down
3 changes: 3 additions & 0 deletions ir/shared/src/main/scala/org/scalajs/ir/Tags.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ private[ir] object Tags {
final val TagWrapAsThrowable = TagJSNewTarget + 1
final val TagUnwrapFromThrowable = TagWrapAsThrowable + 1

// New in 1.18
final val TagLinkTimeProperty = TagUnwrapFromThrowable + 1

// Tags for member defs

final val TagFieldDef = 1
Expand Down
4 changes: 2 additions & 2 deletions ir/shared/src/main/scala/org/scalajs/ir/Transformers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ object Transformers {

case _:Skip | _:Debugger | _:LoadModule | _:StoreModule |
_:SelectStatic | _:SelectJSNativeMember | _:LoadJSConstructor |
_:LoadJSModule | _:JSNewTarget | _:JSImportMeta | _:JSLinkingInfo |
_:Literal | _:VarRef | _:This | _:JSGlobalRef =>
_:LoadJSModule | _:JSNewTarget | _:JSImportMeta |
_:Literal | _:VarRef | _:This | _:JSGlobalRef | _:LinkTimeProperty =>
tree
}
}
Expand Down
4 changes: 2 additions & 2 deletions ir/shared/src/main/scala/org/scalajs/ir/Traversers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,8 @@ object Traversers {

case _:Skip | _:Debugger | _:LoadModule | _:StoreModule |
_:SelectStatic | _:SelectJSNativeMember | _:LoadJSConstructor |
_:LoadJSModule | _:JSNewTarget | _:JSImportMeta | _:JSLinkingInfo |
_:Literal | _:VarRef | _:This | _:JSGlobalRef =>
_:LoadJSModule | _:JSNewTarget | _:JSImportMeta |
_:Literal | _:VarRef | _:This | _:JSGlobalRef | _:LinkTimeProperty =>
}

def traverseClassDef(tree: ClassDef): Unit = {
Expand Down
Loading