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

[Maurizio Cimadamore]

On Wed, 11 Jan 2023 19:10:04 GMT, Archie L. Cobbs <duke at openjdk.org> wrote:

>> 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
>
> Regarding the assignment graph approach, I think that would work if the references are bouncing around strictly between variables, but what if the chain includes any of the more complicated stuff that is currently being tracked, such as various Java expressions, method invocations, conditionals, etc.?
> 
> Consider this example:
> 
> public class ThisEscapeLoop { 
>     
>     public ThisEscapeLoop() { 
>         ThisEscapeLoop ref1 = this;
>         ThisEscapeLoop ref2 = null;
>         ThisEscapeLoop ref3 = null;
>         ThisEscapeLoop ref4 = null;
>         for (int i = 0; i < 100; i++) {
>             ref4 = this.returnMe(ref3);
>             ref3 = ref2;
>             ref2 = ref1;
>             if (ref4 != null)
>                 ref4.mightLeak();
>         }
>     }
> 
>     public <T> T returnMe(T x) {
>         return x;
>     }
> 
>     public void mightLeak() {
>     }
> }
> 
> If you are only looking at variable assignments with values that are other variables, then the chain "breaks" when `ref4` is assigned indirectly via `returnMe()`, and you miss the leak.

> 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
> ```

Thanks for trying it out - good to know that build time isn't affected.

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

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