[Wasm] Require location for buildUnreachable and fix all usages

compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/BodyGenerator.kt

@@ -59,7 +59,8 @@ class BodyGenerator(
         elem.acceptVoid(this)
 
         if (elem is IrExpression && elem.type.isNothing()) {
-            body.buildUnreachable()
+            // TODO Ideally, we should generate unreachable only for specific cases and preferable on declaration site. 
+            body.buildUnreachableAfterNothingType()
         }
     }
 
@@ -437,7 +438,10 @@ class BodyGenerator(
                     body.buildInstr(WasmOp.CALL_REF, WasmImmediate.TypeIdx(context.referenceFunctionType(function.symbol)))
                     body.commentGroupEnd()
                 } else {
-                    body.buildUnreachable()
+                    // We came here for a call to an interface method which interface is not implemented anywhere, 
+                    // so we don't have a slot in the itable and can't generate a correct call, 
+                    // and, anyway, the call effectively is unreachable.
+                    body.buildUnreachableForVerifier()
                 }
             }
 
@@ -552,17 +556,11 @@ class BodyGenerator(
                     val fromType = call.getTypeArgument(0)!!
 
                     if (fromType.isNothing()) {
-                        body.buildUnreachable()
+                        body.buildUnreachableAfterNothingType()
                         // TODO: Investigate why?
                         return true
                     }
 
-                    // Workaround test codegen/box/elvis/nullNullOk.kt
-                    if (fromType.makeNotNull().isNothing()) {
-                        body.buildUnreachable()
-                        return true
-                    }
-
                     val toType = call.getTypeArgument(1)!!
                     val klass: IrClass = backendContext.inlineClassesUtils.getInlinedClass(toType)!!
                     val field = getInlineClassBackingField(klass)
@@ -694,7 +692,7 @@ class BodyGenerator(
     private fun recoverToExpectedType(actualType: IrType, expectedType: IrType) {
         // TYPE -> NOTHING -> FALSE
         if (expectedType.isNothing()) {
-            body.buildUnreachable()
+            body.buildUnreachableAfterNothingType()
             return
         }
 
@@ -724,11 +722,12 @@ class BodyGenerator(
 
         // NOT_NOTHING_TYPE -> NOTHING -> FALSE
         if (expectedTypeErased.isNothing() && !actualTypeErased.isNothing()) {
-            body.buildUnreachable()
+            body.buildUnreachableAfterNothingType()
             return
         }
 
         // TYPE -> BASE -> TRUE
+        // TODO Shouldn't we keep nullability for subtype check?
         if (actualClassErased.isSubclassOf(expectedClassErased)) {
             return
         }
@@ -739,7 +738,8 @@ class BodyGenerator(
         // PRIMITIVE -> REF -> FALSE
         // REF -> PRIMITIVE -> FALSE
         if (expectedIsPrimitive != actualIsPrimitive) {
-            body.buildUnreachable()
+            // TODO Shouldn't we throw ICE instead? 
+            body.buildUnreachableForVerifier()
             return
         }
 
@@ -748,7 +748,7 @@ class BodyGenerator(
             if (expectedClassErased.isSubclassOf(actualClassErased)) {
                 generateRefNullCast(actualTypeErased, expectedTypeErased)
             } else {
-                body.buildUnreachable()
+                body.buildUnreachableForVerifier()
             }
         }
     }
@@ -758,7 +758,6 @@ class BodyGenerator(
         if (nonLocalReturnSymbol != null) {
             generateWithExpectedType(expression.value, nonLocalReturnSymbol.owner.type)
             body.buildBr(functionContext.referenceNonLocalReturnLevel(nonLocalReturnSymbol))
-            body.buildUnreachable()
         } else {
             visitFunctionReturn(expression)
         }
@@ -798,7 +797,7 @@ class BodyGenerator(
                     body.buildGetUnit()
                 }
             } else {
-                body.buildUnreachable()
+                error("'When' without else branch and non Unit type: ${expression.type.dumpKotlinLike()}")
             }
         }
 

compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/DeclarationGenerator.kt

@@ -171,7 +171,7 @@ class DeclarationGenerator(
         // Add unreachable if function returns something but not as a last instruction.
         // We can do a separate lowering which adds explicit returns everywhere instead.
         if (wasmFunctionType.resultTypes.isNotEmpty()) {
-            exprGen.buildUnreachable()
+            exprGen.buildUnreachableForVerifier()
         }
 
         context.defineFunction(declaration.symbol, function)

compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/ir2wasm/OptimisedWhenGenerator.kt

@@ -10,6 +10,7 @@ import org.jetbrains.kotlin.backend.wasm.WasmSymbols
 import org.jetbrains.kotlin.ir.expressions.*
 import org.jetbrains.kotlin.ir.types.IrType
 import org.jetbrains.kotlin.ir.types.isUnit
+import org.jetbrains.kotlin.ir.util.dumpKotlinLike
 import org.jetbrains.kotlin.ir.util.isElseBranch
 import org.jetbrains.kotlin.wasm.ir.*
 import org.jetbrains.kotlin.wasm.ir.source.location.SourceLocation
@@ -225,9 +226,10 @@ private fun BodyGenerator.createBinaryTable(
             // default else block
             if (resultType != null) {
                 if (expectedType.isUnit()) {
+                    // UnitToVoidLowering may optimize "a code" that execution didn't come here.
                     body.buildGetUnit()
                 } else {
-                    body.buildUnreachable()
+                    error("'When' without else branch and non Unit type: ${expectedType.dumpKotlinLike()}")
                 }
             }
         }

wasm/wasm.ir/src/org/jetbrains/kotlin/wasm/ir/WasmExpressionBuilder.kt

@@ -36,8 +36,8 @@ abstract class WasmExpressionBuilder {
         buildInstr(WasmOp.I32_CONST, location, WasmImmediate.SymbolI32(value))
     }
 
-    fun buildUnreachable() {
+    fun buildUnreachable(location: SourceLocation) {
-        buildInstr(WasmOp.UNREACHABLE)
+        buildInstr(WasmOp.UNREACHABLE, location)
     }
 
     @Suppress("UNUSED_PARAMETER")
@@ -191,3 +191,16 @@ abstract class WasmExpressionBuilder {
         buildInstr(WasmOp.PSEUDO_COMMENT_GROUP_END)
     }
 }
+
+fun WasmExpressionBuilder.buildUnreachableForVerifier() {
+    buildUnreachable(SourceLocation.NoLocation("This instruction should never be reached, but required for wasm verifier"))
+}
+
+fun WasmExpressionBuilder.buildUnreachableAfterNothingType() {
+    buildUnreachable(
+        SourceLocation.NoLocation(
+            "The unreachable instruction after an expression with Nothing type to make sure that " +
+                    "execution doesn't come here (or it fails fast if so). It also might be required for wasm verifier."
+        )
+    )
+}