From 46bdd858e8ade58a022b06ac586e496e75f2b598 Mon Sep 17 00:00:00 2001 From: Joshua Gardner Date: Fri, 24 Apr 2026 11:03:43 +1000 Subject: [PATCH 1/7] Introduce `RoutineExit` type to control flow graph types This is to facilitate the distinction between routine exits and exceptional exits from a control flow graph. --- .../delphi/cfg/api/RoutineExit.java | 22 +++++++++++++++++++ .../integradev/delphi/cfg/api/Terminus.java | 2 +- .../delphi/cfg/block/ProtoBlockFactory.java | 10 ++++----- 3 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/RoutineExit.java diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/RoutineExit.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/RoutineExit.java new file mode 100644 index 000000000..f52510be9 --- /dev/null +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/RoutineExit.java @@ -0,0 +1,22 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2026 Integrated Application Development + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package au.com.integradev.delphi.cfg.api; + +/** A block representing the end of a routine */ +public interface RoutineExit extends Terminus {} diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Terminus.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Terminus.java index f3db9210b..e2557a462 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Terminus.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Terminus.java @@ -21,7 +21,7 @@ import java.util.Collections; import java.util.Set; -/** A block which has no successors by nature, e.g., the end of a routine */ +/** A block that ends paths through a {@link ControlFlowGraph} */ public interface Terminus extends Block { @Override default Set getSuccessors() { diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java index d8b82fb26..964299e16 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java @@ -25,7 +25,7 @@ import au.com.integradev.delphi.cfg.api.Halt; import au.com.integradev.delphi.cfg.api.Linear; import au.com.integradev.delphi.cfg.api.PossibleException; -import au.com.integradev.delphi.cfg.api.Terminus; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.api.UnconditionalJump; import au.com.integradev.delphi.cfg.api.UnknownException; import java.util.Collection; @@ -42,7 +42,7 @@ private ProtoBlockFactory() { } public static ProtoBlock exitBlock() { - return new ProtoBlock(TerminusImpl::new, (blocks, block) -> {}); + return new ProtoBlock(RoutineExitImpl::new, (blocks, block) -> {}); } public static ProtoBlock halt(DelphiNode terminator) { @@ -442,9 +442,9 @@ public String getBlockType() { } } - private static class TerminusImpl extends BlockImpl implements Terminus { + private static class RoutineExitImpl extends BlockImpl implements RoutineExit { - public TerminusImpl(List elements) { + public RoutineExitImpl(List elements) { super(elements); } @@ -460,7 +460,7 @@ public String getDescription() { @Override public String getBlockType() { - return "Terminus"; + return "RoutineExit"; } } From c8778f4c2d865c98f7c5eb141e85b21c3a1299a4 Mon Sep 17 00:00:00 2001 From: Joshua Gardner Date: Fri, 24 Apr 2026 12:16:17 +1000 Subject: [PATCH 2/7] Remove `ControlFlowGraph::getExitBlock` This is being removed in favour of the type system and a more generic approach to block identification. Additionally, there may be more than one exit block in a CFG. --- .../checks/LoopExecutingAtMostOnceCheck.java | 3 ++- .../delphi/checks/NoreturnContractCheck.java | 19 +++++++++++-------- .../delphi/cfg/ControlFlowGraphBuilder.java | 3 +-- .../delphi/cfg/ControlFlowGraphImpl.java | 9 +-------- .../delphi/cfg/api/ControlFlowGraph.java | 7 ------- .../delphi/cfg/ControlFlowGraphTest.java | 6 ++---- 6 files changed, 17 insertions(+), 30 deletions(-) diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheck.java index f1d99e559..9afccd1c2 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheck.java @@ -23,6 +23,7 @@ import au.com.integradev.delphi.cfg.api.Branch; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; import au.com.integradev.delphi.cfg.api.Terminated; +import au.com.integradev.delphi.cfg.api.Terminus; import au.com.integradev.delphi.utils.ControlFlowGraphUtils; import java.util.ArrayDeque; import java.util.ArrayList; @@ -244,7 +245,7 @@ private static boolean jumpsBeforeLoop(ControlFlowGraph cfg, Block loopBlock, De return true; } if ((search.getSuccessors().size() == 1 && search.getSuccessors().contains(jumpTarget)) - || search.equals(cfg.getExitBlock())) { + || search instanceof Terminus) { return false; } diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java index b8fea04b1..9564588cf 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java @@ -22,6 +22,7 @@ import au.com.integradev.delphi.cfg.api.ControlFlowGraph; import au.com.integradev.delphi.cfg.api.Finally; import au.com.integradev.delphi.cfg.api.PossibleException; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.api.Terminated; import au.com.integradev.delphi.cfg.api.UnknownException; import au.com.integradev.delphi.cfg.block.TerminatorKind; @@ -50,11 +51,11 @@ public DelphiCheckContext visit(RoutineImplementationNode routine, DelphiCheckCo private static boolean canReturnNormally(RoutineImplementationNode routine) { ControlFlowGraph cfg = ControlFlowGraphUtils.findContainingCFG(routine); - return cfg != null && canReturnNormally(cfg, cfg.getEntryBlock()); + return cfg != null && canReturnNormally(cfg.getEntryBlock()); } - private static boolean canReturnNormally(ControlFlowGraph cfg, Block block) { - if (block.equals(cfg.getExitBlock())) { + private static boolean canReturnNormally(Block block) { + if (block instanceof RoutineExit) { return true; } @@ -63,18 +64,20 @@ private static boolean canReturnNormally(ControlFlowGraph cfg, Block block) { if (isGuaranteedException(block)) { successors = block.getSuccessors().stream() - .filter(s -> notFinallyOrExit(cfg, s)) + .filter(NoreturnContractCheck::notFinallyOrExit) .collect(Collectors.toSet()); } else if (block instanceof PossibleException) { successors = new HashSet<>(); successors.add(((PossibleException) block).getSuccessor()); ((PossibleException) block) - .getExceptions().stream().filter(s -> notFinallyOrExit(cfg, s)).forEach(successors::add); + .getExceptions().stream() + .filter(NoreturnContractCheck::notFinallyOrExit) + .forEach(successors::add); } else { successors = block.getSuccessors(); } - return successors.stream().anyMatch(s -> canReturnNormally(cfg, s)); + return successors.stream().anyMatch(NoreturnContractCheck::canReturnNormally); } private static boolean isGuaranteedException(Block block) { @@ -83,7 +86,7 @@ private static boolean isGuaranteedException(Block block) { && ((Terminated) block).getTerminatorKind() == TerminatorKind.RAISE); } - private static boolean notFinallyOrExit(ControlFlowGraph cfg, Block block) { - return !(block instanceof Finally || block.equals(cfg.getExitBlock())); + private static boolean notFinallyOrExit(Block block) { + return !(block instanceof Finally || block instanceof RoutineExit); } } diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java index ab795cbfa..96cff0fe5 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java @@ -73,8 +73,7 @@ public ControlFlowGraph build() { } ControlFlowGraphImpl cfg = - new ControlFlowGraphImpl( - map.get(currentBlock), map.get(exitBlocks.peek()), new ArrayList<>(map.values())); + new ControlFlowGraphImpl(map.get(currentBlock), new ArrayList<>(map.values())); cfg.prune(); populatePredecessors(cfg); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java index f5db511cd..af20dd4c8 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java @@ -32,12 +32,10 @@ public class ControlFlowGraphImpl implements ControlFlowGraph { private Block entry; - private final Block exit; private final List blocks; - public ControlFlowGraphImpl(Block entry, Block exit, List blocks) { + public ControlFlowGraphImpl(Block entry, List blocks) { this.entry = entry; - this.exit = exit; this.blocks = blocks; } @@ -46,11 +44,6 @@ public Block getEntryBlock() { return entry; } - @Override - public Block getExitBlock() { - return exit; - } - @Override public List getBlocks() { return Collections.unmodifiableList(Lists.reverse(blocks)); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ControlFlowGraph.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ControlFlowGraph.java index adf01982c..72a9e3e23 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ControlFlowGraph.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ControlFlowGraph.java @@ -29,13 +29,6 @@ public interface ControlFlowGraph { */ Block getEntryBlock(); - /** - * The final exit block of the control flow graph - * - * @return the final block of the control flow graph - */ - Block getExitBlock(); - /** * All the blocks within the control flow graph * diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java index 7c6571d5b..46de7c8b4 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java @@ -29,7 +29,7 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; import au.com.integradev.delphi.cfg.api.Linear; -import au.com.integradev.delphi.cfg.api.Terminus; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.block.TerminatorKind; import au.com.integradev.delphi.cfg.checker.GraphChecker; import au.com.integradev.delphi.cfg.checker.StatementTerminator; @@ -247,9 +247,7 @@ void testSimplestCfg() { Block exit = entry.getSuccessors().iterator().next(); assertThat(exit) .withFailMessage("Expecting entry block's successor to be the exit block") - .isEqualTo(cfg.getExitBlock()) - .withFailMessage("Expecting entry block's successor to be of type Terminus.") - .isInstanceOf(Terminus.class); + .isInstanceOf(RoutineExit.class); } @Test From 0a60e10269f875dd70c2986ce9934b17b44c3073 Mon Sep 17 00:00:00 2001 From: Joshua Gardner Date: Fri, 24 Apr 2026 15:19:43 +1000 Subject: [PATCH 3/7] Fix CFG construction of `Continue` and `Break` around `try finally` This fixes a bug that allowed loop control flow statements to be used inside `try finally` statements with the existance of a loop. --- .../delphi/cfg/ControlFlowGraphBuilder.java | 4 ++ .../delphi/cfg/ControlFlowGraphVisitor.java | 8 ++- .../delphi/cfg/ControlFlowGraphTest.java | 62 ++++++++++++++++--- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java index 96cff0fe5..fe1e0ed7d 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java @@ -109,6 +109,10 @@ public ProtoBlock getContinueTarget() { return continueTargets.peek(); } + public boolean isInLoop() { + return !continueTargets.isEmpty(); + } + public void pushLoopContext(ProtoBlock continueTarget, ProtoBlock breakTarget) { continueTargets.push(continueTarget); breakTargets.push(breakTarget); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java index 99b3ed841..c6b077182 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java @@ -551,7 +551,10 @@ private ControlFlowGraphBuilder buildTryFinally( node.getFinallyBlock(), builder.getCurrentBlock(), builder.getExitBlock())); build(finallyNode.getStatementList(), builder); - builder.pushLoopContext(builder.getCurrentBlock(), builder.getCurrentBlock()); + boolean inLoop = builder.isInLoop(); + if (inLoop) { + builder.pushLoopContext(builder.getCurrentBlock(), builder.getCurrentBlock()); + } builder.pushExitBlock(builder.getCurrentBlock()); builder.addBlockBeforeCurrent(); @@ -565,6 +568,9 @@ private ControlFlowGraphBuilder buildTryFinally( builder.addElement(node); builder.popExitBlock(); + if (inLoop) { + builder.popLoopContext(); + } return builder; } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java index 46de7c8b4..e8afd1358 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java @@ -53,6 +53,8 @@ import java.util.function.Consumer; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; @@ -777,18 +779,62 @@ void testEmptyForIn() { block(element(NameReferenceNode.class, "A")).succeedsTo(0))); } - @Test - void testBreakOutsideOfLoop() { - GraphChecker checker = checker(); - assertThatThrownBy(() -> test("Break;", checker)) + @ParameterizedTest + @ValueSource( + strings = { + "Break", + // for to + "Break; for I := Foo to Bar do;", + "for I := Foo to Bar do; Break;", + // for in + "Break; for I in Foo do;", + "for I in Foo do; Break;", + // while + "Break; while Foo do;", + "while Foo do; Break;", + // repeat until + "Break; repeat until Foo;", + "repeat until Foo; Break;", + // try finally + "Break; try finally end;", + "try Break finally end;", + "try finally Break end;", + "try finally end; Break;", + "try try Break finally end finally end;", + "try try finally Break end finally end;", + }) + void testBreakOutsideOfLoop(String test) { + assertThatThrownBy(() -> buildCfg(test)) .withFailMessage("'Break' statement not in loop statement.") .isInstanceOf(IllegalStateException.class); } - @Test - void testContinueOutsideOfLoop() { - GraphChecker checker = checker(); - assertThatThrownBy(() -> test("Continue;", checker)) + @ParameterizedTest + @ValueSource( + strings = { + "Continue", + // for to + "Continue; for I := Foo to Bar do;", + "for I := Foo to Bar do; Continue;", + // for in + "Continue; for I in Foo do;", + "for I in Foo do; Continue;", + // while + "Continue; while Foo do;", + "while Foo do; Continue;", + // repeat until + "Continue; repeat until Foo;", + "repeat until Foo; Continue;", + // try finally + "Continue; try finally end;", + "try Continue finally end;", + "try finally Continue end;", + "try finally end; Continue;", + "try try Continue finally end finally end;", + "try try finally Continue end finally end;", + }) + void testContinueOutsideOfLoop(String test) { + assertThatThrownBy(() -> buildCfg(test)) .withFailMessage("'Continue' statement not in loop statement.") .isInstanceOf(IllegalStateException.class); } From c2f60ab237575af7de79b0693cf5903dc5c2240f Mon Sep 17 00:00:00 2001 From: Joshua Gardner Date: Mon, 11 May 2026 16:24:13 +1000 Subject: [PATCH 4/7] Ensure `RoutineExit` block is always id 0 --- .../delphi/cfg/ControlFlowGraphBuilder.java | 11 +- .../delphi/cfg/ControlFlowGraphTest.java | 288 +++++++++--------- 2 files changed, 154 insertions(+), 145 deletions(-) diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java index fe1e0ed7d..afb1c1982 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java @@ -20,6 +20,7 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.block.BlockImpl; import au.com.integradev.delphi.cfg.block.ProtoBlock; import au.com.integradev.delphi.cfg.block.ProtoBlockFactory; @@ -92,8 +93,14 @@ private static void populatePredecessors(ControlFlowGraph cfg) { private static void populateIds(ControlFlowGraph cfg) { List blocks = Lists.reverse(cfg.getBlocks()); - for (int blockId = 0; blockId < blocks.size(); blockId++) { - ((BlockImpl) blocks.get(blockId)).setId(blockId); + int blockId = 0; + for(Block block : blocks) { + if (block instanceof RoutineExit) { + // Ensure `RoutineExit` is always id 0 + ((BlockImpl) block).setId(0); + } else { + ((BlockImpl) block).setId(++blockId); + } } } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java index e8afd1358..276ac3579 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java @@ -85,6 +85,8 @@ class ControlFlowGraphTest { private static final Logger LOG = LoggerFactory.getLogger(ControlFlowGraphTest.class); + private static final int EXIT_ID = 0; + private ControlFlowGraph buildCfg(String input) { return buildCfg(Collections.emptyMap(), input); } @@ -241,7 +243,7 @@ void testEmptyCfg() { @Test void testSimplestCfg() { final ControlFlowGraph cfg = buildCfg("Foo;"); - checker(block(element(NameReferenceNode.class, "Foo")).succeedsTo(0)).check(cfg); + checker(block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID)).check(cfg); Block entry = cfg.getEntryBlock(); assertThat(entry) .withFailMessage("Expecting entry block to have single successor") @@ -258,9 +260,9 @@ void testIfThen() { "if A then Foo;", checker( block(element(NameReferenceNode.class, "A")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -271,8 +273,8 @@ void testIfThenElse() { block(element(NameReferenceNode.class, "A")) .branchesTo(2, 1) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0), - block(element(NameReferenceNode.class, "Bar")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -283,11 +285,11 @@ void testIfThenElseIf() { block(element(NameReferenceNode.class, "A")) .branchesTo(3, 2) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0), + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID), block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Bar")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -300,9 +302,9 @@ void testIfOr() { .withTerminator(BinaryExpressionNode.class) .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.OR)), block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -311,13 +313,13 @@ void testIfAnd() { "if A and B then Foo;", checker( block(element(NameReferenceNode.class, "A")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(BinaryExpressionNode.class) .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.AND)), block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -328,7 +330,7 @@ void testEmptyIf() { block(element(NameReferenceNode.class, "C")) .branchesTo(1, 1) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -339,14 +341,14 @@ void testEmptyIfElse() { block(element(NameReferenceNode.class, "C")) .branchesTo(1, 1) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test void testLocalVarDeclaration() { test( "var Bar: TObject;", - checker(block(element(NameDeclarationNode.class, "Bar")).succeedsTo(0))); + checker(block(element(NameDeclarationNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -357,7 +359,7 @@ void testLocalVarListDeclaration() { block( element(NameDeclarationNode.class, "Foo"), element(NameDeclarationNode.class, "Bar")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -366,7 +368,7 @@ void testLocalVarValueDeclaration() { "var Bar: TObject := nil;", checker( block(element(NilLiteralNode.class), element(NameDeclarationNode.class, "Bar")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -375,7 +377,7 @@ void testUntypedConstDeclaration() { "const Foo = 0;", checker( block(element(IntegerLiteralNode.class), element(NameDeclarationNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -384,7 +386,7 @@ void testTypedConstDeclaration() { "const Foo: Integer = 0;", checker( block(element(IntegerLiteralNode.class), element(NameDeclarationNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -392,9 +394,9 @@ void testCaseStatement1Arm() { test( "case Foo of Bar: Bar1; end;", checker( - block(element(NameReferenceNode.class, "Bar1")).succeedsTo(0), + block(element(NameReferenceNode.class, "Bar1")).succeedsTo(EXIT_ID), block(element(NameReferenceNode.class, "Foo"), element(NameReferenceNode.class, "Bar")) - .succeedsToCases(0, 2) + .succeedsToCases(EXIT_ID, 2) .withTerminator(CaseStatementNode.class))); } @@ -403,14 +405,14 @@ void testCaseStatement2Arm() { test( "case Foo of Bar: Bar1; Baz: Baz1; end;", checker( - block(element(NameReferenceNode.class, "Bar1")).succeedsTo(0), - block(element(NameReferenceNode.class, "Baz1")).succeedsTo(0), + block(element(NameReferenceNode.class, "Bar1")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Baz1")).succeedsTo(EXIT_ID), block( element(NameReferenceNode.class, "Foo"), element(NameReferenceNode.class, "Bar"), element(NameReferenceNode.class, "Baz")) .withTerminator(CaseStatementNode.class) - .succeedsToCases(0, 2, 3))); + .succeedsToCases(EXIT_ID, 2, 3))); } @Test @@ -418,8 +420,8 @@ void testCaseStatementRangeArm() { test( "case Foo of 1..2: Bar1; 3..4: Baz1; end;", checker( - block(element(NameReferenceNode.class, "Bar1")).succeedsTo(0), - block(element(NameReferenceNode.class, "Baz1")).succeedsTo(0), + block(element(NameReferenceNode.class, "Bar1")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Baz1")).succeedsTo(EXIT_ID), block( element(NameReferenceNode.class, "Foo"), element(IntegerLiteralNode.class, "1"), @@ -427,7 +429,7 @@ void testCaseStatementRangeArm() { element(IntegerLiteralNode.class, "3"), element(IntegerLiteralNode.class, "4")) .withTerminator(CaseStatementNode.class) - .succeedsToCases(0, 2, 3))); + .succeedsToCases(EXIT_ID, 2, 3))); } @Test @@ -435,9 +437,9 @@ void testCaseStatementElse() { test( "case Foo of Bar: Bar1; Baz: Baz1; else Flarp; end;", checker( - block(element(NameReferenceNode.class, "Bar1")).succeedsTo(0), - block(element(NameReferenceNode.class, "Baz1")).succeedsTo(0), - block(element(NameReferenceNode.class, "Flarp")).succeedsTo(0), + block(element(NameReferenceNode.class, "Bar1")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Baz1")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Flarp")).succeedsTo(EXIT_ID), block( element(NameReferenceNode.class, "Foo"), element(NameReferenceNode.class, "Bar"), @@ -454,7 +456,7 @@ void testEmptyCase() { block(element(NameReferenceNode.class, "S")) .withTerminator(CaseStatementNode.class) .succeedsToCases(1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -465,7 +467,7 @@ void testEmptyCaseElse() { block(element(NameReferenceNode.class, "S")) .withTerminator(CaseStatementNode.class) .succeedsToCases(1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -479,7 +481,7 @@ void testEmptyCaseArm() { element(NameReferenceNode.class, "S2")) .withTerminator(CaseStatementNode.class) .succeedsToCases(1, 1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -489,7 +491,7 @@ void testRepeat() { checker( block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), block(element(NameReferenceNode.class, "Foo")) - .branchesTo(0, 2) + .branchesTo(EXIT_ID, 2) .withTerminator(RepeatStatementNode.class))); } @@ -501,7 +503,7 @@ void testRepeatContinue() { terminator(StatementTerminator.CONTINUE).jumpsTo(1, 2), block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), block(element(NameReferenceNode.class, "Foo")) - .branchesTo(0, 3) + .branchesTo(EXIT_ID, 3) .withTerminator(RepeatStatementNode.class))); } @@ -511,10 +513,10 @@ void testRepeatBreak() { "repeat Bar; break; until Foo;", checker( block(element(NameReferenceNode.class, "Bar")) - .jumpsTo(0, 1) + .jumpsTo(EXIT_ID, 1) .withTerminator(StatementTerminator.BREAK), block(element(NameReferenceNode.class, "Foo")) - .branchesTo(0, 2) + .branchesTo(EXIT_ID, 2) .withTerminator(RepeatStatementNode.class))); } @@ -526,7 +528,7 @@ void testEmptyRepeat() { block(element(NameReferenceNode.class, "C")) .branchesTo(1, 2) .withTerminator(RepeatStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -535,7 +537,7 @@ void testWhile() { "while Foo do Bar;", checker( block(element(NameReferenceNode.class, "Foo")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(WhileStatementNode.class), block(element(NameReferenceNode.class, "Bar")).succeedsTo(2))); } @@ -546,7 +548,7 @@ void testWhileContinue() { "while Foo do begin Continue; Bar; end;", checker( block(element(NameReferenceNode.class, "Foo")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(WhileStatementNode.class), terminator(StatementTerminator.CONTINUE).jumpsTo(3, 1), block(element(NameReferenceNode.class, "Bar")).succeedsTo(3))); @@ -558,10 +560,10 @@ void testWhileBreak() { "while Foo do begin Bar; break; end;", checker( block(element(NameReferenceNode.class, "Foo")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(WhileStatementNode.class), block(element(NameReferenceNode.class, "Bar")) - .jumpsTo(0, 2) + .jumpsTo(EXIT_ID, 2) .withTerminator(StatementTerminator.BREAK))); } @@ -573,7 +575,7 @@ void testEmptyWhile() { block(element(NameReferenceNode.class, "C")) .branchesTo(2, 1) .withTerminator(WhileStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -585,7 +587,7 @@ void testForToVarDecl() { block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), block(element(NameReferenceNode.class, "Baz")).succeedsTo(1), block(element(NameDeclarationNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -598,7 +600,7 @@ void testForToVarRef() { block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), block(element(NameReferenceNode.class, "Baz")).succeedsTo(1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -609,9 +611,9 @@ void testForToBreak() { checker( block(element(NameReferenceNode.class, "Foo")).succeedsTo(3), block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), - terminator(StatementTerminator.BREAK).jumpsTo(0, 1), + terminator(StatementTerminator.BREAK).jumpsTo(EXIT_ID, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -629,9 +631,9 @@ void testForToConditionalBreak() { .withCheck(binaryOpTest(BinaryOperator.EQUAL))) .branchesTo(2, 1) .withTerminator(IfStatementNode.class), - terminator(StatementTerminator.BREAK).jumpsTo(0, 1), + terminator(StatementTerminator.BREAK).jumpsTo(EXIT_ID, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -644,7 +646,7 @@ void testForToContinue() { block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -664,7 +666,7 @@ void testForToConditionalContinue() { .withTerminator(IfStatementNode.class), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -678,7 +680,7 @@ void testEmptyForTo() { block(element(NameReferenceNode.class, "I")) .branchesTo(2, 1) .withTerminator(ForToStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -689,7 +691,7 @@ void testForInVarDecl() { block(element(NameReferenceNode.class, "List")).succeedsTo(1), block(element(NameReferenceNode.class, "Foo")).succeedsTo(1), block(element(NameDeclarationNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -701,7 +703,7 @@ void testForInVarRef() { block(element(NameReferenceNode.class, "List")).succeedsTo(1), block(element(NameReferenceNode.class, "Foo")).succeedsTo(1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -711,9 +713,9 @@ void testForInBreak() { "for I in List do Break;", checker( block(element(NameReferenceNode.class, "List")).succeedsTo(1), - terminator(StatementTerminator.BREAK).jumpsTo(0, 1), + terminator(StatementTerminator.BREAK).jumpsTo(EXIT_ID, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -730,9 +732,9 @@ void testForInConditionalBreak() { .withCheck(binaryOpTest(BinaryOperator.EQUAL))) .branchesTo(2, 1) .withTerminator(IfStatementNode.class), - terminator(StatementTerminator.BREAK).jumpsTo(0, 1), + terminator(StatementTerminator.BREAK).jumpsTo(EXIT_ID, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -744,7 +746,7 @@ void testForInContinue() { block(element(NameReferenceNode.class, "List")).succeedsTo(1), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -763,7 +765,7 @@ void testForInConditionalContinue() { .withTerminator(IfStatementNode.class), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -776,7 +778,7 @@ void testEmptyForIn() { block(element(NameReferenceNode.class, "I")) .branchesTo(2, 1) .withTerminator(ForInStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @ParameterizedTest @@ -845,7 +847,7 @@ void testWith() { "with TObject.Create do Foo;", checker( block(element(NameReferenceNode.class, "TObject.Create")).succeedsTo(1), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -854,7 +856,7 @@ void testEmptyWith() { "with S do; A;", checker( block(element(NameReferenceNode.class, "S")).succeedsTo(1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -867,7 +869,7 @@ void testTryFinallyNoRaise() { block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXIT_ID))); } @Test @@ -881,7 +883,7 @@ void testTryFinallyRaise() { .jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXIT_ID))); } @Test @@ -893,7 +895,7 @@ void testTryFinallyExit() { terminator(StatementTerminator.EXIT).jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXIT_ID))); } @Test @@ -902,13 +904,13 @@ void testTryFinallyBreak() { "while True do try Break finally Bar end;", checker( block(element(NameReferenceNode.class, "True")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(WhileStatementNode.class), block(element(TryStatementNode.class)).succeedsTo(2), terminator(StatementTerminator.BREAK).jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(4, 0))); + .succeedsToWithExit(4, EXIT_ID))); } @Test @@ -917,13 +919,13 @@ void testTryFinallyContinue() { "while True do try Continue finally Bar end;", checker( block(element(NameReferenceNode.class, "True")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(WhileStatementNode.class), block(element(TryStatementNode.class)).succeedsTo(2), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(4, 0))); + .succeedsToWithExit(4, EXIT_ID))); } @Test @@ -935,7 +937,7 @@ void testTryFinallyHalt() { terminator(StatementTerminator.HALT).isSink(), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXIT_ID))); } @Test @@ -955,9 +957,9 @@ void testTryContinueFinallyInLoop() { terminator(StatementTerminator.CONTINUE).jumpsTo(2, 2), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(1, 0), + .succeedsToWithExit(1, EXIT_ID), block(element(NameDeclarationNode.class, "A")) - .branchesTo(4, 0) + .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -978,9 +980,9 @@ void testTryBreakFinallyInLoop() { terminator(StatementTerminator.BREAK).jumpsTo(2, 2), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(1, 0), + .succeedsToWithExit(1, EXIT_ID), block(element(NameDeclarationNode.class, "A")) - .branchesTo(4, 0) + .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -1000,10 +1002,10 @@ void testTryExitFinallyInLoop() { block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.EXIT).jumpsTo(2, 2), block(element(NameReferenceNode.class, "Bar")) - .succeedsToWithExit(1, 0) + .succeedsToWithExit(1, EXIT_ID) .withTerminator(FinallyBlockNode.class), block(element(NameDeclarationNode.class, "A")) - .branchesTo(4, 0) + .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -1023,10 +1025,10 @@ void testTryHaltFinallyInLoop() { block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.HALT).isSink(), block(element(NameReferenceNode.class, "Bar")) - .succeedsToWithExit(1, 0) + .succeedsToWithExit(1, EXIT_ID) .withTerminator(FinallyBlockNode.class), block(element(NameDeclarationNode.class, "A")) - .branchesTo(4, 0) + .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -1040,7 +1042,7 @@ void testTryExceptNoRaise() { block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXIT_ID))); } @Test @@ -1050,8 +1052,8 @@ void testTryBareExceptNoRaise() { "try Foo; except Bar; end;", checker( block(element(TryStatementNode.class)).succeedsTo(2), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(0, 1), - block(element(NameReferenceNode.class, "Bar")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(EXIT_ID, 1), + block(element(NameReferenceNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -1066,11 +1068,11 @@ void testTryExceptOnNoRaise() { + "end;", checker( block(element(TryStatementNode.class)).succeedsTo(3), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(0, 0, 1, 2), + block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(EXIT_ID, EXIT_ID, 1, 2), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0), + .succeedsTo(EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1093,12 +1095,12 @@ void testTryExceptRaise1stCatch() { block( element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "EAbort.Create")) - .succeedsToWithExceptions(3, 0, 1, 2), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(2, 0), + .succeedsToWithExceptions(3, EXIT_ID, 1, 2), + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(2, EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0), + .succeedsTo(EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1121,12 +1123,12 @@ void testTryExceptRaise2ndCatch() { block( element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "Exception.Create")) - .succeedsToWithExceptions(3, 0, 1, 2), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, 0), + .succeedsToWithExceptions(3, EXIT_ID, 1, 2), + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0), + .succeedsTo(EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1140,8 +1142,8 @@ void testTryBareExceptRaise() { element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "Exception.Create")) .succeedsToWithExceptions(2, 1), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, 0), - block(element(NameReferenceNode.class, "Bar")).succeedsTo(0))); + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, EXIT_ID), + block(element(NameReferenceNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -1165,12 +1167,12 @@ void testTryExceptElseRaise() { block(element(TryStatementNode.class)).succeedsTo(5), block(element(NameReferenceNode.class, "TObject.Create")) .succeedsToWithExceptions(4, 1, 2, 3), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, 0), + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0), + .succeedsTo(EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0), - block(element(NameReferenceNode.class, "Flarp")).succeedsTo(0))); + .succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Flarp")).succeedsTo(EXIT_ID))); } @Test @@ -1199,7 +1201,7 @@ void testNestedTryExceptFinally() { .succeedsToWithExceptions(1, 1), block(element(NameReferenceNode.class, "Baz")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXIT_ID))); } @Test @@ -1216,11 +1218,11 @@ void testNestedTryFinallyExcept() { block(element(TryStatementNode.class)).succeedsTo(6), block(element(TryStatementNode.class)).succeedsTo(5), block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(4, 4), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExceptions(3, 0, 1), - terminator(FinallyBlockNode.class).succeedsToWithExit(2, 0), - block().succeedsToWithExceptions(0, 0, 1), + block(element(NameReferenceNode.class, "Bar")).succeedsToWithExceptions(3, EXIT_ID, 1), + terminator(FinallyBlockNode.class).succeedsToWithExit(2, EXIT_ID), + block().succeedsToWithExceptions(EXIT_ID, EXIT_ID, 1), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1234,8 +1236,8 @@ void testTryBareExcept() { element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "Exception.Create")) .succeedsToWithExceptions(2, 1), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, 0), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, EXIT_ID), + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -1245,8 +1247,8 @@ void testTryBareExceptReRaise() { "try Foo except raise; end;", checker( block(element(TryStatementNode.class)).succeedsTo(2), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(0, 1), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).throwsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(EXIT_ID, 1), + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).throwsTo(EXIT_ID))); } @Test @@ -1256,10 +1258,10 @@ void testTryExceptReRaise() { "try Foo except on E: Exception do raise; end;", checker( block(element(TryStatementNode.class)).succeedsTo(2), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(0, 0, 1), + block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(EXIT_ID, EXIT_ID, 1), block(element(NameDeclarationNode.class, "E")) .withTerminator(RaiseStatementNode.class, TerminatorKind.RAISE) - .throwsTo(0))); + .throwsTo(EXIT_ID))); } @Test @@ -1267,7 +1269,7 @@ void testNoreturnRoutineInvocation() { test( Map.of("", List.of("procedure Boom; noreturn; begin end")), "Boom;", - checker(terminator(NameReferenceNode.class, TerminatorKind.HALT).throwsTo(0))); + checker(terminator(NameReferenceNode.class, TerminatorKind.HALT).throwsTo(EXIT_ID))); } @Test @@ -1278,7 +1280,7 @@ void testTryExceptNoreturnRoutineInvocation() { checker( block(element(TryStatementNode.class)).succeedsTo(2), terminator(NameReferenceNode.class, TerminatorKind.HALT).throwsTo(1), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -1287,7 +1289,7 @@ void testEmptyTryExcept() { "try except end; A", checker( block(element(TryStatementNode.class)).succeedsTo(1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -1296,8 +1298,8 @@ void testEmptyTryFinally() { "try finally end; A", checker( block(element(TryStatementNode.class)).succeedsTo(2), - terminator(FinallyBlockNode.class).succeedsToWithExit(1, 0), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + terminator(FinallyBlockNode.class).succeedsToWithExit(1, EXIT_ID), + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -1306,7 +1308,7 @@ void testRaiseOutsideTry() { "raise A;", checker( block(element(NameReferenceNode.class, "A")) - .jumpsTo(0, 0) + .jumpsTo(EXIT_ID, EXIT_ID) .withTerminator(RaiseStatementNode.class, TerminatorKind.RAISE))); } @@ -1316,7 +1318,7 @@ void testCompoundStatement() { "begin Foo; end; begin Bar; end;", checker( block(element(NameReferenceNode.class, "Foo"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1329,7 +1331,7 @@ void testAssignmentAnd() { .branchesTo(2, 1) .withTerminator(BinaryExpressionNode.class), block(element(NameReferenceNode.class, "B")).succeedsTo(1), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -1343,7 +1345,7 @@ void testAssignmentPlus() { element(RealLiteralNode.class, "1.1"), element(BinaryExpressionNode.class), element(NameReferenceNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1363,7 +1365,7 @@ void testAssignmentMultipleBinary() { .withCheck(binaryOpTest(BinaryOperator.MULTIPLY)), element(BinaryExpressionNode.class).withCheck(binaryOpTest(BinaryOperator.ADD)), element(NameReferenceNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1378,7 +1380,7 @@ void testArrayConstructor() { element(IntegerLiteralNode.class, "4"), element(IntegerLiteralNode.class, "5"), element(NameReferenceNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1392,9 +1394,9 @@ void testExitStatement() { element(NilLiteralNode.class), element(BinaryExpressionNode.class) .withCheck(binaryOpTest(BinaryOperator.EQUAL))) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - terminator(StatementTerminator.EXIT).jumpsTo(0, 0))); + terminator(StatementTerminator.EXIT).jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1408,11 +1410,11 @@ void testExitValueStatement() { element(NilLiteralNode.class), element(BinaryExpressionNode.class) .withCheck(binaryOpTest(BinaryOperator.EQUAL))) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), block(element(NameReferenceNode.class, "Bar")) .withTerminator(StatementTerminator.EXIT) - .jumpsTo(0, 0))); + .jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1430,7 +1432,7 @@ void testExitCascadedOr() { .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.OR)) .branchesTo(1, 2), block(element(NameReferenceNode.class, "C")).succeedsTo(1), - terminator(StatementTerminator.EXIT).jumpsTo(0, 0))); + terminator(StatementTerminator.EXIT).jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1448,7 +1450,7 @@ void testExitCascadedAnd() { .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.AND)) .branchesTo(2, 1), block(element(NameReferenceNode.class, "C")).succeedsTo(1), - terminator(StatementTerminator.EXIT).jumpsTo(0, 0))); + terminator(StatementTerminator.EXIT).jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1472,12 +1474,12 @@ void testExitComplexBoolean() { .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.AND)) .branchesTo(2, 1), block(element(NameReferenceNode.class, "B")).succeedsTo(1), - terminator(StatementTerminator.EXIT).jumpsTo(0, 0))); + terminator(StatementTerminator.EXIT).jumpsTo(EXIT_ID, EXIT_ID))); } @Test void testBareInherited() { - test("inherited;", checker(block(element(CommonDelphiNode.class, "inherited")).succeedsTo(0))); + test("inherited;", checker(block(element(CommonDelphiNode.class, "inherited")).succeedsTo(EXIT_ID))); } @Test @@ -1488,7 +1490,7 @@ void testNamedInheritedNoArgs() { block( element(CommonDelphiNode.class, "inherited"), element(NameReferenceNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1502,7 +1504,7 @@ void testInherited() { element(NameReferenceNode.class, "A"), element(NameReferenceNode.class, "B"), element(NameReferenceNode.class, "C")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1512,9 +1514,9 @@ void testSucceedingLabelGoto() { "if B then goto A; A:", checker( block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - terminator(GotoStatementNode.class, TerminatorKind.GOTO).jumpsTo(0, 0))); + terminator(GotoStatementNode.class, TerminatorKind.GOTO).jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1524,11 +1526,11 @@ void testPrecedingLabelGoto() { "A: if B then goto A;", checker( block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), block(element(NameReferenceNode.class, "A")) .withTerminator(GotoStatementNode.class, TerminatorKind.GOTO) - .jumpsTo(2, 0))); + .jumpsTo(2, EXIT_ID))); } @Test @@ -1539,11 +1541,11 @@ void testLabelSeparatesBlock() { checker( block(element(NameReferenceNode.class, "Foo")).succeedsTo(2), block(element(NameReferenceNode.class, "Bar"), element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), block(element(NameReferenceNode.class, "A")) .withTerminator(GotoStatementNode.class, TerminatorKind.GOTO) - .jumpsTo(2, 0))); + .jumpsTo(2, EXIT_ID))); } @Test @@ -1551,21 +1553,21 @@ void testEmptyLabel() { test( Map.of("label", List.of("A,B")), "A: B: C;", - checker(block(element(NameReferenceNode.class, "C")).succeedsTo(0))); + checker(block(element(NameReferenceNode.class, "C")).succeedsTo(EXIT_ID))); } @Test void testAnonymousRoutinesAreIgnored() { test( "A := procedure begin Foo; Bar; end;", - checker(block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + checker(block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test void testInlineAssemblyIsIgnored() { test( "Foo; asm XOR EAX, EAX end;", - checker(block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + checker(block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -1659,7 +1661,7 @@ void testComplexConstructions() { .succeedsTo(2), block(element(TextLiteralNode.class, "'b'"), element(NameReferenceNode.class, "X")) .succeedsTo(2), - terminator(FinallyBlockNode.class).succeedsToWithExit(12, 0), - block(element(NameReferenceNode.class, "Foo4")).succeedsTo(0))); + terminator(FinallyBlockNode.class).succeedsToWithExit(12, EXIT_ID), + block(element(NameReferenceNode.class, "Foo4")).succeedsTo(EXIT_ID))); } } From f9f7f38b377b2a7c53ec961241f68f2c8be583bd Mon Sep 17 00:00:00 2001 From: Joshua Gardner Date: Mon, 11 May 2026 16:40:08 +1000 Subject: [PATCH 5/7] Replace AssertJ `withFailMessage` with `as` in CFG `BlockChecker` --- .../delphi/cfg/checker/BlockChecker.java | 49 ++++++++----------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/BlockChecker.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/BlockChecker.java index 48c9b7d0b..102fd2243 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/BlockChecker.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/BlockChecker.java @@ -79,7 +79,7 @@ public void check(final Block block) { .check(block.getElements().get(elementId)); } assertThat(successorChecker) - .withFailMessage("%s should have its successors declared", getBlockDisplay(block)) + .as("%s should have its successors declared", getBlockDisplay(block)) .isNotNull(); successorChecker.check(block); @@ -87,7 +87,7 @@ public void check(final Block block) { terminatorChecker.check(block); } else { assertThat(block) - .withFailMessage("%s should have its terminator specified", getBlockDisplay(block)) + .as("%s should have its terminator specified", getBlockDisplay(block)) .isNotInstanceOf(Terminated.class); } terminatorNodeChecks.forEach(check -> check.check(block)); @@ -99,8 +99,7 @@ public BlockChecker succeedsTo(int successor) { block -> { Linear branch = assertBlockIsType(block, Linear.class); assertThat(getBlockId(branch.getSuccessor())) - .withFailMessage( - getBlockDisplay(block) + " is expected to have successor of B" + successor) + .as(getBlockDisplay(block) + " is expected to have successor of B" + successor) .isEqualTo(successor); }); return this; @@ -112,11 +111,10 @@ public BlockChecker succeedsToWithExit(int successor, int exitSuccessor) { block -> { Finally branch = assertBlockIsType(block, Finally.class); assertThat(getBlockId(branch.getSuccessor())) - .withFailMessage( - getBlockDisplay(block) + " is expected to have successor of B" + successor) + .as(getBlockDisplay(block) + " is expected to have successor of B" + successor) .isEqualTo(successor); assertThat(getBlockId(branch.getExceptionSuccessor())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have exit successor of B" + exitSuccessor) @@ -131,13 +129,13 @@ public BlockChecker branchesTo(int trueBlock, int falseBlock) { block -> { Branch branch = assertBlockIsType(block, Branch.class); assertThat(getBlockId(branch.getTrueBlock())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have true successor of B" + trueBlock) .isEqualTo(trueBlock); assertThat(getBlockId(branch.getFalseBlock())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have false successor of B" + falseBlock) @@ -152,11 +150,10 @@ public BlockChecker jumpsTo(int successor, int successorWithoutJump) { block -> { UnconditionalJump branch = assertBlockIsType(block, UnconditionalJump.class); assertThat(getBlockId(branch.getSuccessor())) - .withFailMessage( - getBlockDisplay(block) + " is expected to have successor of B" + successor) + .as(getBlockDisplay(block) + " is expected to have successor of B" + successor) .isEqualTo(successor); assertThat(getBlockId(branch.getSuccessorIfRemoved())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have successor without jump of B" + successorWithoutJump) @@ -176,7 +173,7 @@ public BlockChecker succeedsToCases(int fallthrough, int... cases) { .map(BlockChecker::getBlockId) .collect(Collectors.toSet()); assertThat(caseIds) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have case successors of [" + expectedCases.stream() @@ -185,7 +182,7 @@ public BlockChecker succeedsToCases(int fallthrough, int... cases) { + "]") .containsExactlyInAnyOrderElementsOf(expectedCases); assertThat(getBlockId(caseSuccessor.getFallthroughSuccessor())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have fallthrough successor of B" + fallthrough) @@ -201,15 +198,14 @@ public BlockChecker succeedsToWithExceptions(int successor, int... unknownExcept block -> { PossibleException branch = assertBlockIsType(block, PossibleException.class); assertThat(getBlockId(branch.getSuccessor())) - .withFailMessage( - getBlockDisplay(block) + " is expected to have successor of B" + successor) + .as(getBlockDisplay(block) + " is expected to have successor of B" + successor) .isEqualTo(successor); Set blockIds = branch.getExceptions().stream() .map(BlockChecker::getBlockId) .collect(Collectors.toSet()); assertThat(blockIds) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have exception successors of [" + exceptions.stream() @@ -232,7 +228,7 @@ public BlockChecker throwsTo(int... unknownExceptions) { .map(BlockChecker::getBlockId) .collect(Collectors.toSet()); assertThat(blockIds) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have exception successors of [" + exceptions.stream() @@ -264,26 +260,24 @@ public BlockChecker withTerminator(StatementTerminator terminator) { block -> { Terminated terminated = assertBlockTerminated(block); assertThat(terminated.getTerminatorKind()) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to be terminated with kind " + terminator.getTerminatorKind()) .isEqualTo(terminator.getTerminatorKind()); assertThat(terminated.getTerminator()) - .withFailMessage( - getBlockDisplay(block) + " is expected to be terminated with name reference") + .as(getBlockDisplay(block) + " is expected to be terminated with name reference") .isInstanceOf(NameReferenceNode.class); NameReferenceNode nameReferenceNode = (NameReferenceNode) terminated.getTerminator(); assertThat(nameReferenceNode.getLastName().getNameDeclaration()) - .withFailMessage( - getBlockDisplay(block) + " is expected to be terminated with routine") + .as(getBlockDisplay(block) + " is expected to be terminated with routine") .isInstanceOf(RoutineNameDeclaration.class); assertThat( ((RoutineNameDeclaration) nameReferenceNode.getLastName().getNameDeclaration()) .fullyQualifiedName()) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to be terminated with " + terminator.getRoutineName()) @@ -299,14 +293,13 @@ public BlockChecker withTerminator( block -> { Terminated terminated = assertBlockTerminated(block); assertThat(terminated.getTerminator()) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to be terminated with " + terminatorClass.getTypeName()) .isInstanceOf(terminatorClass); assertThat(terminated.getTerminatorKind()) - .withFailMessage( - getBlockDisplay(block) + " is expected to be terminated with kind " + kind) + .as(getBlockDisplay(block) + " is expected to be terminated with kind " + kind) .isEqualTo(kind); }); return this; @@ -324,7 +317,7 @@ public BlockChecker withTerminatorNodeCheck(Consumer extraChecker) { private Terminated assertBlockTerminated(Block block) { assertThat(block) - .withFailMessage(getBlockDisplay(block) + " is expected to be terminated") + .as(getBlockDisplay(block) + " is expected to be terminated") .isInstanceOf(Terminated.class); return (Terminated) block; } From 5eed1e6c71dad9757f42199e3b18cc78c5a53a49 Mon Sep 17 00:00:00 2001 From: Joshua Gardner Date: Fri, 8 May 2026 15:49:54 +1000 Subject: [PATCH 6/7] Add `ExceptionalRoutineExit` to appropriate `ControlFlowGraph`s --- .../delphi/checks/RedundantJumpCheck.java | 8 ++- .../delphi/cfg/ControlFlowGraphBuilder.java | 24 ++++++-- .../delphi/cfg/ControlFlowGraphImpl.java | 15 +++++ .../delphi/cfg/ControlFlowGraphVisitor.java | 4 +- .../cfg/api/ExceptionalRoutineExit.java | 22 +++++++ .../delphi/cfg/block/ProtoBlockFactory.java | 28 +++++++++ .../delphi/cfg/ControlFlowGraphTest.java | 60 +++++++++++-------- .../delphi/cfg/checker/GraphChecker.java | 21 +++++-- 8 files changed, 144 insertions(+), 38 deletions(-) create mode 100644 delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ExceptionalRoutineExit.java diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/RedundantJumpCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/RedundantJumpCheck.java index d564ed4b1..f11c2f95b 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/RedundantJumpCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/RedundantJumpCheck.java @@ -20,7 +20,9 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; import au.com.integradev.delphi.cfg.api.Finally; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.api.Terminated; import au.com.integradev.delphi.cfg.api.UnconditionalJump; import au.com.integradev.delphi.utils.ControlFlowGraphUtils; @@ -134,7 +136,11 @@ private boolean isViolation(ControlFlowGraph cfg) { } if (!finallyBlock.getSuccessor().equals(finallyBlock.getExceptionSuccessor())) { // multiple paths after the finally corresponds to code that would be skipped from the jump - return false; + if (!(finallyBlock.getSuccessor() instanceof RoutineExit + && finallyBlock.getExceptionSuccessor() instanceof ExceptionalRoutineExit)) { + // if both of the paths are routine exits, then they are effectively equivalent + return false; + } } } // if no invalidating try-finally blocks are found, the use is a violation diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java index afb1c1982..b2d1a3352 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java @@ -20,6 +20,7 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.block.BlockImpl; import au.com.integradev.delphi.cfg.block.ProtoBlock; @@ -48,6 +49,7 @@ public class ControlFlowGraphBuilder { private final List blocks = new ArrayList<>(); private final Deque exitBlocks = new ArrayDeque<>(); + private final Deque exceptionalExitBlocks = new ArrayDeque<>(); private final Deque breakTargets = new ArrayDeque<>(); private final Deque continueTargets = new ArrayDeque<>(); private final Map labelTargets = new HashMap<>(); @@ -58,9 +60,14 @@ public class ControlFlowGraphBuilder { @SuppressWarnings("this-escape") public ControlFlowGraphBuilder() { + ProtoBlock exceptionalExitBlock = ProtoBlockFactory.exceptionalExitBlock(); + addBlock(exceptionalExitBlock); + exceptionalExitBlocks.add(exceptionalExitBlock); + ProtoBlock exitBlock = ProtoBlockFactory.exitBlock(); addBlock(exitBlock); exitBlocks.add(exitBlock); + addBlockBefore(exitBlock); } @@ -94,10 +101,13 @@ private static void populatePredecessors(ControlFlowGraph cfg) { private static void populateIds(ControlFlowGraph cfg) { List blocks = Lists.reverse(cfg.getBlocks()); int blockId = 0; - for(Block block : blocks) { + for (Block block : blocks) { if (block instanceof RoutineExit) { // Ensure `RoutineExit` is always id 0 ((BlockImpl) block).setId(0); + } else if (block instanceof ExceptionalRoutineExit) { + // Ensure `ExceptionalRoutineExit` is always id -1 + ((BlockImpl) block).setId(-1); } else { ((BlockImpl) block).setId(++blockId); } @@ -108,6 +118,10 @@ public ProtoBlock getExitBlock() { return exitBlocks.peek(); } + public ProtoBlock getExceptionalExitBlock() { + return exceptionalExitBlocks.peek(); + } + public ProtoBlock getBreakTarget() { return breakTargets.peek(); } @@ -132,10 +146,12 @@ public void popLoopContext() { public void pushExitBlock(ProtoBlock target) { exitBlocks.push(target); + exceptionalExitBlocks.push(target); } public void popExitBlock() { exitBlocks.pop(); + exceptionalExitBlocks.pop(); } private static class UnresolvedLabel { @@ -203,7 +219,7 @@ public boolean inTryContext() { public ProtoBlock getCatchTarget(Type exceptionType) { if (tryContexts.isEmpty()) { - return getExitBlock(); + return getExceptionalExitBlock(); } TryContext tryContext = tryContexts.peek(); return tryContext.catches.keySet().stream() @@ -211,7 +227,7 @@ public ProtoBlock getCatchTarget(Type exceptionType) { .findFirst() .map(tryContext.catches::get) .or(() -> Optional.ofNullable(tryContext.elseBlock)) - .orElse(getExitBlock()); + .orElse(getExceptionalExitBlock()); } private static boolean isCompatibleType(Type exceptionType, Type catchType) { @@ -224,7 +240,7 @@ public Set getAllCatchTargets() { } TryContext context = tryContexts.peek(); Stream elseOrExit = - Stream.of(Optional.ofNullable(context.elseBlock).orElse(getExitBlock())); + Stream.of(Optional.ofNullable(context.elseBlock).orElse(getExceptionalExitBlock())); return Stream.concat(context.catches.values().stream(), elseOrExit) .filter(Objects::nonNull) .collect(Collectors.toSet()); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java index af20dd4c8..c00ccdd9e 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java @@ -20,6 +20,7 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; import au.com.integradev.delphi.cfg.api.Terminated; import au.com.integradev.delphi.cfg.api.UnconditionalJump; import au.com.integradev.delphi.cfg.block.BlockImpl; @@ -68,6 +69,20 @@ public void prune() { blocks.forEach(inactiveBlocks::remove); } while (!inactiveBlocks.isEmpty()); + + // Remove the exceptional exit if there are no paths to it + Block exceptionalExitBlock = + Lists.reverse(blocks).stream() + .filter(ExceptionalRoutineExit.class::isInstance) + .findFirst() + .orElseThrow( + () -> new IllegalStateException("Couldn't find the exceptional exit block")); + + if (blocks.stream() + .flatMap(block -> block.getSuccessors().stream()) + .noneMatch(successor -> successor.equals(exceptionalExitBlock))) { + blocks.remove(exceptionalExitBlock); + } } private boolean isInactive(Block block) { diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java index c6b077182..bf8240419 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java @@ -208,7 +208,7 @@ private static void handleExceptionalPaths(ControlFlowGraphBuilder builder) { private static Set getUnknownExceptionTargets(ControlFlowGraphBuilder builder) { Set exceptions = builder.getAllCatchTargets(); if (exceptions.isEmpty()) { - return Set.of(builder.getExitBlock()); + return Set.of(builder.getExceptionalExitBlock()); } return exceptions; } @@ -548,7 +548,7 @@ private ControlFlowGraphBuilder buildTryFinally( FinallyBlockNode finallyNode = node.getFinallyBlock(); builder.addBlock( ProtoBlockFactory.finallyBlock( - node.getFinallyBlock(), builder.getCurrentBlock(), builder.getExitBlock())); + node.getFinallyBlock(), builder.getCurrentBlock(), builder.getExceptionalExitBlock())); build(finallyNode.getStatementList(), builder); boolean inLoop = builder.isInLoop(); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ExceptionalRoutineExit.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ExceptionalRoutineExit.java new file mode 100644 index 000000000..e1c2c8a28 --- /dev/null +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ExceptionalRoutineExit.java @@ -0,0 +1,22 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2026 Integrated Application Development + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package au.com.integradev.delphi.cfg.api; + +/** A block representing an exceptional end of a routine, e.g., an uncaught exception */ +public interface ExceptionalRoutineExit extends Terminus {} diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java index 964299e16..b1847b61b 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java @@ -21,6 +21,7 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.Branch; import au.com.integradev.delphi.cfg.api.Cases; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; import au.com.integradev.delphi.cfg.api.Finally; import au.com.integradev.delphi.cfg.api.Halt; import au.com.integradev.delphi.cfg.api.Linear; @@ -45,6 +46,10 @@ public static ProtoBlock exitBlock() { return new ProtoBlock(RoutineExitImpl::new, (blocks, block) -> {}); } + public static ProtoBlock exceptionalExitBlock() { + return new ProtoBlock(ExceptionalRoutineExitImpl::new, (blocks, block) -> {}); + } + public static ProtoBlock halt(DelphiNode terminator) { return new ProtoBlock(HaltImpl::new, (blocks, block) -> ((HaltImpl) block).setData(terminator)); } @@ -464,6 +469,29 @@ public String getBlockType() { } } + private static class ExceptionalRoutineExitImpl extends BlockImpl + implements ExceptionalRoutineExit { + + public ExceptionalRoutineExitImpl(List elements) { + super(elements); + } + + @Override + public void replaceInactiveSuccessor(Block inactiveBlock, Block target) { + // Block has no successors + } + + @Override + public String getDescription() { + return String.format("%n\t(ExceptionalExit)"); + } + + @Override + public String getBlockType() { + return "ExceptionalExit"; + } + } + static class BranchImpl extends BlockImpl implements Branch { private Block trueBlock; private Block falseBlock; diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java index 276ac3579..2092ba313 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java @@ -86,6 +86,7 @@ class ControlFlowGraphTest { private static final Logger LOG = LoggerFactory.getLogger(ControlFlowGraphTest.class); private static final int EXIT_ID = 0; + private static final int EXCEPTION_EXIT_ID = -1; private ControlFlowGraph buildCfg(String input) { return buildCfg(Collections.emptyMap(), input); @@ -869,7 +870,7 @@ void testTryFinallyNoRaise() { block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(EXIT_ID, EXIT_ID))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -883,7 +884,7 @@ void testTryFinallyRaise() { .jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(EXIT_ID, EXIT_ID))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -895,7 +896,7 @@ void testTryFinallyExit() { terminator(StatementTerminator.EXIT).jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(EXIT_ID, EXIT_ID))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -910,7 +911,7 @@ void testTryFinallyBreak() { terminator(StatementTerminator.BREAK).jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(4, EXIT_ID))); + .succeedsToWithExit(4, EXCEPTION_EXIT_ID))); } @Test @@ -925,7 +926,7 @@ void testTryFinallyContinue() { terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(4, EXIT_ID))); + .succeedsToWithExit(4, EXCEPTION_EXIT_ID))); } @Test @@ -937,7 +938,7 @@ void testTryFinallyHalt() { terminator(StatementTerminator.HALT).isSink(), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(EXIT_ID, EXIT_ID))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -957,7 +958,7 @@ void testTryContinueFinallyInLoop() { terminator(StatementTerminator.CONTINUE).jumpsTo(2, 2), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(1, EXIT_ID), + .succeedsToWithExit(1, EXCEPTION_EXIT_ID), block(element(NameDeclarationNode.class, "A")) .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); @@ -980,7 +981,7 @@ void testTryBreakFinallyInLoop() { terminator(StatementTerminator.BREAK).jumpsTo(2, 2), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(1, EXIT_ID), + .succeedsToWithExit(1, EXCEPTION_EXIT_ID), block(element(NameDeclarationNode.class, "A")) .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); @@ -1002,7 +1003,7 @@ void testTryExitFinallyInLoop() { block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.EXIT).jumpsTo(2, 2), block(element(NameReferenceNode.class, "Bar")) - .succeedsToWithExit(1, EXIT_ID) + .succeedsToWithExit(1, EXCEPTION_EXIT_ID) .withTerminator(FinallyBlockNode.class), block(element(NameDeclarationNode.class, "A")) .branchesTo(4, EXIT_ID) @@ -1025,7 +1026,7 @@ void testTryHaltFinallyInLoop() { block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.HALT).isSink(), block(element(NameReferenceNode.class, "Bar")) - .succeedsToWithExit(1, EXIT_ID) + .succeedsToWithExit(1, EXCEPTION_EXIT_ID) .withTerminator(FinallyBlockNode.class), block(element(NameDeclarationNode.class, "A")) .branchesTo(4, EXIT_ID) @@ -1042,7 +1043,7 @@ void testTryExceptNoRaise() { block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(EXIT_ID, EXIT_ID))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -1068,7 +1069,8 @@ void testTryExceptOnNoRaise() { + "end;", checker( block(element(TryStatementNode.class)).succeedsTo(3), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(EXIT_ID, EXIT_ID, 1, 2), + block(element(NameReferenceNode.class, "Foo")) + .succeedsToWithExceptions(EXIT_ID, EXCEPTION_EXIT_ID, 1, 2), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) .succeedsTo(EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) @@ -1095,7 +1097,7 @@ void testTryExceptRaise1stCatch() { block( element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "EAbort.Create")) - .succeedsToWithExceptions(3, EXIT_ID, 1, 2), + .succeedsToWithExceptions(3, EXCEPTION_EXIT_ID, 1, 2), terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(2, EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) .succeedsTo(EXIT_ID), @@ -1123,7 +1125,7 @@ void testTryExceptRaise2ndCatch() { block( element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "Exception.Create")) - .succeedsToWithExceptions(3, EXIT_ID, 1, 2), + .succeedsToWithExceptions(3, EXCEPTION_EXIT_ID, 1, 2), terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) .succeedsTo(EXIT_ID), @@ -1201,7 +1203,7 @@ void testNestedTryExceptFinally() { .succeedsToWithExceptions(1, 1), block(element(NameReferenceNode.class, "Baz")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(EXIT_ID, EXIT_ID))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -1218,9 +1220,10 @@ void testNestedTryFinallyExcept() { block(element(TryStatementNode.class)).succeedsTo(6), block(element(TryStatementNode.class)).succeedsTo(5), block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(4, 4), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExceptions(3, EXIT_ID, 1), - terminator(FinallyBlockNode.class).succeedsToWithExit(2, EXIT_ID), - block().succeedsToWithExceptions(EXIT_ID, EXIT_ID, 1), + block(element(NameReferenceNode.class, "Bar")) + .succeedsToWithExceptions(3, EXCEPTION_EXIT_ID, 1), + terminator(FinallyBlockNode.class).succeedsToWithExit(2, EXCEPTION_EXIT_ID), + block().succeedsToWithExceptions(EXIT_ID, EXCEPTION_EXIT_ID, 1), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) .succeedsTo(EXIT_ID))); } @@ -1248,7 +1251,8 @@ void testTryBareExceptReRaise() { checker( block(element(TryStatementNode.class)).succeedsTo(2), block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(EXIT_ID, 1), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).throwsTo(EXIT_ID))); + terminator(RaiseStatementNode.class, TerminatorKind.RAISE) + .throwsTo(EXCEPTION_EXIT_ID))); } @Test @@ -1258,10 +1262,11 @@ void testTryExceptReRaise() { "try Foo except on E: Exception do raise; end;", checker( block(element(TryStatementNode.class)).succeedsTo(2), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(EXIT_ID, EXIT_ID, 1), + block(element(NameReferenceNode.class, "Foo")) + .succeedsToWithExceptions(EXIT_ID, EXCEPTION_EXIT_ID, 1), block(element(NameDeclarationNode.class, "E")) .withTerminator(RaiseStatementNode.class, TerminatorKind.RAISE) - .throwsTo(EXIT_ID))); + .throwsTo(EXCEPTION_EXIT_ID))); } @Test @@ -1269,7 +1274,8 @@ void testNoreturnRoutineInvocation() { test( Map.of("", List.of("procedure Boom; noreturn; begin end")), "Boom;", - checker(terminator(NameReferenceNode.class, TerminatorKind.HALT).throwsTo(EXIT_ID))); + checker( + terminator(NameReferenceNode.class, TerminatorKind.HALT).throwsTo(EXCEPTION_EXIT_ID))); } @Test @@ -1298,7 +1304,7 @@ void testEmptyTryFinally() { "try finally end; A", checker( block(element(TryStatementNode.class)).succeedsTo(2), - terminator(FinallyBlockNode.class).succeedsToWithExit(1, EXIT_ID), + terminator(FinallyBlockNode.class).succeedsToWithExit(1, EXCEPTION_EXIT_ID), block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @@ -1308,7 +1314,7 @@ void testRaiseOutsideTry() { "raise A;", checker( block(element(NameReferenceNode.class, "A")) - .jumpsTo(EXIT_ID, EXIT_ID) + .jumpsTo(EXCEPTION_EXIT_ID, EXIT_ID) .withTerminator(RaiseStatementNode.class, TerminatorKind.RAISE))); } @@ -1479,7 +1485,9 @@ void testExitComplexBoolean() { @Test void testBareInherited() { - test("inherited;", checker(block(element(CommonDelphiNode.class, "inherited")).succeedsTo(EXIT_ID))); + test( + "inherited;", + checker(block(element(CommonDelphiNode.class, "inherited")).succeedsTo(EXIT_ID))); } @Test @@ -1661,7 +1669,7 @@ void testComplexConstructions() { .succeedsTo(2), block(element(TextLiteralNode.class, "'b'"), element(NameReferenceNode.class, "X")) .succeedsTo(2), - terminator(FinallyBlockNode.class).succeedsToWithExit(12, EXIT_ID), + terminator(FinallyBlockNode.class).succeedsToWithExit(12, EXCEPTION_EXIT_ID), block(element(NameReferenceNode.class, "Foo4")).succeedsTo(EXIT_ID))); } } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/GraphChecker.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/GraphChecker.java index b33b303b8..ee7da8b5f 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/GraphChecker.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/GraphChecker.java @@ -22,6 +22,8 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.block.BlockImpl; import java.util.ArrayList; import java.util.Collections; @@ -41,19 +43,28 @@ private GraphChecker(BlockChecker... checkers) { } public void check(final ControlFlowGraph cfg) { - assertThat(cfg.getBlocks()).as("block count").hasSize(checkers.size() + 1); + List blocks = new ArrayList<>(cfg.getBlocks()); + List exitBlocks = new ArrayList<>(); + for (int i = blocks.size() - 1; i >= 0; i--) { + Block block = blocks.get(i); + if (block instanceof RoutineExit || block instanceof ExceptionalRoutineExit) { + exitBlocks.add(block); + blocks.remove(i); + } + } + + assertThat(blocks).as("block count").hasSize(checkers.size()); final Iterator checkerIterator = checkers.iterator(); - List blocks = new ArrayList<>(cfg.getBlocks()); - final Block exitBlock = blocks.remove(blocks.size() - 1); for (Block block : blocks) { checkerIterator.next().check(block); int blockId = ((BlockImpl) block).getId(); checkLinkedBlocks("Successor of B" + blockId, cfg.getBlocks(), block.getSuccessors()); checkLinkedBlocks("Predecessor of B" + blockId, cfg.getBlocks(), block.getPredecessors()); } - assertThat(exitBlock.getElements()).isEmpty(); - assertThat(exitBlock.getSuccessors()).isEmpty(); + + assertThat(exitBlocks.stream().map(Block::getElements)).allMatch(List::isEmpty); + assertThat(exitBlocks.stream().map(Block::getSuccessors)).allMatch(Set::isEmpty); assertThat(cfg.getBlocks()) .withFailMessage("CFG entry block is no longer in the list of blocks!") .contains(cfg.getEntryBlock()); From da15b244ab4c9ce92ceb029559ec57a70bc64dd6 Mon Sep 17 00:00:00 2001 From: Joshua Gardner Date: Tue, 12 May 2026 11:37:11 +1000 Subject: [PATCH 7/7] Improve `NoreturnContractCheck` --- .../com/integradev/delphi/checks/NoreturnContractCheck.java | 3 ++- .../integradev/delphi/checks/NoreturnContractCheckTest.java | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java index 9564588cf..c851e73f8 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java @@ -20,6 +20,7 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; import au.com.integradev.delphi.cfg.api.Finally; import au.com.integradev.delphi.cfg.api.PossibleException; import au.com.integradev.delphi.cfg.api.RoutineExit; @@ -87,6 +88,6 @@ private static boolean isGuaranteedException(Block block) { } private static boolean notFinallyOrExit(Block block) { - return !(block instanceof Finally || block instanceof RoutineExit); + return !(block instanceof Finally || block instanceof ExceptionalRoutineExit); } } diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/NoreturnContractCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/NoreturnContractCheckTest.java index 8ce3da7a2..18b125b37 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/NoreturnContractCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/NoreturnContractCheckTest.java @@ -341,7 +341,7 @@ void testNoreturnRoutineWithRaiseToIrrelevantExceptShouldAddIssue() { } @Test - void testFailOnUpgradeNoreturnRoutineWithRaiseToSpecificEmptyExceptShouldNotAddIssue() { + void testNoreturnRoutineWithRaiseToSpecificEmptyExceptShouldAddIssue() { CheckVerifier.newVerifier() .withCheck(new NoreturnContractCheck()) .onFile( @@ -349,7 +349,7 @@ void testFailOnUpgradeNoreturnRoutineWithRaiseToSpecificEmptyExceptShouldNotAddI .appendImpl("uses") .appendImpl(" System.SysUtils;") .appendImpl("") - .appendImpl("procedure RaiseToSpecificExcept; noreturn;") + .appendImpl("procedure RaiseToSpecificExcept; noreturn; // Noncompliant") .appendImpl("begin") .appendImpl(" try") .appendImpl(" raise Exception.Create('Error');") @@ -359,6 +359,6 @@ void testFailOnUpgradeNoreturnRoutineWithRaiseToSpecificEmptyExceptShouldNotAddI .appendImpl(" end;") .appendImpl(" end;") .appendImpl("end;")) - .verifyNoIssues(); + .verifyIssues(); } }