@wip bugfixes

both in the BC->IR compiler and in parsing/printing

fixed node IDs parsed and printed outside of the CFG they were defined in.

src/main/java/org/prlprg/bc/BcInstr.java

@@ -934,7 +934,7 @@ public BcOp op() {
     }
   }
 
-  record BaseGuard(ConstPool.Idx<LangSXP> expr, @LabelName("baseGuardFail") BcLabel ifFail)
+  record BaseGuard(ConstPool.Idx<LangSXP> expr, @LabelName("baseGuardAfter") BcLabel ifFail)
       implements BcInstr {
     @Override
     public BcOp op() {

src/main/java/org/prlprg/bc2ir/CFGCompiler.java

@@ -320,27 +320,38 @@ private CFGCompiler(Bc bc, CFG cfg, boolean isPromise, Module module) {
 
   /** Actually compile {@code bc} into {@code cfg}. */
   void doCompile() {
-    var code = bc.code();
-
-    // Put BBs at jump destinations.
-    // We will add more BBs when compiling some instructions, but IR in those BBs will be specific
-    // to the instruction, whereas labels in instructions may jump to within the IR of previously-
-    // compiled instructions. To insert a BB later that jumps to previously-compiled IR: it's
-    // possible, but adds complexity, because we have to remember which bytecode positions
-    // correspond to which IR positions at these positions, and move already-compiled IR into the
-    // new BB. It's easier to insert these BBs first, then we don't have to move IR and we only need
-    // to track the current bytecode and IR position.
-    for (var i = 0; i < code.size(); i++) {
-      addBcLabelBBs(code.get(i));
-    }
+    try {
+      var code = bc.code();
+
+      // Put BBs at jump destinations.
+      // We will add more BBs when compiling some instructions, but IR in those BBs will be specific
+      // to the instruction, whereas labels in instructions may jump to within the IR of previously-
+      // compiled instructions. To insert a BB later that jumps to previously-compiled IR: it's
+      // possible, but adds complexity, because we have to remember which bytecode positions
+      // correspond to which IR positions at these positions, and move already-compiled IR into the
+      // new BB. It's easier to insert these BBs first, then we don't have to move IR and we only
+      // need
+      // to track the current bytecode and IR position.
+      for (var i = 0; i < code.size(); i++) {
+        addBcLabelBBs(code.get(i));
+      }
 
-    // Add instructions to BBs, including jumps which connect them.
-    // Also, move to the BB at each label.
-    for (bcPos = 0; bcPos < code.size(); bcPos++) {
-      if (bcPos != 0 && bbByLabel.containsKey(bcPos)) {
-        moveTo(bbByLabel.get(bcPos));
+      // Add instructions to BBs, including jumps which connect them.
+      // Also, move to the BB at each label.
+      for (bcPos = 0; bcPos < code.size(); bcPos++) {
+        if (bcPos != 0 && bbByLabel.containsKey(bcPos)) {
+          var nextBb = bbByLabel.get(bcPos);
+          if (cursor.bb().jump() == null) {
+            cursor.insert(new JumpData.Goto(nextBb));
+          }
+          moveTo(nextBb);
+        }
+        addBcInstrIrInstrs(code.get(bcPos));
       }
-      addBcInstrIrInstrs(code.get(bcPos));
+    } catch (Throwable e) {
+      throw e instanceof CFGCompilerException e1
+          ? e1
+          : new CFGCompilerException("uncaught exception: " + e, bc, bcPos, cursor, e);
     }
   }
 
@@ -565,7 +576,7 @@ case GetVar(var name) -> {
         pushInsert(new StmtData.Force(pop(RValue.class), compileFrameState(), env));
       }
       case DdVal(var _) -> throw failUnsupported("`..n` variables");
-      case SetVar(var name) -> insert(new StmtData.StVar(get(name), pop(RValue.class), env));
+      case SetVar(var name) -> insert(new StmtData.StVar(get(name), top(RValue.class), env));
       case GetFun(var name) -> pushCallFunInsert(new StmtData.LdFun(get(name), env));
       case GetGlobFun(var name) ->
           pushCallFunInsert(new StmtData.LdFun(get(name), StaticEnv.GLOBAL));
@@ -667,7 +678,7 @@ case Or(var ast) ->
               new StmtData.LOr(Optional.of(get(ast)), pop(RValue.class), pop(RValue.class), env));
       case Not(var ast) ->
           pushInsert(new StmtData.Not(Optional.of(get(ast)), pop(RValue.class), env));
-      case DotsErr() -> pushInsert(new StmtData.Error("'...' used in an incorrect context", env));
+      case DotsErr() -> insert(new StmtData.Error("'...' used in an incorrect context", env));
       case StartAssign(var _), EndAssign(var _) -> throw failUnsupported("complex assignment");
       case StartSubset(var ast, var after) ->
           compileStartDispatch(Dispatch.Type.SUBSET, ast, after);
@@ -890,7 +901,7 @@ case GetVarMissOk(var name) -> {
       }
       case DdValMissOk(var _) -> throw failUnsupported("`..n` variables");
       case Visible() -> insert(new StmtData.Visible());
-      case SetVar2(var name) -> insert(new StmtData.StVarSuper(get(name), pop(RValue.class), env));
+      case SetVar2(var name) -> insert(new StmtData.StVarSuper(get(name), top(RValue.class), env));
       case StartAssign2(var _), EndAssign2(var _), SetterCall(var _, var _), GetterCall(var _) ->
           throw failUnsupported("complex assignment");
       case SpecialSwap() -> {
@@ -1037,8 +1048,8 @@ case BaseGuard(var expr, var after) -> {
         var base = cursor.insert(new StmtData.LdFun(fun, StaticEnv.BASE));
         var guard = cursor.insert(new StmtData.Eq(Optional.of(get(expr)), sym, base, env));
 
-        var safeBb = cfg.addBB();
-        var fallbackBb = cfg.addBB();
+        var safeBb = cfg.addBB("baseGuardSafe");
+        var fallbackBb = cfg.addBB("baseGuardFail");
         var afterBb = bbAt(after);
         cursor.insert(new JumpData.Branch(guard, safeBb, fallbackBb));
 
@@ -1161,9 +1172,8 @@ private void compileCall(LangSXP ast) {
                       ? new StmtData.NamedCall(ast, fun, names, args, env, compileFrameState())
                       : new StmtData.Call(ast, fun, args, env, compileFrameState());
               case Call.Fun.Builtin(var fun) ->
-                  isNamed
-                      ? new StmtData.Error("Name in builtin call", env)
-                      : new StmtData.CallBuiltin(ast, fun, args, env);
+                  // Even if `isNamed` is true, R just ignores the names.
+                  new StmtData.CallBuiltin(ast, fun, args, env);
             });
     push(callInstr);
   }
@@ -1190,14 +1200,15 @@ private FrameState compileFrameState() {
    * {@link #bbAt(BcLabel)} leaves the stack unchanged since it's still in the oly block.
    */
   private void moveTo(BB bb) {
-    cursor.moveToStart(bb);
-
     // Make sure this block has phis representing the arguments currently on the stack, then replace
     // the stack with those phis (this is how to convert a stack-based bytecode to an SSA-form IR).
     addPhiInputsForStack(bb);
     // Then replace the stack with those phis.
     stack.clear();
     stack.addAll(bb.phis());
+
+    // Finally, actually move the cursor to the BB.
+    cursor.moveToStart(bb);
   }
 
   /**
@@ -1221,6 +1232,11 @@ private void addPhiInputsForStack(BB bb) {
   private void addPhiInputsForStack(BB bb, BB incoming) {
     // Ensure the BB has phis for each stack value.
     if (bbsWithPhis.add(bb)) {
+      // Add phis for all of the nodes on the stack.
+      for (var node : stack) {
+        bb.addPhi(node.getClass());
+      }
+    } else {
       // Already added phis,
       // but sanity-check that they're still same type and amount as the nodes on the stack.
       var phis = bb.phis().iterator();
@@ -1235,7 +1251,7 @@ private void addPhiInputsForStack(BB bb, BB incoming) {
                   + i);
         }
         var phi = phis.next();
-        if (phi.getClass() != stack.get(i).getClass()) {
+        if (!phi.nodeClass().isInstance(stack.get(i))) {
           throw fail(
               "BB stack mismatch: "
                   + bb.id()
@@ -1255,17 +1271,12 @@ private void addPhiInputsForStack(BB bb, BB incoming) {
         }
         throw fail(
             "BB stack mismatch: "
-                + bb
+                + bb.id()
                 + " has too many phis; expected "
                 + stack.size()
                 + " but got "
                 + i);
       }
-    } else {
-      // Add phis for all of the nodes on the stack.
-      for (var node : stack) {
-        bb.addPhi(node.getClass());
-      }
     }
 
     // Add an input to each phi, for the corresponding stack value and the current BB.

src/main/java/org/prlprg/bc2ir/CFGCompilerException.java

@@ -1,5 +1,6 @@
 package org.prlprg.bc2ir;
 
+import javax.annotation.Nullable;
 import org.prlprg.bc.Bc;
 import org.prlprg.ir.cfg.BB;
 import org.prlprg.ir.cfg.CFG;
@@ -18,7 +19,12 @@ public class CFGCompilerException extends RuntimeException {
   private final CFGCursor irPos;
 
   CFGCompilerException(String message, Bc bc, int bcPos, CFGCursor irPos) {
-    super(message);
+    this(message, bc, bcPos, irPos, null);
+  }
+
+  CFGCompilerException(
+      String message, Bc bc, int bcPos, CFGCursor irPos, @Nullable Throwable cause) {
+    super(message, cause);
     this.bc = bc;
     this.bcPos = bcPos;
     this.irPos = irPos;
@@ -50,6 +56,6 @@ public String getMessage() {
         + " in:\n\n"
         + irPos.cfg()
         + "\n\n"
-        + bc;
+        + bc.code();
   }
 }

src/main/java/org/prlprg/ir/cfg/CFGCleanup.java

@@ -74,7 +74,7 @@ default void cleanup() {
           }
 
           // Remove basic blocks that are unreachable from the entry block [2].
-          removeAll(iter.remainingBBIds);
+          removeAll(iter.remainingBBIds());
         });
   }
 }

src/main/java/org/prlprg/ir/cfg/CFGEdit.java

@@ -235,6 +235,7 @@ public RemovePhis apply(CFG cfg) {
     public Iterable<? extends InsertPhi<?>> subEdits() {
       return phis.stream()
           .map(
+              // Sometimes this line fails to compile. Clean/recompile and it should magically pass.
               a -> new InsertPhi<>(bbId, a.id(), a.nodeClass(), ImmutableList.copyOf(a.inputIds())))
           .toList();
     }