[code-reflection] RFR: Disallow insertion of a root op in a block [v9]

Paul Sandoz psandoz at openjdk.org
Fri Aug 22 20:19:07 UTC 2025


On Fri, 22 Aug 2025 11:11:43 GMT, Mourad Abbay <mabbay at openjdk.org> wrote:

>> A root operation shouldn't be inserted in a block. This changes enforce this rule.
>
> Mourad Abbay has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Apply suggestions

The naming update looks good. We need to test this carefully before integrating.

Noting down some terminology and behaviour. Operations have the following states:
- unbound. The operation has no result, and therefore it has no parent (block).
- bound. Appending an unbound operation to a block builder results in the operation being bound. The bound operation is assigned an operation result and has a parent block. The parent's block is only accessible once the block's parent body is built. 
- sealed. An unbound operation can be sealed. The operation has no result, and therefore no parent (block).

Neither a bound or sealed operation can unbound. Appending a bound or sealed operation to a block builder results in it being transformed to a new unbound operation that is then appended.

src/jdk.incubator.code/share/classes/jdk/incubator/code/Op.java line 173:

> 171:          * If assigned to an operation result, it indicates the operation is sealed
> 172:         */
> 173:         private static final Result SEALED_OPR = new Result();

Suggestion:

        private static final Result SEALED_RESULT = new Result();

src/jdk.incubator.code/share/classes/jdk/incubator/code/Op.java line 546:

> 544:     }
> 545: 
> 546:     public void seal() {

Document this method and `isSealed`.

src/jdk.incubator.code/share/classes/jdk/incubator/code/Op.java line 551:

> 549:         }
> 550:         if (result != null) {
> 551:             throw new IllegalStateException("Can't freeze a bound operation");

Suggestion:

            throw new IllegalStateException("Operation cannot be sealed since it bound to a parent block");

src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/ReflectMethods.java line 2779:

> 2777:             Assert.check(funcOp.body().blocks().size() == 1);
> 2778: 
> 2779:             Set<MethodRef> mRefs = new HashSet<>(List.of(M_BLOCK_BUILDER_OP, M_BLOCK_BUILDER_PARAM, M_OP_SEAL));

This can be a static final field. See `Set.of`.

-------------

PR Review: https://git.openjdk.org/babylon/pull/425#pullrequestreview-3145746932
PR Review Comment: https://git.openjdk.org/babylon/pull/425#discussion_r2294601003
PR Review Comment: https://git.openjdk.org/babylon/pull/425#discussion_r2294639430
PR Review Comment: https://git.openjdk.org/babylon/pull/425#discussion_r2294603709
PR Review Comment: https://git.openjdk.org/babylon/pull/425#discussion_r2294636191


More information about the babylon-dev mailing list