Fix: Bytecode DSL parser should detect bad root node overrides in parent classes; code generation should not delegate to super method if they are abstract.

DSouzaM · DSouzaM · commit bc63fb10445f · 2025-11-07T08:47:13.000-05:00
diff --git a/truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/error_tests/ErrorTests.java b/truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/error_tests/ErrorTests.java
@@ -367,7 +367,7 @@ protected BadOverrides(ErrorLanguage language, FrameDescriptor frameDescriptor)
             super(language, frameDescriptor);
         }
 
-        private static final String ERROR_MESSAGE_DELEGATED = "This method is overridden by the generated Bytecode DSL class, so it cannot be declared final.";
+        private static final String ERROR_MESSAGE_DELEGATED = "This method is overridden by the generated Bytecode DSL class. It cannot be declared final.";
         private static final String ERROR_MESSAGE = ERROR_MESSAGE_DELEGATED +
                         " You can remove the final modifier to resolve this issue, but since the override will make this method unreachable, it is recommended to simply remove it.";
 
@@ -389,7 +389,7 @@ public final int findBytecodeIndex(Node node, Frame frame) {
             return 0;
         }
 
-        @ExpectError(ERROR_MESSAGE)
+        @ExpectError(ERROR_MESSAGE_DELEGATED)
         @Override
         public final Node findInstrumentableCallNode(Node callNode, Frame frame, int bytecodeIndex) {
             return null;
@@ -401,7 +401,7 @@ public final boolean isInstrumentable() {
             return false;
         }
 
-        @ExpectError(ERROR_MESSAGE)
+        @ExpectError(ERROR_MESSAGE + " Bytecode DSL interpreters should use GenerateBytecode#captureFramesForTrace instead.")
         @Override
         public final boolean isCaptureFramesForTrace(boolean compiledFrame) {
             return false;
@@ -459,6 +459,11 @@ public final BytecodeRootNodes<?> getRootNodes() {
      * methods. The generated code should respect the wider visibility (otherwise, a compiler error
      * will occur).
      */
+    @ExpectWarning({
+                    "Method isInstrumentable() in supertype com.oracle.truffle.api.bytecode.test.error_tests.ErrorTests.RootNodeWithOverrides is overridden by the generated Bytecode DSL class." +
+                                    " You can suppress this warning by re-declaring the method as abstract in this class.",
+                    "Method prepareForInstrumentation(Set<Class<?>>) in supertype com.oracle.truffle.api.bytecode.test.error_tests.ErrorTests.RootNodeWithMoreOverrides is overridden by the generated Bytecode DSL class." +
+                                    " You can suppress this warning by re-declaring the method as abstract in this class."})
     @GenerateBytecode(languageClass = ErrorLanguage.class, enableTagInstrumentation = true)
     public abstract static class AcceptableOverrides extends RootNodeWithMoreOverrides implements BytecodeRootNode {
         protected AcceptableOverrides(ErrorLanguage language, FrameDescriptor frameDescriptor) {
@@ -473,19 +478,47 @@ static int add(int x, int y) {
             }
         }
 
-        @ExpectWarning("This method is overridden by the generated Bytecode DSL class, so this definition is unreachable and can be removed.")
+        @ExpectWarning("This method is overridden by the generated Bytecode DSL class. It is unreachable and can be removed.")
         @Override
         public int findBytecodeIndex(Node node, Frame frame) {
             return super.findBytecodeIndex(node, frame);
         }
 
-        @ExpectWarning("This method is overridden by the generated Bytecode DSL class, so this definition is unreachable and can be removed.")
+        @ExpectWarning("This method is overridden by the generated Bytecode DSL class. It is unreachable and can be removed. Bytecode DSL interpreters should use GenerateBytecode#captureFramesForTrace instead.")
         @Override
         public boolean isCaptureFramesForTrace(boolean compiledFrame) {
             return super.isCaptureFramesForTrace(compiledFrame);
         }
     }
 
+    // Unlike AcceptableOverrides, this class suppresses warnings about inherited overrides by
+    // redeclaring them as abstract.
+    @GenerateBytecode(languageClass = ErrorLanguage.class, enableTagInstrumentation = true)
+    public abstract static class SuppressedOverrideWarnings extends RootNodeWithMoreOverrides implements BytecodeRootNode {
+
+        protected SuppressedOverrideWarnings(ErrorLanguage language, FrameDescriptor frameDescriptor) {
+            super(language, frameDescriptor);
+        }
+
+        @Operation
+        public static final class Add {
+            @Specialization
+            static int add(int x, int y) {
+                return x + y;
+            }
+        }
+
+        @Override
+        protected abstract Node findInstrumentableCallNode(Node callNode, Frame frame, int bytecodeIndex);
+
+        @Override
+        public abstract boolean isInstrumentable();
+
+        @Override
+        public abstract void prepareForInstrumentation(Set<Class<?>> tags);
+
+    }
+
     private abstract static class RootNodeWithOverrides extends RootNode {
         protected RootNodeWithOverrides(ErrorLanguage language, FrameDescriptor frameDescriptor) {
             super(language, frameDescriptor);
@@ -517,6 +550,63 @@ public void prepareForInstrumentation(Set<Class<?>> tags) {
             super.prepareForInstrumentation(tags);
         }
 
+        @Override
+        protected boolean prepareForCompilation(boolean rootCompilation, int compilationTier, boolean lastTier) {
+            return super.prepareForCompilation(rootCompilation, compilationTier, lastTier);
+        }
+
+    }
+
+    @GenerateBytecode(languageClass = ErrorLanguage.class, enableTagInstrumentation = true)
+    @ExpectError({
+                    "Method findInstrumentableCallNode(Node, Frame, int) in supertype com.oracle.truffle.api.bytecode.test.error_tests.ErrorTests.RootNodeWithFinalOverrides is overridden by the generated Bytecode DSL class." +
+                                    " It cannot be declared final.",
+                    "Method prepareForInstrumentation(Set<Class<?>>) in supertype com.oracle.truffle.api.bytecode.test.error_tests.ErrorTests.RootNodeWithFinalOverrides is overridden by the generated Bytecode DSL class." +
+                                    " It cannot be declared final.",
+                    "Method isCaptureFramesForTrace(boolean) in supertype com.oracle.truffle.api.bytecode.test.error_tests.ErrorTests.RootNodeWithFinalOverrides is overridden by the generated Bytecode DSL class." +
+                                    " It cannot be declared final. Bytecode DSL interpreters should use GenerateBytecode#captureFramesForTrace instead."
+    })
+    public abstract static class InheritsFinalOverrides extends RootNodeWithFinalOverrides implements BytecodeRootNode {
+
+        protected InheritsFinalOverrides(ErrorLanguage language, FrameDescriptor frameDescriptor) {
+            super(language, frameDescriptor);
+        }
+
+        @Operation
+        public static final class Add {
+            @Specialization
+            static int add(int x, int y) {
+                return x + y;
+            }
+        }
+
+    }
+
+    private abstract static class RootNodeWithFinalOverrides extends RootNode {
+        protected RootNodeWithFinalOverrides(ErrorLanguage language, FrameDescriptor frameDescriptor) {
+            super(language, frameDescriptor);
+        }
+
+        @Override
+        protected final Node findInstrumentableCallNode(Node callNode, Frame frame, int bytecodeIndex) {
+            return super.findInstrumentableCallNode(callNode, frame, bytecodeIndex);
+        }
+
+        @Override
+        protected final boolean isCaptureFramesForTrace(boolean compiledFrame) {
+            return super.isCaptureFramesForTrace(compiledFrame);
+        }
+
+        @Override
+        public final void prepareForInstrumentation(Set<Class<?>> tags) {
+            super.prepareForInstrumentation(tags);
+        }
+
+        // This one is OK, we only override if not final
+        @Override
+        protected final Object translateStackTraceElement(TruffleStackTraceElement element) {
+            return super.translateStackTraceElement(element);
+        }
     }
 
     @ExpectError("The used type system is invalid. Fix errors in the type system first.")
diff --git a/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/generator/BytecodeRootNodeElement.java b/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/generator/BytecodeRootNodeElement.java
@@ -904,7 +904,13 @@ private CodeExecutableElement createFindInstrumentableCallNode() {
         CodeTreeBuilder b = ex.createBuilder();
         b.startDeclaration(types.BytecodeNode, "bc").startStaticCall(types.BytecodeNode, "get").string("callNode").end().end();
         b.startIf().string("bc == null || !(bc instanceof AbstractBytecodeNode bytecodeNode)").end().startBlock();
-        b.startReturn().string("super.findInstrumentableCallNode(callNode, frame, bytecodeIndex)").end();
+        ExecutableElement superImpl = ElementUtils.findMethodInClassHierarchy(ElementUtils.findMethod(types.RootNode, "findInstrumentableCallNode"), model.templateType);
+        if (superImpl.getModifiers().contains(ABSTRACT)) {
+            // edge case: root node could redeclare findInstrumentableCallNode as abstract.
+            b.startReturn().string("null").end();
+        } else {
+            b.startReturn().string("super.findInstrumentableCallNode(callNode, frame, bytecodeIndex)").end();
+        }
         b.end();
         b.statement("return bytecodeNode.findInstrumentableCallNode(bytecodeIndex)");
         return ex;
@@ -1705,8 +1711,8 @@ private CodeExecutableElement createPrepareForCompilation() {
         // Disable compilation for the uncached interpreter.
         b.string("bytecode.getTier() != ").staticReference(types.BytecodeTier, "UNCACHED");
 
-        ExecutableElement parentImpl = ElementUtils.findOverride(ElementUtils.findMethod(types.RootNode, "prepareForCompilation", 3), model.templateType);
-        if (parentImpl != null) {
+        ExecutableElement parentImpl = ElementUtils.findMethodInClassHierarchy(ElementUtils.findMethod(types.RootNode, "prepareForCompilation", 3), model.templateType);
+        if (parentImpl != null && !parentImpl.getModifiers().contains(ABSTRACT)) {
             // Delegate to the parent impl.
             b.string(" && ").startCall("super.prepareForCompilation").variables(ex.getParameters()).end();
         }
diff --git a/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/parser/BytecodeDSLParser.java b/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/parser/BytecodeDSLParser.java
@@ -387,53 +387,7 @@ private void parseBytecodeDSLModel(TypeElement typeElement, BytecodeDSLModel mod
         model.interceptInternalException = ElementUtils.findMethod(typeElement, "interceptInternalException");
         model.interceptTruffleException = ElementUtils.findMethod(typeElement, "interceptTruffleException");
 
-        // Detect method implementations that will be overridden by the generated class.
-        List<ExecutableElement> overrides = new ArrayList<>(List.of(
-                        ElementUtils.findMethod(types.RootNode, "execute"),
-                        ElementUtils.findMethod(types.RootNode, "computeSize"),
-                        ElementUtils.findMethod(types.RootNode, "findBytecodeIndex"),
-                        ElementUtils.findMethod(types.RootNode, "findInstrumentableCallNode"),
-                        ElementUtils.findMethod(types.RootNode, "isInstrumentable"),
-                        ElementUtils.findMethod(types.RootNode, "isCaptureFramesForTrace"),
-                        ElementUtils.findMethod(types.RootNode, "prepareForCall"),
-                        ElementUtils.findMethod(types.RootNode, "prepareForInstrumentation"),
-                        ElementUtils.findMethod(types.BytecodeRootNode, "getBytecodeNode"),
-                        ElementUtils.findMethod(types.BytecodeRootNode, "getRootNodes"),
-                        ElementUtils.findMethod(types.BytecodeOSRNode, "executeOSR"),
-                        ElementUtils.findMethod(types.BytecodeOSRNode, "getOSRMetadata"),
-                        ElementUtils.findMethod(types.BytecodeOSRNode, "setOSRMetadata"),
-                        ElementUtils.findMethod(types.BytecodeOSRNode, "storeParentFrameInArguments"),
-                        ElementUtils.findMethod(types.BytecodeOSRNode, "restoreParentFrameFromArguments")));
-
-        for (ExecutableElement override : overrides) {
-            ExecutableElement declared = ElementUtils.findMethod(typeElement, override.getSimpleName().toString());
-            if (declared == null) {
-                continue;
-            }
-
-            if (declared.getModifiers().contains(Modifier.FINAL)) {
-                model.addError(declared,
-                                "This method is overridden by the generated Bytecode DSL class, so it cannot be declared final. " +
-                                                "You can remove the final modifier to resolve this issue, but since the override will make this method unreachable, it is recommended to simply remove it.");
-            } else {
-                model.addWarning(declared, "This method is overridden by the generated Bytecode DSL class, so this definition is unreachable and can be removed.");
-            }
-        }
-
-        List<ExecutableElement> overridesWithDelegation = new ArrayList<>(List.of(
-                        ElementUtils.findMethod(types.RootNode, "prepareForCompilation")));
-
-        for (ExecutableElement override : overridesWithDelegation) {
-            ExecutableElement declared = ElementUtils.findMethod(typeElement, override.getSimpleName().toString());
-            if (declared == null) {
-                continue;
-            }
-
-            if (declared.getModifiers().contains(Modifier.FINAL)) {
-                model.addError(declared, "This method is overridden by the generated Bytecode DSL class, so it cannot be declared final.");
-            }
-        }
-
+        checkRootNodeOverrides(typeElement, model);
         if (model.hasErrors()) {
             return;
         }
@@ -710,6 +664,92 @@ private void parseBytecodeDSLModel(TypeElement typeElement, BytecodeDSLModel mod
         return;
     }
 
+    /**
+     * Detect method implementations that will be overridden by the generated class and report an
+     * appropriate message.
+     */
+    private void checkRootNodeOverrides(TypeElement typeElement, BytecodeDSLModel model) {
+        List<ExecutableElement> overrides = new ArrayList<>(List.of(
+                        ElementUtils.findMethod(types.RootNode, "execute"),
+                        ElementUtils.findMethod(types.RootNode, "computeSize"),
+                        ElementUtils.findMethod(types.RootNode, "findBytecodeIndex"),
+                        ElementUtils.findMethod(types.RootNode, "isInstrumentable"),
+                        ElementUtils.findMethod(types.RootNode, "prepareForCall"),
+                        ElementUtils.findMethod(types.RootNode, "prepareForInstrumentation"),
+                        ElementUtils.findMethod(types.BytecodeRootNode, "getBytecodeNode"),
+                        ElementUtils.findMethod(types.BytecodeRootNode, "getRootNodes"),
+                        ElementUtils.findMethod(types.BytecodeOSRNode, "executeOSR"),
+                        ElementUtils.findMethod(types.BytecodeOSRNode, "getOSRMetadata"),
+                        ElementUtils.findMethod(types.BytecodeOSRNode, "setOSRMetadata"),
+                        ElementUtils.findMethod(types.BytecodeOSRNode, "storeParentFrameInArguments"),
+                        ElementUtils.findMethod(types.BytecodeOSRNode, "restoreParentFrameFromArguments")));
+
+        for (ExecutableElement override : overrides) {
+            checkRootNodeOverride(typeElement, override, model, false, null);
+        }
+
+        String captureFramesForTraceMessage = String.format("Bytecode DSL interpreters should use %s#captureFramesForTrace instead.", getSimpleName(types.GenerateBytecode));
+        checkRootNodeOverride(typeElement, ElementUtils.findInstanceMethod(context.getTypeElement(types.RootNode), "isCaptureFramesForTrace", new TypeMirror[]{context.getType(boolean.class)}), model,
+                        false, captureFramesForTraceMessage);
+
+        List<ExecutableElement> overridesWithDelegation = new ArrayList<>(List.of(
+                        ElementUtils.findMethod(types.RootNode, "findInstrumentableCallNode"),
+                        ElementUtils.findMethod(types.RootNode, "prepareForCompilation")));
+
+        for (ExecutableElement override : overridesWithDelegation) {
+            checkRootNodeOverride(typeElement, override, model, true, null);
+        }
+    }
+
+    private void checkRootNodeOverride(TypeElement typeElement, ExecutableElement rootNodeMethod, BytecodeDSLModel model, boolean delegated, String customMessage) {
+        ExecutableElement override = ElementUtils.findOverride(rootNodeMethod, typeElement);
+        if (override == null || ElementUtils.typeEquals(override.getEnclosingElement().asType(), types.RootNode) || override.getModifiers().contains(Modifier.ABSTRACT)) {
+            return;
+        }
+        boolean inherited = !override.getEnclosingElement().equals(typeElement);
+
+        Element messageElement;
+        String message;
+        if (inherited) {
+            messageElement = typeElement;
+            message = String.format("Method %s in supertype %s", ElementUtils.getReadableSignature(override), override.getEnclosingElement());
+        } else {
+            messageElement = override;
+            message = "This method";
+        }
+        message += " is overridden by the generated Bytecode DSL class.";
+
+        if (override.getModifiers().contains(Modifier.FINAL)) {
+            model.addError(messageElement, message + " " + badFinalOverrideMessage(delegated, inherited, customMessage));
+        } else if (!delegated) {
+            model.addWarning(messageElement, message + " " + badOverrideMessage(inherited, customMessage));
+        }
+    }
+
+    private static String badFinalOverrideMessage(boolean delegated, boolean inherited, String customMessage) {
+        String message = "It cannot be declared final.";
+        if (!delegated && !inherited) {
+            message += " You can remove the final modifier to resolve this issue, but since the override will make this method unreachable, it is recommended to simply remove it.";
+        }
+        if (customMessage != null) {
+            message += " " + customMessage;
+        }
+        return message;
+    }
+
+    private static String badOverrideMessage(boolean inherited, String customMessage) {
+        String message;
+        if (inherited) {
+            message = "You can suppress this warning by re-declaring the method as abstract in this class.";
+        } else {
+            message = "It is unreachable and can be removed.";
+        }
+        if (customMessage != null) {
+            message += " " + customMessage;
+        }
+        return message;
+    }
+
     private void resolveBoxingElimination(BytecodeDSLModel model, List<QuickenDecision> manualQuickenings) {
         /*
          * If boxing elimination is enabled and the language uses operations with statically known
diff --git a/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/java/ElementUtils.java b/truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/java/ElementUtils.java
