[code-reflection] RFR: Lower StringTemplateOp [v5]

Paul Sandoz psandoz at openjdk.org
Tue Mar 19 16:44:28 UTC 2024


On Tue, 19 Mar 2024 16:16:00 GMT, Mourad Abbay <mabbay at openjdk.org> wrote:

>> The lowering of the StringTemplateOp. We chose not to have a block for every expression body, rather blocks will be created when necessary.
>
> Mourad Abbay has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Apply recommendations

Looks good. You can avoid the awkward capture, and also lean into the naming, current, next, and last (see comments). Update and integrate, no need for another review round.

src/java.base/share/classes/java/lang/reflect/code/op/ExtendedOps.java line 2819:

> 2817:             for (int i = 0; i < expressions().size(); i++) {
> 2818:                 Block.Builder bb = builders.get(i);
> 2819:                 int ci = i;

Suggestion:

                Block.Builder current = builders.get(i);
                Block.Builder next = builders.get(i + 1);

src/java.base/share/classes/java/lang/reflect/code/op/ExtendedOps.java line 2823:

> 2821:                     if (op instanceof YieldOp yop) {
> 2822:                         expressions.add(block.context().getValue(yop.yieldValue()));
> 2823:                         block.op(branch(builders.get(ci + 1).successor()));

Suggestion:

                        block.op(branch(next.successor()));

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

Marked as reviewed by psandoz (Lead).

PR Review: https://git.openjdk.org/babylon/pull/39#pullrequestreview-1946803904
PR Review Comment: https://git.openjdk.org/babylon/pull/39#discussion_r1530732049
PR Review Comment: https://git.openjdk.org/babylon/pull/39#discussion_r1530731336


More information about the babylon-dev mailing list