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

Archie L. Cobbs duke at openjdk.org
Thu Jan 12 17:53:25 UTC 2023


On Thu, 12 Jan 2023 12:15:17 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Archie L. Cobbs has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Use the more appropriate Type comparison method Types.isSameType().
>>  - Add some more comments to clarify how the analysis works.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java line 175:
> 
>> 173:     private DiagnosticPosition[] pendingWarning;
>> 174: 
>> 175: // These fields are scoped to the CONSTRUCTOR OR INVOKED METHOD BEING ANALYZED
> 
> Watch out for "all caps"

Will fix.

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java line 900:
> 
>> 898:             final Type.ClassType currentClassType = (Type.ClassType)this.methodClass.sym.type;
>> 899:             final Type methodOwnerType = sym.owner.type;
>> 900:             if (this.isSubtype(currentClassType, methodOwnerType)) {
> 
> I believe what you need here is not subtyping but subclassing - see `Symbol.isSubclass`

Hmm, I tried using `Symbol.isSubclass()` but it caused test failures. Obviously I don't quite know what I'm doing and I'm loathe to break what is already working. Do you have a suggested patch?

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java line 909:
> 
>> 907: 
>> 908:             // Check for implicit outer 'this' reference
>> 909:             if (this.types.hasOuterClass(currentClassType, methodOwnerType)) {
> 
> Similarly here - look for `Symbol.isEnclosedBy`

Same comment as previous: I don't quite know what I'm doing and I'm loathe to break what is already working. Do you have a suggested patch?

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

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



More information about the build-dev mailing list