Loosening requirements for super() invocation

[Archie Cobbs]

Hmm.. I think we can do something pretty close to what @mcimadamore is
proposing in a way that doesn't care how the compilation is performed.

The requirement is to be able to look at some class X being compiled and
decide whether any subclasses of X could exist outside of its "maintenance
domain".

Currently, we have maintenance domain = the compilation unit (i.e., the
file X.java). In this case we ask: Is X not final, not sealed (or, sealed
but permitting some class defined outside of X.java), and does X have a
non-private constructor?

In the definition that @mcimadamore is proposing, we could ask: Is X
public, not final, not sealed, and does X have a non-private constructor?

Ideally we'd also ask "Is X's package exported by its module, if any?" but
now you're making assumptions about how the code is being compiled. But
even without that, the above criteria should get you pretty close I would
guess.

The tangible benefit here is exemplified by the reduction in JDK warnings
from 2093 to 1334 (36%) when you use the tighter criteria.

I'm thinking since this is a new warning, we should err on the side of less
aggressiveness, but then we can dial it up over time if needed. Kind of
like how GCC's -Wcast-qual has gotten more and more picky over the years
(it seems).

-Archie

On Mon, Jan 16, 2023 at 12:45 PM Brian Goetz <brian.goetz at oracle.com> wrote:

> This is an interesting one.  The design of sealed classes deliberately
> took the unit of maintenance into account: in the unnamed module, they must
> be the same package, and in a named module, they must be in the same
> module.  I am not sure if we require that they always be co-compiled; if we
> already do, then having the analysis cross the boundary makes sense, but if
> we permit a sealed nest to be compiled separately, then it might be weird
> to only do the this-escape analysis if they happen to be being co-compiled?
>
> On 1/16/2023 1:02 PM, Archie Cobbs wrote:
>
> Regarding 'this' escape, folks may have already seen the blizzard of PR
> comments but I thought I'd pull out this one
> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/11874*discussion_r1071455800__;Iw!!ACWV5N9M2RV99hQ!IZznC0UlwtXHI3-jvGUGjvt-p66LYa7TQXYMdn47RPw_H98g1KX4T3UnmOjDoKLkzN5TAcDnGIgTSz142mT7ZuQ$>
> from @mcimadamore as particularly interesting from a design perspective:
>
> To be fair, this is what my brain was reading when you talked about
>> "compilation unit" - but then saw that the code was doing it differently.
>> You see, for "sealed" classes we have a notion of "maintenance domain".
>> E.g. the classes in a permits clause of a sealed declaration must belong
>> to the same module (if available) or same package (if no module is
>> available). That's how you get the exhaustiveness analysis and all the
>> goodies, by essentially making a close-world assumption on the classes that
>> are passed to javac for a given compilation task. I think it would make a
>> lot of sense to apply these very same boundaries to the escape-this
>> analysis (and, in fact, when looking at warnings coming out of the JDK, I
>> often found false positives that were caused by this).
>>
>
> Any thoughts? Should we define the "boundary of concern" differently for
> sealed classes?
>
> -Archie
>
> --
> Archie L. Cobbs
>
>
>

-- 
Archie L. Cobbs
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-dev/attachments/20230116/ce95c0d7/attachment-0001.htm>