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

Archie L. Cobbs duke at openjdk.org
Thu Jan 12 16:55:39 UTC 2023


On Thu, 12 Jan 2023 10:48:49 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 227:
> 
>> 225:                 final boolean privateOuterPrev = this.privateOuter;
>> 226:                 final Lint lintPrev = this.lint;
>> 227:                 this.lint = this.lint.augment(tree.sym);
> 
> general stylistic comment - I see here and everywhere in this class `this.xyz` - this is not the norm in the rest of the javac codebase, and it would be better to replace it with just `xyz`

OK. I will reluctantly remove...

`<PointlessRantDoNotRead>`
> Why on earth wouldn't you want to make it clear which one of N outer classes a field comes from, and that it's a field, and not a variable declared somewhere off screen??
> 
> IMHO omitting 'this' qualifiers is effectively accepting the grave offense of code obfuscation in exchange for a tiny smidgen of brevity.
>
> It's definitely made it harder for me to read and understand the compiler, with all the levels of nesting it has.
> 
`</PointlessRantDoNotRead>`

I readily admit I'm probably in the minority on this and anyway it's a bikeshed thing so there's no point in debating it.

I will go with the flow :) though I feel a little sorry for the next person who has to read this code.

> From a documentation perspective it might carry a bit of value

Yes, that's the purpose - the `final` is for the human viewer, not the compiler. Just trying to be helpful to the next person.

But you're right it's inconsistent so I'll remove.

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

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



More information about the build-dev mailing list