On Tue, 13 Jan 2026 19:46:48 GMT, Vicente Romero <[email protected]> wrote:
>> Vicente Romero has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> addressing review comments
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/NullChecksWriter.java
> line 170:
>
>> 168: public void visitLetExpr(LetExpr tree) {
>> 169: // don't recurse inside let expressions, they are backend
>> artifacts
>> 170: result = tree;
>
> offline suggestion from Jan Lahoda:
>
> this code will skip all let expressions which is too broad, as let
> expressions are generated also by switch expressions but could contain user
> code. For example there is no null check generated in this case:
>
> void main() {
> int i = switch (new Object()) {
> default -> {
> Object obj = null;
> Object! nonNull = obj;
> yield 0;
> }
> };
> }
Actually, I think we can remove this visitor. Sure, javac will add an extra,
redundant null check for null-restricted type test patterns, but that's not
incorrect (this check will never throw NPE because, if `null`, the pattern
won't match -- thanks to your changes in `TransPatterns`). I think I prefer a
redundant check than having to mark all LetExpr that might need recursion (as
we will surely forget some).
Separately, we can work on removing redundant null checks -- e.g. for things
that are co-compiled and such, in which case this will fall out for free.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1884#discussion_r2689983882
