Improve model-building error messages

src/jdk.incubator.code/share/classes/jdk/incubator/code/Block.java

@@ -26,7 +26,13 @@
 package jdk.incubator.code;
 
 import java.util.*;
+import java.util.function.Function;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import jdk.incubator.code.dialect.java.JavaType;
+import jdk.incubator.code.dialect.java.impl.JavaTypeUtils;
+import jdk.incubator.code.extern.OpWriter;
+import jdk.incubator.code.internal.OpWriterOption;
 
 /**
  * A block containing an ordered sequence of operations, where the last operation is a
@@ -659,11 +665,13 @@ public Reference reference(List<? extends Value> args) {
             check();
 
             if (isEntryBlock()) {
-                throw new IllegalStateException("Entry block cannot be referenced and targeted as a successor");
+                throw new IllegalStateException("Entry block cannot be referenced and targeted as a successor\n"
+                        + diagnosticText("entry block"));
             }
             for (Value operand : args) {
                 if (operand.isBuilt()) {
-                    throw new IllegalArgumentException("Argument's declaring block is built: " + operand);
+                    throw new IllegalArgumentException("Argument's declaring block is built\n"
+                            + operand.block.diagnosticText(operand, "argument"));
                 }
             }
 
@@ -854,32 +862,44 @@ private Op.Result insertOp(Op op) {
     private void bindOp(Op.Result opr, Op op) {
         // Structural checks
         if (!ops.isEmpty() && ops.getLast() instanceof Op.Terminating) {
-            throw new IllegalStateException("Operation cannot be appended, the block has a terminating operation");
+            throw new IllegalStateException(
+                    "Operation cannot be appended, the block has a terminating operation\n"
+                    + diagnosticText(ops.getLast(), "terminating operation"));
         }
 
         for (Body b : op.bodies()) {
             if (b.connectedAncestorBody != null && b.connectedAncestorBody != this.parentBody) {
-                throw new IllegalStateException("Body of operation is connected to a different ancestor body: ");
+                throw new IllegalStateException(
+                        "Operation body's connected ancestor body differs from this block's parent body\n"
+                        + b.entryBlock().diagnosticText("operation body")
+                        + diagnosticText("block receiving operation"));
             }
         }
 
         for (Value v : op.operands()) {
             if (!isReachable(v)) {
                 throw new IllegalStateException(
-                        String.format("Operand of operation %s is not defined in tree: %s", op, v));
+                        "Cannot append an operation because its operand is not reachable\n"
+                        + v.block.diagnosticText(v, "unreachable operand")
+                        + diagnosticText("operation appended here"));
             }
             assert !v.isBuilt();
         }
 
         for (Reference s : op.successors()) {
             if (s.target.parentBody != this.parentBody) {
-                throw new IllegalStateException("Target of block reference is not a sibling of this block");
+                throw new IllegalStateException(
+                        "Target of block reference is not a sibling of this block\n"
+                        + s.target.diagnosticText("reference target")
+                        + diagnosticText("append block"));
             }
 
             for (Value v : s.arguments()) {
                 if (!isReachable(v)) {
                     throw new IllegalStateException(
-                            String.format("Argument of block reference %s of terminating operation %s is not defined in tree: %s", s, op, v));
+                            "Cannot append an operation because its successor argument is not reachable\n"
+                            + v.block.diagnosticText(v, "unreachable successor argument")
+                            + diagnosticText("referencing operation appended here"));
                 }
                 assert !v.isBuilt();
             }
@@ -915,4 +935,58 @@ private boolean isReachable(Value v) {
     boolean isBuilt() {
         return index >= 0;
     }
+
+    String diagnosticText(String label) {
+        return diagnosticText(null, label);
+    }
+
+    String diagnosticText(CodeItem pos, String label) {
+        Map<Block, String> blockNames = new IdentityHashMap<>();
+        Function<CodeItem, String> valueNamer = OpWriter.CodeItemNamerOption.defaultValue().namer();
+        Function<CodeItem, String> namer = i -> switch (i) {
+            case Block b -> blockNames.computeIfAbsent(b, _ -> "block_" + blockNames.size());
+            case Block.Reference r -> blockNames.computeIfAbsent(r.target, _ -> "block_" + blockNames.size());
+            case Value v -> valueNamer.apply(v);
+            default -> throw new IllegalArgumentException("Cannot name code item: " + i);
+        };
+        Body root = parentBody;
+        while (root.connectedAncestorBody != null) {
+            root = root.connectedAncestorBody;
+        }
+        root.elements().forEach(e -> {
+            switch (e) {
+                case Block b -> {
+                    blockNames.put(b, "block_" + blockNames.size());
+                    b.parameters().forEach(namer::apply);
+                }
+                case Op op when op.result() != null && !op.resultType().equals(JavaType.VOID) -> namer.apply(op.result());
+                default -> {
+                }
+            }
+        });
+        StringBuilder header = new StringBuilder("^").append(blockNames.computeIfAbsent(this, _ -> "block_" + blockNames.size()));
+        if (!parameters().isEmpty()) {
+            header.append(parameters().stream().map(value -> ("%" + namer.apply(value)) + " : "
+                    + JavaTypeUtils.flatten(value.type().externalize())).collect(Collectors.joining(", ", "(", ")")));
+        }
+        StringBuilder out = new StringBuilder(header).append(":\n");
+        if (pos instanceof Block.Parameter parameter) {
+            String value = "%" + namer.apply(parameter);
+            out.append(" ".repeat(header.indexOf(value)))
+               .append('^').append("~".repeat(Math.max(0, value.length() - 1))).append(' ')
+               .append(label).append('\n');
+        } else if (pos == null && label != null) {
+            out.append('^').append("~".repeat(Math.max(0, header.length() - 1))).append(' ').append(label).append('\n');
+        }
+        for (Op op : ops) {
+            String line = "  " + OpWriter.toText(op, OpWriter.CodeItemNamerOption.of(namer),
+                                                     OpWriter.OpDescendantsOption.DROP_DESCENDANTS,
+                                                     OpWriter.WritingModeOption.LENIENT);
+            out.append(line).append('\n');
+            if (pos == op || pos instanceof Op.Result result && result.op() == op) {
+                out.append("  ^").append("~".repeat(Math.max(0, line.length() - 3))).append(' ').append(label).append('\n');
+            }
+        }
+        return out.toString();
+    }
 }

src/jdk.incubator.code/share/classes/jdk/incubator/code/Body.java

@@ -746,9 +746,12 @@ private void sortReversePostorder() {
 
                         // Check successor arity
                         if (s.arguments().size() != target.parameters().size()) {
-                            String m = String.format("Reference to block %s with %d arguments but the block has %d parameters",
-                                    target, s.arguments().size(), target.parameters().size());
-                            throw new IllegalStateException(m);
+                            String m = String.format(
+                                    "Block reference argument count (%d) differs from target block parameter count (%d)",
+                                    s.arguments().size(), target.parameters().size());
+                            throw new IllegalStateException(m + "\n"
+                                    + n.diagnosticText(n.ops.getLast(), "block reference with wrong arity")
+                                    + target.diagnosticText("target block"));
                         }
 
                         // Update target's predecessors with n
@@ -781,7 +784,9 @@ private void sortReversePostorder() {
 
         private static void checkBlock(Block b) {
             if (b.ops.isEmpty() || !(b.ops.getLast() instanceof Op.Terminating)) {
-                throw new IllegalStateException("Block has no terminating operation as the last operation");
+                throw new IllegalStateException(
+                        "Block has no terminating operation as the last operation\n"
+                        + b.diagnosticText("missing terminating op"));
             }
         }
 
@@ -827,13 +832,19 @@ private void removeUnreachableBlocksAndValueUses(int nUnreachableBlocks) {
                         case Op op -> {
                             Op.Result use = op.result();
                             if (!use.uses.isEmpty()) {
-                                throw new IllegalStateException("Use of an operation result is not dominated by the result");
+                                throw new IllegalStateException(
+                                        "Use of an operation result is not dominated by the result\n"
+                                        + use.block.diagnosticText(use, "value declared here")
+                                        + use.uses.getFirst().block.diagnosticText(use.uses.getFirst(), "invalid use"));
                             }
                         }
                         case Block bb -> {
                             for (Block.Parameter p : bb.parameters()) {
                                 if (!p.uses.isEmpty()) {
-                                    throw new IllegalStateException("Use of block parameter is not dominated by the parameter");
+                                    throw new IllegalStateException(
+                                            "Use of block parameter is not dominated by the parameter\n"
+                                            + p.block.diagnosticText(p, "value declared here")
+                                            + p.uses.getFirst().block.diagnosticText(p.uses.getFirst(), "invalid use"));
                                 }
                             }
                         }
@@ -864,7 +875,9 @@ private void checkValueUse() {
                     for (Block.Parameter p : block.parameters()) {
                         for (Op.Result use : p.uses()) {
                             if (!use.declaringBlock().isDominatedBy(block)) {
-                                throw new IllegalStateException("Use of value is not dominated by value");
+                                throw new IllegalStateException("Use of value is not dominated by value\n"
+                                        + p.block.diagnosticText(p, "value declared here")
+                                        + use.block.diagnosticText(use, "invalid use"));
                             }
                         }
                     }
@@ -873,7 +886,9 @@ private void checkValueUse() {
                         Op.Result r = o.result();
                         for (Op.Result use : r.uses()) {
                             if (!use.declaringBlock().isDominatedBy(block)) {
-                                throw new IllegalStateException("Use of value is not dominated by value");
+                                throw new IllegalStateException("Use of value is not dominated by value\n"
+                                        + r.block.diagnosticText(r, "value declared here")
+                                        + use.block.diagnosticText(use, "invalid use"));
                             }
                         }
                     }

src/jdk.incubator.code/share/classes/jdk/incubator/code/CodeContext.java

@@ -125,7 +125,8 @@ public Value getValue(Value input) {
         if (output != null) {
             return output;
         }
-        throw new IllegalArgumentException("No mapping for input value: " + input);
+        throw new IllegalArgumentException("No output value is mapped to this input value in the code context\n"
+                + input.block.diagnosticText(input, "unmapped input value"));
     }
 
     /**
@@ -193,7 +194,8 @@ public void mapValue(Value input, Value output) {
         Objects.requireNonNull(output);
 
         if (output.isBuilt()) {
-            throw new IllegalArgumentException("Output value's declaring block is built: " + output);
+            throw new IllegalArgumentException("Output value's declaring block is built\n"
+                    + output.block.diagnosticText(output, "output value"));
         }
 
         if (valueMap == EMPTY_MAP) {
@@ -262,7 +264,8 @@ public Block.Builder getBlock(Block input) {
 
         Block.Builder output = blockMap.get(input);
         if (output == null) {
-            throw new IllegalArgumentException("No mapping for input block: " + input);
+            throw new IllegalArgumentException("No mapping for input block\n"
+                    + input.diagnosticText("input block"));
         }
 
         return output;
@@ -299,7 +302,8 @@ public void mapBlock(Block input, Block.Builder output) {
         Objects.requireNonNull(output);
 
         if (output.target().isBuilt()) {
-            throw new IllegalArgumentException("Output block builder is built: " + output);
+            throw new IllegalArgumentException("Output block builder is built\n"
+                    + output.target().diagnosticText("output block"));
         }
 
         if (blockMap == EMPTY_MAP) {
@@ -347,7 +351,8 @@ public Block.Reference getReference(Block.Reference input) {
 
         Block.Reference output = referenceMap.get(input);
         if (output == null) {
-            throw new IllegalArgumentException("No mapping for input block reference: " + input);
+            throw new IllegalArgumentException("No mapping for input block reference\n"
+                    + input.target.diagnosticText("reference target"));
         }
 
         return output;
@@ -385,12 +390,14 @@ public void mapReference(Block.Reference input, Block.Reference output) {
         Objects.requireNonNull(output);
 
         if (output.target.isBuilt()) {
-            throw new IllegalArgumentException("Output block reference's target block is built: " + output);
+            throw new IllegalArgumentException("Output block reference's target block is built\n"
+                    + output.target.diagnosticText("reference target"));
         }
 
         for (Value outputArgument : output.arguments()) {
             if (outputArgument.isBuilt()) {
-                throw new IllegalArgumentException("Output block reference argument's declaring block is built: " + outputArgument);
+                throw new IllegalArgumentException("Output block reference argument's declaring block is built\n"
+                        + outputArgument.block.diagnosticText(outputArgument, "reference argument"));
             }
         }
 
@@ -428,7 +435,9 @@ public Block.Reference getReferenceOrCreate(Block.Reference input) {
         // Create reference
         Block.Builder outputBlock = blockMap.get(input.targetBlock());
         if (outputBlock == null) {
-            throw new IllegalArgumentException("No mapping for input reference target block" + input.targetBlock());
+            throw new IllegalArgumentException(
+                    "No mapping for input block reference's target block\n"
+                    + input.target.diagnosticText("reference target"));
         }
         return outputBlock.reference(getValues(input.arguments()));
     }

src/jdk.incubator.code/share/classes/jdk/incubator/code/InternalAbstractOp.java

@@ -52,7 +52,9 @@ sealed abstract class InternalAbstractOp implements Op permits AbstractOp, Abstr
     protected InternalAbstractOp(List<? extends Value> operands) {
         for (Value operand : operands) {
             if (operand.isBuilt()) {
-                throw new IllegalArgumentException("Operand's declaring block is built: " + operand);
+                throw new IllegalArgumentException(
+                        "Operand's declaring block is built\n"
+                        + operand.block.diagnosticText(operand, "operand"));
             }
         }
         this.operands = List.copyOf(operands);
@@ -96,7 +98,9 @@ public final Block parent() {
         }
 
         if (!result.block.isBuilt()) {
-            throw new IllegalStateException("Parent block is unobservable");
+            throw new IllegalStateException(
+                    "Operation's parent block is unobservable\n"
+                    + result.block.diagnosticText(this, "operation with unobservable parent block"));
         }
 
         return result.block;

src/jdk.incubator.code/share/classes/jdk/incubator/code/Quoted.java

@@ -58,19 +58,22 @@ public final class Quoted<T extends Op> {
     public Quoted(T op, Map<Value, Object> operandsAndCapturedValues) {
         SequencedMap<Value, Object> m = new LinkedHashMap<>();
         for (Value value : op.operands()) {
-            m.put(value, runtimeValue(operandsAndCapturedValues, value));
+            m.put(value, runtimeValue(operandsAndCapturedValues, value, "operand"));
         }
         for (Value value : op.capturedValues()) {
-            m.put(value, runtimeValue(operandsAndCapturedValues, value));
+            m.put(value, runtimeValue(operandsAndCapturedValues, value, "captured value"));
         }
 
         this.op = op;
         this.operandsAndCapturedValues = Collections.unmodifiableSequencedMap(m);
     }
 
-    static Object runtimeValue(Map<Value, Object> operandsAndCapturedValues, Value value) {
+    static Object runtimeValue(Map<Value, Object> operandsAndCapturedValues, Value value, String role) {
         if (!operandsAndCapturedValues.containsKey(value)) {
-            throw new IllegalArgumentException("Value is not present as a key in the map of values");
+            throw new IllegalArgumentException(
+                    "Cannot create the quoted form of an operation because no runtime value was provided for this "
+                    + role + "\n"
+                    + value.block.diagnosticText(value, role + " without runtime value"));
         }
         return operandsAndCapturedValues.get(value);
     }