Make CompilerError.invariant assert the condition

This allows us to mimic the `invariant` api, which means you can just assert that something holds true after execution proceeds to the next line without throwing
2025-11-01 09:12:30 +00:00 · 2023-06-22 10:18:41 -04:00
parent 6c425f6b36
commit 3f2b346ba0
15 changed files with 179 additions and 193 deletions
@@ -70,20 +70,23 @@ export class CompilerError extends Error {
  details: CompilerErrorDetail[] = [];

  static invariant(
+    condition: unknown,
    reason: string,
    loc: SourceLocation,
    description: string | null = null
-  ): never {
-    const errors = new CompilerError();
-    errors.pushErrorDetail(
-      new CompilerErrorDetail({
-        description,
-        loc,
-        reason,
-        severity: ErrorSeverity.Invariant,
-      })
-    );
-    throw errors;
+  ): asserts condition {
+    if (!condition) {
+      const errors = new CompilerError();
+      errors.pushErrorDetail(
+        new CompilerErrorDetail({
+          description,
+          loc,
+          reason,
+          severity: ErrorSeverity.Invariant,
+        })
+      );
+      throw errors;
+    }
  }

  static todo(
@@ -53,12 +53,11 @@ export function compileProgram(
        return;
      }

-      if (fn.node.id == null) {
-        CompilerError.invariant(
-          "FunctionDeclaration must have a name",
-          fn.node.loc ?? GeneratedSource
-        );
-      }
+      CompilerError.invariant(
+        fn.node.id != null,
+        "FunctionDeclaration must have a name",
+        fn.node.loc ?? GeneratedSource
+      );
      const originalIdent = fn.node.id;

      if (pass.opts.gating != null) {
@@ -66,12 +65,11 @@ export function compileProgram(
        fn.node.id = addSuffix(fn.node.id, "_uncompiled");

        // Rename and append compiled function
-        if (compiled.id == null) {
-          CompilerError.invariant(
-            "FunctionDeclaration must produce a name",
-            fn.node.loc ?? GeneratedSource
-          );
-        }
+        CompilerError.invariant(
+          compiled.id != null,
+          "FunctionDeclaration must produce a name",
+          fn.node.loc ?? GeneratedSource
+        );
        compiled.id = addSuffix(compiled.id, "_forget");
        const compiledFn = fn.insertAfter(compiled)[0];
        compiledFn.skip();
@@ -409,12 +407,12 @@ function buildBlockStatement(
    return wrappedBody.node;
  }

-  if (!body.isBlockStatement()) {
-    CompilerError.invariant(
-      "Body must be a BlockStatement",
-      body.node.loc ?? GeneratedSource
-    );
-  }
+  CompilerError.invariant(
+    body.isBlockStatement(),
+    "Body must be a BlockStatement",
+    body.node.loc ?? GeneratedSource
+  );
+
  return body.node;
 }

@@ -35,20 +35,18 @@ export function assertConsistentIdentifiers(fn: HIRFunction): void {
      }
    }
    for (const instr of block.instructions) {
-      if (instr.lvalue.identifier.name !== null) {
-        CompilerError.invariant(
-          `Expected all lvalues to be temporaries`,
-          instr.lvalue.loc,
-          `Found named lvalue '${instr.lvalue.identifier.name}'`
-        );
-      }
-      if (assignments.has(instr.lvalue.identifier.id)) {
-        CompilerError.invariant(
-          `Expected lvalues to be assigned exactly once`,
-          instr.lvalue.loc,
-          `Found duplicate assignment of '${printPlace(instr.lvalue)}'`
-        );
-      }
+      CompilerError.invariant(
+        instr.lvalue.identifier.name === null,
+        `Expected all lvalues to be temporaries`,
+        instr.lvalue.loc,
+        `Found named lvalue '${instr.lvalue.identifier.name}'`
+      );
+      CompilerError.invariant(
+        !assignments.has(instr.lvalue.identifier.id),
+        `Expected lvalues to be assigned exactly once`,
+        instr.lvalue.loc,
+        `Found duplicate assignment of '${printPlace(instr.lvalue)}'`
+      );
      assignments.add(instr.lvalue.identifier.id);
      for (const operand of eachInstructionLValue(instr)) {
        validate(identifiers, operand.identifier, operand.loc);
@@ -73,8 +71,9 @@ function validate(
  const previous = identifiers.get(identifier.id);
  if (previous === undefined) {
    identifiers.set(identifier.id, identifier);
-  } else if (identifier !== previous) {
+  } else {
    CompilerError.invariant(
+      identifier === previous,
      `Duplicate identifier object`,
      loc ?? GeneratedSource,
      `Found duplicate identifier object for id ${identifier.id}`
@@ -13,15 +13,14 @@ import { mapTerminalSuccessors } from "./visitors";
 export function assertTerminalSuccessorsExist(fn: HIRFunction): void {
  for (const [, block] of fn.body.blocks) {
    mapTerminalSuccessors(block.terminal, (successor) => {
-      if (!fn.body.blocks.has(successor)) {
-        CompilerError.invariant(
-          `Terminal successor references unknown block`,
-          (block.terminal as any).loc ?? GeneratedSource,
-          `Block bb${successor} does not exist for terminal '${printTerminal(
-            block.terminal
-          )}'`
-        );
-      }
+      CompilerError.invariant(
+        fn.body.blocks.has(successor),
+        `Terminal successor references unknown block`,
+        (block.terminal as any).loc ?? GeneratedSource,
+        `Block bb${successor} does not exist for terminal '${printTerminal(
+          block.terminal
+        )}'`
+      );
      return successor;
    });
  }
@@ -137,20 +137,18 @@ function handleAssignment(
      for (const property of path.get("properties")) {
        if (property.isObjectProperty()) {
          const valuePath = property.get("value");
-          if (!valuePath.isLVal()) {
-            CompilerError.invariant(
-              `[FindContextIdentifiers] Expected object property value to be an LVal, got: ${valuePath.type}`,
-              valuePath.node.loc ?? GeneratedSource
-            );
-          }
+          CompilerError.invariant(
+            valuePath.isLVal(),
+            `[FindContextIdentifiers] Expected object property value to be an LVal, got: ${valuePath.type}`,
+            valuePath.node.loc ?? GeneratedSource
+          );
          handleAssignment(reassigned, valuePath);
        } else {
-          if (!property.isRestElement()) {
-            CompilerError.invariant(
-              `[FindContextIdentifiers] Invalid assumptions for babel types.`,
-              property.node.loc ?? GeneratedSource
-            );
-          }
+          CompilerError.invariant(
+            property.isRestElement(),
+            `[FindContextIdentifiers] Invalid assumptions for babel types.`,
+            property.node.loc ?? GeneratedSource
+          );
          handleAssignment(reassigned, property);
        }
      }
@@ -884,7 +884,7 @@ export function isMutableEffect(
    }

    case Effect.Unknown: {
-      CompilerError.invariant("Unexpected unknown effect", location);
+      CompilerError.invariant(false, "Unexpected unknown effect", location);
    }
    case Effect.Read:
    case Effect.Freeze: {
@@ -215,13 +215,12 @@ class InferenceState {
      const kind = this.#values.get(value)!;
      mergedKind = mergedKind !== null ? mergeValues(mergedKind, kind) : kind;
    }
-    if (mergedKind === null) {
-      CompilerError.invariant(
-        `InferReferenceEffects::kind: Expected at least one value`,
-        place.loc,
-        `No value found at '${printPlace(place)}'`
-      );
-    }
+    CompilerError.invariant(
+      mergedKind !== null,
+      `InferReferenceEffects::kind: Expected at least one value`,
+      place.loc,
+      `No value found at '${printPlace(place)}'`
+    );
    return mergedKind;
  }

@@ -268,12 +267,11 @@ class InferenceState {
  reference(place: Place, effectKind: Effect): void {
    const values = this.#variables.get(place.identifier.id);
    if (values === undefined) {
-      if (effectKind === Effect.Store) {
-        CompilerError.invariant(
-          "[InferReferenceEffects] Unhandled store reference effect",
-          place.loc
-        );
-      }
+      CompilerError.invariant(
+        effectKind !== Effect.Store,
+        "[InferReferenceEffects] Unhandled store reference effect",
+        place.loc
+      );
      place.effect =
        effectKind === Effect.ConditionallyMutate
          ? Effect.ConditionallyMutate
@@ -543,12 +543,11 @@ class Driver {
        let block: ReactiveBlock;
        if (this.cx.isScheduled(terminal.block)) {
          const break_ = this.visitBreak(terminal.block, null);
-          if (break_ === null) {
-            CompilerError.invariant(
-              "Expected a break target for a label whose body is already scheduled",
-              terminal.loc
-            );
-          }
+          CompilerError.invariant(
+            break_ !== null,
+            "Expected a break target for a label whose body is already scheduled",
+            terminal.loc
+          );
          block = [break_];
        } else {
          block = this.traverseBlock(this.cx.ir.blocks.get(terminal.block)!);
@@ -345,13 +345,12 @@ function codegenTerminal(
      );
    }
    case "for-of": {
-      if (terminal.init.kind !== "SequenceExpression") {
-        CompilerError.invariant(
-          `Expected a sequence expression init for ForOf`,
-          terminal.init.loc,
-          `Got '${terminal.init.kind}' expression instead`
-        );
-      }
+      CompilerError.invariant(
+        terminal.init.kind === "SequenceExpression",
+        `Expected a sequence expression init for ForOf`,
+        terminal.init.loc,
+        `Got '${terminal.init.kind}' expression instead`
+      );
      if (terminal.init.instructions.length !== 2) {
        CompilerError.todo(
          "Support non-trivial ForOf inits",
@@ -372,6 +371,7 @@ function codegenTerminal(
        }
        default:
          CompilerError.invariant(
+            false,
            `Expected a StoreLocal or Destructure to be assigned to the collection`,
            iterableItem.value.loc,
            `Found ${iterableItem.value.kind}`
@@ -387,6 +387,7 @@ function codegenTerminal(
          break;
        case InstructionKind.Reassign:
          CompilerError.invariant(
+            false,
            "Destructure should never be Reassign as it would be an Object/ArrayPattern",
            iterableItem.loc
          );
@@ -510,6 +511,7 @@ function codegenInstructionNullable(
      }
      if (hasReasign && hasDeclaration) {
        CompilerError.invariant(
+          false,
          "Encountered a destructuring operation where some identifiers are already declared (reassignments) but others are not (declarations)",
          instr.loc
        );
@@ -520,23 +522,21 @@ function codegenInstructionNullable(
    }
    switch (kind) {
      case InstructionKind.Const: {
-        if (instr.lvalue !== null) {
-          CompilerError.invariant(
-            `Const declaration cannot be referenced as an expression`,
-            instr.value.loc
-          );
-        }
+        CompilerError.invariant(
+          instr.lvalue === null,
+          `Const declaration cannot be referenced as an expression`,
+          instr.value.loc
+        );
        return createVariableDeclaration(instr.loc, "const", [
          t.variableDeclarator(codegenLValue(lvalue), value),
        ]);
      }
      case InstructionKind.Let: {
-        if (instr.lvalue !== null) {
-          CompilerError.invariant(
-            `Const declaration cannot be referenced as an expression`,
-            instr.value.loc
-          );
-        }
+        CompilerError.invariant(
+          instr.lvalue === null,
+          `Const declaration cannot be referenced as an expression`,
+          instr.value.loc
+        );
        return createVariableDeclaration(instr.loc, "let", [
          t.variableDeclarator(codegenLValue(lvalue), value),
        ]);
@@ -768,6 +768,7 @@ function codegenInstructionValue(
        }
        default: {
          CompilerError.invariant(
+            false,
            "Expected an optional value to resolve to a call expression or member expression",
            instrValue.loc,
            `Got a '${optionalValue.type}'`
@@ -1097,6 +1098,7 @@ function codegenInstructionValue(
    case "StoreLocal":
    case "StoreContext": {
      CompilerError.invariant(
+        false,
        `Unexpected ${instrValue.kind} in codegenInstructionValue`,
        instrValue.loc
      );
@@ -1276,13 +1278,12 @@ function codegenPlace(cx: Context, place: Place): t.Expression {
  if (tmp != null) {
    return tmp;
  }
-  if (place.identifier.name === null && tmp === undefined) {
-    CompilerError.invariant(
-      `[Codegen] No value found for temporary`,
-      place.loc,
-      `Value for '${printPlace(place)}' was not set in the codegen context`
-    );
-  }
+  CompilerError.invariant(
+    place.identifier.name !== null || tmp !== undefined,
+    `[Codegen] No value found for temporary`,
+    place.loc,
+    `Value for '${printPlace(place)}' was not set in the codegen context`
+  );
  const identifier = convertIdentifier(place.identifier);
  identifier.loc = place.loc as any;
  return identifier;
@@ -105,7 +105,11 @@ class Visitor extends ReactiveFunctionVisitor<State> {
            break;
          }
          case Effect.Unknown: {
-            CompilerError.invariant("Unexpected unknown effect", operand.loc);
+            CompilerError.invariant(
+              false,
+              "Unexpected unknown effect",
+              operand.loc
+            );
          }
          default: {
            assertExhaustive(
@@ -75,12 +75,11 @@ export function promoteUsedTemporaries(fn: ReactiveFunction): void {
 }

 function promoteTemporary(identifier: Identifier, state: VisitorState): void {
-  if (identifier.name !== null) {
-    CompilerError.invariant(
-      "promoteTemporary: Expected to be called only for temporary variables",
-      GeneratedSource
-    );
-  }
+  CompilerError.invariant(
+    identifier.name === null,
+    "promoteTemporary: Expected to be called only for temporary variables",
+    GeneratedSource
+  );
  if (state.tags.has(identifier.id)) {
    identifier.name = `T${state.nextId++}`;
  } else {
@@ -459,22 +459,20 @@ class PropagationVisitor extends ReactiveFunctionVisitor<Context> {
        // OptionalExpression value is a SequenceExpression where the instructions
        // represent the code prior to the `?` and the final value represents the
        // conditional code that follows.
-        if (inner.kind === "SequenceExpression") {
-          // Instructions are the unconditionally executed portion before the `?`
-          for (const instr of inner.instructions) {
-            this.visitInstruction(instr, context);
-          }
-          // The final value is the conditional portion following the `?`
-          context.enterConditional(() => {
-            this.visitReactiveValue(context, id, inner.value);
-          });
-        } else {
-          CompilerError.invariant(
-            "Expected OptionalExpression value to be a SequenceExpression",
-            value.loc,
-            `Found a '${value.kind}'`
-          );
+        CompilerError.invariant(
+          inner.kind === "SequenceExpression",
+          "Expected OptionalExpression value to be a SequenceExpression",
+          value.loc,
+          `Found a '${value.kind}'`
+        );
+        // Instructions are the unconditionally executed portion before the `?`
+        for (const instr of inner.instructions) {
+          this.visitInstruction(instr, context);
        }
+        // The final value is the conditional portion following the `?`
+        context.enterConditional(() => {
+          this.visitReactiveValue(context, id, inner.value);
+        });
        break;
      }
      case "LogicalExpression": {
@@ -601,7 +601,7 @@ function computeMemoizationInputs(
      };
    }
    case "UnsupportedNode": {
-      CompilerError.invariant(`Unexpected unsupported node`, value.loc);
+      CompilerError.invariant(false, `Unexpected unsupported node`, value.loc);
    }
    default: {
      assertExhaustive(value, `Unexpected value kind '${(value as any).kind}'`);
@@ -96,13 +96,12 @@ class SSABuilder {

  definePlace(oldPlace: Place): Place {
    const oldId = oldPlace.identifier;
-    if (this.#unknown.has(oldId)) {
-      CompilerError.invariant(
-        `EnterSSA: Expected identifier to be defined before being used`,
-        oldPlace.loc,
-        `Identifier ${printIdentifier(oldId)} is undefined`
-      );
-    }
+    CompilerError.invariant(
+      !this.#unknown.has(oldId),
+      `EnterSSA: Expected identifier to be defined before being used`,
+      oldPlace.loc,
+      `Identifier ${printIdentifier(oldId)} is undefined`
+    );

    // Do not redefine context references.
    if (this.#context.has(oldId)) {
@@ -249,12 +248,11 @@ function enterSSAImpl(
    if (blockId === rootEntry) {
      // NOTE: func.context should be empty for the root function
      if (func.env.enableOptimizeFunctionExpressions) {
-        if (func.context.length !== 0) {
-          CompilerError.invariant(
-            `Expected function context to be empty for outer function declarations`,
-            func.loc
-          );
-        }
+        CompilerError.invariant(
+          func.context.length === 0,
+          `Expected function context to be empty for outer function declarations`,
+          func.loc
+        );
      } else {
        func.context = func.context.map((p) => builder.defineContext(p));
      }
@@ -132,13 +132,12 @@ export function leaveSSA(fn: HIRFunction): void {
      if (value.kind === "DeclareLocal") {
        const name = value.lvalue.place.identifier.name;
        if (name !== null) {
-          if (declarations.has(name)) {
-            CompilerError.invariant(
-              `Unexpected duplicate declaration`,
-              value.lvalue.place.loc,
-              `Found duplicate declaration for '${name}'`
-            );
-          }
+          CompilerError.invariant(
+            !declarations.has(name),
+            `Unexpected duplicate declaration`,
+            value.lvalue.place.loc,
+            `Found duplicate declaration for '${name}'`
+          );
          declarations.set(name, {
            lvalue: value.lvalue,
            place: value.lvalue.place,
@@ -153,12 +152,11 @@ export function leaveSSA(fn: HIRFunction): void {
            originalLVal === undefined ||
            originalLVal.lvalue === value.lvalue // in case this was pre-declared for the `for` initializer
          ) {
-            if (originalLVal === undefined && block.kind !== "block") {
-              CompilerError.invariant(
-                `TODO: Handle reassignment in a value block where the original declaration was removed by dead code elimination (DCE)`,
-                value.lvalue.place.loc
-              );
-            }
+            CompilerError.invariant(
+              originalLVal !== undefined || block.kind === "block",
+              `TODO: Handle reassignment in a value block where the original declaration was removed by dead code elimination (DCE)`,
+              value.lvalue.place.loc
+            );
            declarations.set(value.lvalue.place.identifier.name, {
              lvalue: value.lvalue,
              place: value.lvalue.place,
@@ -177,15 +175,12 @@ export function leaveSSA(fn: HIRFunction): void {
        let kind: InstructionKind | null = null;
        for (const place of eachPatternOperand(value.lvalue.pattern)) {
          if (place.identifier.name == null) {
-            if (kind !== null && kind !== InstructionKind.Const) {
-              CompilerError.invariant(
-                `Expected consistent kind for destructuring`,
-                place.loc,
-                `other places were '${kind}' but '${printPlace(
-                  place
-                )}' is const`
-              );
-            }
+            CompilerError.invariant(
+              kind === null || kind === InstructionKind.Const,
+              `Expected consistent kind for destructuring`,
+              place.loc,
+              `other places were '${kind}' but '${printPlace(place)}' is const`
+            );
            kind = InstructionKind.Const;
          } else {
            const originalLVal = declarations.get(place.identifier.name);
@@ -193,36 +188,33 @@ export function leaveSSA(fn: HIRFunction): void {
              originalLVal === undefined ||
              originalLVal.lvalue === value.lvalue
            ) {
-              if (originalLVal === undefined && block.kind === "value") {
-                CompilerError.invariant(
-                  `TODO: Handle reassignment in a value block where the original declaration was removed by dead code elimination (DCE)`,
-                  place.loc
-                );
-              }
+              CompilerError.invariant(
+                originalLVal !== undefined || block.kind !== "value",
+                `TODO: Handle reassignment in a value block where the original declaration was removed by dead code elimination (DCE)`,
+                place.loc
+              );
              declarations.set(place.identifier.name, {
                lvalue: value.lvalue,
                place,
              });
-              if (kind !== null && kind !== InstructionKind.Const) {
-                CompilerError.invariant(
-                  `Expected consistent kind for destructuring`,
-                  place.loc,
-                  `Other places were '${kind}' but '${printPlace(
-                    place
-                  )}' is const`
-                );
-              }
+              CompilerError.invariant(
+                kind === null || kind === InstructionKind.Const,
+                `Expected consistent kind for destructuring`,
+                place.loc,
+                `Other places were '${kind}' but '${printPlace(
+                  place
+                )}' is const`
+              );
              kind = InstructionKind.Const;
            } else {
-              if (kind !== null && kind !== InstructionKind.Reassign) {
-                CompilerError.invariant(
-                  `Expected consistent kind for destructuring`,
-                  place.loc,
-                  `Other places were '${kind}' but '${printPlace(
-                    place
-                  )}' is reassigned`
-                );
-              }
+              CompilerError.invariant(
+                kind === null || kind === InstructionKind.Reassign,
+                `Expected consistent kind for destructuring`,
+                place.loc,
+                `Other places were '${kind}' but '${printPlace(
+                  place
+                )}' is reassigned`
+              );
              kind = InstructionKind.Reassign;
              originalLVal.lvalue.kind = InstructionKind.Let;
            }