On Thu, 12 Jan 2023 10:48:49 GMT, Maurizio Cimadamore <mcimadam...@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
