[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