RFR: 8164714: Constructor.newInstance creates instance of inner class with null outer class [v3]

[Vicente Romero]

On Wed, 5 Mar 2025 16:42:02 GMT, Chen Liang <liach at openjdk.org> wrote:

>> The Java Language Specification anticipates that inner classes always have non-null enclosing instances. It ensures the non-nullness by enforcing null checks at the use sites that provides immediately enclosing instances to inner class constructors, such as for super invocations, or an `outer.new Inner()` invocation.
>> 
>> However, the translations do not require a null check in the actual constructor, when the immediately enclosing instance is received through a mandated parameter and stored into a synthetic field or discarded.  As a result, class file constructs, such as core reflection, method handles, or arbitrary class files can pass in `null` for the immediately enclosing instance, and later execution may fail with NPE by chance if any enclosing instance is used.
>> 
>> This patch proposes to add a null check against the "outer this" in inner class constructors that call a superclass constructor, including when the "outer this" is discarded immediately thereafter (#4966) for consistency. This null check will be emitted regardless of source or target versions. This change is considered an implementation artifact like the synthetic field that captures the enclosing instance; as a result, there is no JLS change.
>> 
>> The reason for this eager NPE decision is that there is no compatibility of such NPE behaviors - any evolution of the inner classes constructed with null enclosing instances may suddenly start using an enclosing instance and fail with NPE. Therefore, there's no compatibility aspect in such out-of-spec usages of passing `null` as immediately enclosing instance, and this null check can be considered such an evolution.
>
> Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/enclosing-this-null-check
>  - Fix javap test
>  - 8164714: Constructor.newInstance creates instance of inner class with null outer class

looks sensible

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 1797:

> 1795:     }
> 1796: 
> 1797:     /** Return tree simulating null checking outer this and assigning. */

probably this comment should state that the assignment is generated or not depending on the value of argument `stores`

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

Marked as reviewed by vromero (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23875#pullrequestreview-2671613345
PR Review Comment: https://git.openjdk.org/jdk/pull/23875#discussion_r1987661446