Merge pull request scala-js#4625 from sjrd/fix-js-ops-side-effects

Fix scala-js#4621: Preserve the side effects of JS binary and unary ops.

Author: sjrd

linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/FunctionEmitter.scala

@@ -1256,8 +1256,6 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
         case If(cond, thenp, elsep)  => test(cond) && test(thenp) && test(elsep)
         case BinaryOp(_, lhs, rhs)   => test(lhs) && test(rhs)
         case UnaryOp(_, lhs)         => test(lhs)
-        case JSBinaryOp(_, lhs, rhs) => test(lhs) && test(rhs)
-        case JSUnaryOp(_, lhs)       => test(lhs)
         case ArrayLength(array)      => test(array)
         case RecordSelect(record, _) => test(record)
         case IsInstanceOf(expr, _)   => test(expr)
@@ -1346,6 +1344,10 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
           allowSideEffects
         case LoadJSModule(_) =>
           allowSideEffects
+        case JSBinaryOp(_, lhs, rhs) =>
+          allowSideEffects && test(lhs) && test(rhs)
+        case JSUnaryOp(_, lhs) =>
+          allowSideEffects && test(lhs)
         case JSGlobalRef(_) =>
           allowSideEffects
         case JSTypeOfGlobalRef(_) =>

test-suite/js/src/test/scala/org/scalajs/testsuite/compiler/RegressionJSTest.scala

@@ -17,6 +17,10 @@ import scala.scalajs.js.annotation._
 
 import org.junit.Test
 import org.junit.Assert._
+import org.junit.Assume._
+
+import org.scalajs.testsuite.utils.AssertThrows.assertThrows
+import org.scalajs.testsuite.utils.Platform._
 
 class RegressionJSTest {
   import RegressionJSTest._
@@ -96,6 +100,43 @@ class RegressionJSTest {
     assertEquals(6, obj2.bar)
   }
 
+  @Test def preserveSideEffectsOfJSOpsWithBigInts_Issue4621(): Unit = {
+    assumeTrue("requires BigInts support", jsBigInts)
+
+    @noinline def bi(x: Int): js.BigInt = js.BigInt(x)
+
+    // These must be stored as `val`s first in order to trigger the original problem
+    val bi5: Any = bi(5)
+    val bi0: Any = bi(0)
+    val bi1: Any = bi(1)
+
+    assertThrows(classOf[js.JavaScriptException], {
+      bi5.asInstanceOf[js.Dynamic] / bi0.asInstanceOf[js.Dynamic]
+      fail("unreachable") // required for the above line to be in statement position
+    })
+    assertThrows(classOf[js.JavaScriptException], {
+      +bi5.asInstanceOf[js.Dynamic]
+      fail("unreachable")
+    })
+  }
+
+  @Test def preserveSideEffectsOfJSOpsWithCustomValueOf_Issue4621(): Unit = {
+    // This must be a `val` in order to trigger the original problem
+    val obj: Any = new js.Object {
+      override def valueOf(): Double =
+        throw new UnsupportedOperationException()
+    }
+
+    assertThrows(classOf[UnsupportedOperationException], {
+      obj.asInstanceOf[js.Dynamic] + 5.asInstanceOf[js.Dynamic]
+      fail("unreachable")
+    })
+    assertThrows(classOf[UnsupportedOperationException], {
+      -obj.asInstanceOf[js.Dynamic]
+      fail("unreachable")
+    })
+  }
+
 }
 
 object RegressionJSTest {