RFR: 8015831: Add lint check for calling overridable methods from a constructor [v5]

[Archie L. Cobbs]

On Wed, 11 Jan 2023 16:14:24 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> True - probably 3 * 3 can be achieved if this:
>> 
>> 
>>     ThisEscapeLoop ref21 = ref14;
>> 
>> Is replaced with   
>> 
>> 
>>     ThisEscapeLoop ref21 = this;
>> 
>> 
>> In which case the inner loop won't converge immediately (as it will have to propagate from ref21 to ref22 to ref23 to ref24).
>> 
>> I guess what I'm uncomfortable with is that we have effectively unbounded computation here (especially when we also consider the fact that the analysis "follows" method bodies as well, if they are found in the same compilation unit).
>> 
>> I suggest one experiment where you:
>> 1. downgrade the warnings to notes (so that they won't make the JDK build fail)
>> 2. enable this Lint everywhere
>> 3. compare JDK `clean images` time w/ and w/o the Lint
>
> Also, looking at the loop test more closely, it seems to me that what the compiler needs to do is to prove that there can be possible paths by which a `this` can land into ref4.
> 
> If we build a graph of all the assignments, we get:
> 
> ref4 <- ref3 <- ref2 <- ref1 <- this
> 
> So, if we ask "can ref4 possibly contain `this`?" we could "walk" the variable dependencies backwards and discover that, yes, there exist a possible path in which `this` would get there.
> 
> Now, of course without a loop this can never be a real issue (since you can never send a `this` fully down the chain) - but again, this is a question of how much effort should be spend to handle false negatives in what look like a pathological case.

Good idea. Looks like the difference is in the noise, at least on my Macbook:

Builds of master (jdk-21+3-69-gc6588d5bb3f)
==================================

Build times:

    real	2m24.650s
    user	13m46.727s
    sys	2m33.554s

    real	2m27.224s
    user	13m43.464s
    sys	2m37.251s

    real	2m26.658s
    user	13m42.578s
    sys	2m36.133s

Builds of ThisEscape (jdk-21+3-125-g6e96a7d76f8)
==================================

Modifications:

    - Reverted files in the make/ subdirectory to enable warning
    - Commented out lines 363-382 in ThisEscapeAnalyzer.java
      so no warnings are actually reported

Build times:

    real	2m25.912s
    user	13m45.860s
    sys	2m32.741s

    real	2m27.213s
    user	13m44.830s
    sys	2m36.596s

    real	2m25.756s
    user	13m42.889s
    sys	2m35.659s

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

PR: https://git.openjdk.org/jdk/pull/11874