[code-reflection] RFR: Assertion Interpreter Implementation [v3]

Paul Sandoz psandoz at openjdk.org
Fri Mar 8 23:44:21 UTC 2024


On Fri, 8 Mar 2024 23:20:42 GMT, Ian Graves <igraves at openjdk.org> wrote:

>> The interpreter side of the assertion work.
>
> Ian Graves has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Removing unnecessary List argument for unused args

src/java.base/share/classes/java/lang/reflect/code/interpreter/Interpreter.java line 331:

> 329:     }
> 330: 
> 331:     static <T extends Op & Op.Invokable>

Can we refactor to also share this code in the `invoke` method rather than duplicate?

Suggest renaming from `invokeBody` to `interpretBody`.

src/java.base/share/classes/java/lang/reflect/code/interpreter/Interpreter.java line 604:

> 602:         } else if (o instanceof CoreOps.AssertOp _assert) {
> 603:             Body testBody = _assert.bodies.get(0);
> 604:             Boolean testResult = (Boolean) invokeBody(l,testBody,oc);

Consistent style and can cast to primitive type:

Suggestion:

            boolean testResult = (boolean) invokeBody(l, testBody, oc);

src/java.base/share/classes/java/lang/reflect/code/interpreter/Interpreter.java line 607:

> 605:             if (!testResult) {
> 606:                 if (_assert.bodies.size() > 1) {
> 607:                     Body messageBlock = _assert.bodies.get(1);

Suggestion:

                    Body messageBody = _assert.bodies.get(1);

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ReflectMethods.java line 1842:

> 1840: 
> 1841:                 pushBody(detail,
> 1842:                          FunctionType.functionType(typeToTypeElement(tree.detail.type)));

Recommend adding another test to the language test for an assert with a second expression evaluating to something other than String.

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

PR Review Comment: https://git.openjdk.org/babylon/pull/36#discussion_r1518411757
PR Review Comment: https://git.openjdk.org/babylon/pull/36#discussion_r1518412637
PR Review Comment: https://git.openjdk.org/babylon/pull/36#discussion_r1518414044
PR Review Comment: https://git.openjdk.org/babylon/pull/36#discussion_r1518411059


More information about the babylon-dev mailing list