Squash with GP commit: Address review comments.

sjrd · sjrd · commit 0b7aa2f54868 · 2024-12-22T13:21:59.000+01:00
diff --git a/linker/shared/src/main/scala/org/scalajs/linker/backend/wasmemitter/FunctionEmitter.scala b/linker/shared/src/main/scala/org/scalajs/linker/backend/wasmemitter/FunctionEmitter.scala
@@ -1377,6 +1377,8 @@ private class FunctionEmitter private (
       return NothingType
     }
 
+    // scalastyle:on return
+
     genTree(lhs, op match {
       case IdentityHashCode        => AnyNotNullType
       case WrapAsThrowable | Throw => AnyType
@@ -1507,6 +1509,7 @@ private class FunctionEmitter private (
         }
 
       case IdentityHashCode =>
+        // TODO Avoid dispatch when we know a more precise type than any
         fb += wa.Call(genFunctionID.identityHashCode)
 
       case WrapAsThrowable =>
@@ -1561,8 +1564,6 @@ private class FunctionEmitter private (
     }
 
     tree.tpe
-
-    // scalastyle:on return
   }
 
   private def genBinaryOp(tree: BinaryOp): Type = {
@@ -2658,43 +2659,6 @@ private class FunctionEmitter private (
     ClassType(boxClassName, nullable = false)
   }
 
-  private def genWrapAsThrowable(tree: UnaryOp): Type = {
-    val UnaryOp(op, expr) = tree
-    assert(op == UnaryOp.WrapAsThrowable)
-
-    val nonNullThrowableType = watpe.RefType(genTypeID.ThrowableStruct)
-    val jsExceptionType = watpe.RefType(genTypeID.JSExceptionStruct)
-
-    fb.block(nonNullThrowableType) { doneLabel =>
-      genTree(expr, AnyType)
-
-      markPosition(tree)
-
-      // if expr.isInstanceOf[Throwable], then br $done
-      fb += wa.BrOnCast(doneLabel, watpe.RefType.anyref, nonNullThrowableType)
-
-      // otherwise, wrap in a new JavaScriptException
-
-      val exprLocal = addSyntheticLocal(watpe.RefType.anyref)
-      val instanceLocal = addSyntheticLocal(jsExceptionType)
-
-      fb += wa.LocalSet(exprLocal)
-      fb += wa.Call(genFunctionID.newDefault(SpecialNames.JSExceptionClass))
-      fb += wa.LocalTee(instanceLocal)
-      fb += wa.LocalGet(exprLocal)
-      fb += wa.Call(
-        genFunctionID.forMethod(
-          MemberNamespace.Constructor,
-          SpecialNames.JSExceptionClass,
-          SpecialNames.AnyArgConstructorName
-        )
-      )
-      fb += wa.LocalGet(instanceLocal)
-    }
-
-    tree.tpe
-  }
-
   private def genLinkTimeProperty(tree: LinkTimeProperty): Type = {
     val lit = ctx.coreSpec.linkTimeProperties.transformLinkTimeProperty(tree)
     genLiteral(lit, lit.tpe)
diff --git a/linker/shared/src/main/scala/org/scalajs/linker/checker/ClassDefChecker.scala b/linker/shared/src/main/scala/org/scalajs/linker/checker/ClassDefChecker.scala
@@ -798,7 +798,7 @@ private final class ClassDefChecker(classDef: ClassDef,
 
         checkApplyGeneric(method, args)
 
-      case UnaryOp(op, lhs) =>
+      case UnaryOp(_, lhs) =>
         checkTree(lhs, env)
 
       case BinaryOp(_, lhs, rhs) =>
diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/IncOptimizer.scala
@@ -882,7 +882,8 @@ final class IncOptimizer private[optimizer] (config: CommonPhaseConfig, collOps:
         case UnaryOp(UnaryOp.CheckNotNull, expr) =>
           config.coreSpec.semantics.nullPointers == CheckedBehavior.Unchecked &&
           isTriviallySideEffectFree(expr)
-        case UnaryOp(UnaryOp.GetClass, expr) => // Before 1.17, we used GetClass as CheckNotNull
+        case UnaryOp(op, expr) => // Before 1.17, we used GetClass as CheckNotNull
+          UnaryOp.isSideEffectFreeOp(op) &&
           isTriviallySideEffectFree(expr)
 
         case New(className, _, args) =>
diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala
@@ -3472,34 +3472,35 @@ private[optimizer] abstract class OptimizerCore(
     val UnaryOp(op, arg) = tree
 
     pretransformExpr(arg) { tlhs =>
-      val folded = foldUnaryOp(op, tlhs)
+      def folded: PreTransform =
+        foldUnaryOp(op, tlhs)
 
-      folded match {
-        case PreTransUnaryOp(UnaryOp.WrapAsThrowable, texpr) =>
-          if (isSubtype(texpr.tpe.base, ThrowableClassType.toNonNullable)) {
-            cont(texpr)
+      op match {
+        case UnaryOp.WrapAsThrowable =>
+          if (isSubtype(tlhs.tpe.base, ThrowableClassType.toNonNullable)) {
+            cont(tlhs)
           } else {
-            if (texpr.tpe.isExact) {
+            if (tlhs.tpe.isExact) {
               pretransformNew(AllocationSite.Tree(tree), JavaScriptExceptionClass,
-                  MethodIdent(AnyArgConstructorName), texpr :: Nil)(cont)
+                  MethodIdent(AnyArgConstructorName), tlhs :: Nil)(cont)
             } else {
               cont(folded)
             }
           }
 
-        case PreTransUnaryOp(UnaryOp.UnwrapFromThrowable, texpr) =>
-          val baseTpe = texpr.tpe.base
+        case UnaryOp.UnwrapFromThrowable =>
+          val baseTpe = tlhs.tpe.base
 
           if (baseTpe == NothingType) {
-            cont(texpr)
+            cont(tlhs)
           } else if (baseTpe == NullType) {
-            cont(checkNotNull(texpr))
+            cont(checkNotNull(tlhs))
           } else if (isSubtype(baseTpe, JavaScriptExceptionClassType)) {
-            pretransformSelectCommon(AnyType, texpr, optQualDeclaredType = None,
+            pretransformSelectCommon(AnyType, tlhs, optQualDeclaredType = None,
                 FieldIdent(exceptionFieldName), isLhsOfAssign = false)(cont)
           } else {
-            if (texpr.tpe.isExact || !isSubtype(JavaScriptExceptionClassType.toNonNullable, baseTpe))
-              cont(checkNotNull(texpr))
+            if (tlhs.tpe.isExact || !isSubtype(JavaScriptExceptionClassType.toNonNullable, baseTpe))
+              cont(checkNotNull(tlhs))
             else
               cont(folded)
           }
diff --git a/linker/shared/src/test/scala/org/scalajs/linker/IRCheckerTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/IRCheckerTest.scala
@@ -31,6 +31,7 @@ import org.scalajs.linker.interface._
 import org.scalajs.linker.interface.unstable.IRFileImpl
 import org.scalajs.linker.standard._
 import org.scalajs.linker.frontend.Refiner
+import org.scalajs.linker.backend.emitter.PrivateLibHolder
 
 import org.scalajs.linker.testutils._
 import org.scalajs.linker.testutils.TestIRBuilder._
@@ -255,13 +256,43 @@ class IRCheckerTest {
     }
   }
 
+  @Test
+  def unaryOpNonNullableArguments(): AsyncResult = await {
+    import UnaryOp._
+
+    // List of ops that take non-nullable reference types as argument
+    val ops = List(
+      Class_name,
+      Class_isPrimitive,
+      Class_isInterface,
+      Class_isArray,
+      Class_componentType,
+      Class_superClass,
+      Array_length,
+      GetClass,
+      Clone,
+      UnwrapFromThrowable
+    )
+
+    val results = for (op <- ops) yield {
+      val classDefs = Seq(
+        mainTestClassDef(UnaryOp(op, Null()))
+      )
+
+      for (log <- testLinkIRErrors(classDefs, MainTestModuleInitializers)) yield {
+        log.assertContainsError("expected but null found")
+      }
+    }
+
+    Future.sequence(results)
+  }
+
   @Test
   def arrayOpsNullOrNothing(): AsyncResult = await {
     val classDefs = Seq(
       mainTestClassDef(
         Block(
           ArraySelect(Null(), int(1))(NothingType),
-          UnaryOp(UnaryOp.Array_length, UnaryOp(UnaryOp.CheckNotNull, Null())),
           ArraySelect(UnaryOp(UnaryOp.Throw, Null()), int(1))(NothingType),
           UnaryOp(UnaryOp.Array_length, UnaryOp(UnaryOp.Throw, Null()))
         )
@@ -453,7 +484,8 @@ object IRCheckerTest {
         val linkerFrontend = StandardLinkerFrontend(config)
         val irFiles = (
           stdLibFiles ++
-          classDefs.map(MemClassDefIRFile(_))
+          classDefs.map(MemClassDefIRFile(_)) ++
+          PrivateLibHolder.files
         )
         linkerFrontend.link(irFiles, moduleInitializers, noSymbolRequirements, logger)
       }
