Loosening requirements for super() invocation

[Brian Goetz]

With the relaxation you suggest, it seems like abstract base classes 
that permit this-escape would not be flagged if they are 
package-private, even if the subclasses are themselves public?

I think what we're really appealing to is that the base class offers the 
_potential_ for this-escape, but some subclasses may decline the 
opportunity to exploit it, and we have some combinations of classes 
(abstract package-private base + final public subclass, sealed base + 
final subclass, etc) where we could do a more global analysis and say 
"this nest of classes is OK."  But that feels like we're making the 
analysis more complicated, rather than less?  Or am I missing the question?


On 1/16/2023 7:10 PM, Archie Cobbs wrote:
> 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/20230117/00e28c64/attachment.htm>