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