On Thu, 12 Jan 2023 19:24:50 GMT, Archie L. Cobbs <d...@openjdk.org> wrote:
>> This patch passes all tests:
>>
>>
>> diff --git
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>>
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> index 9d35c2fbc0a..e755c54d0c8 100644
>> ---
>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> +++
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java
>> @@ -451,7 +451,7 @@ class ThisEscapeAnalyzer extends TreeScanner {
>> }
>>
>> // If the expression type is incompatible with 'this', discard
>> it
>> - if (type != null && !this.isSubtype(this.targetClass.sym.type,
>> type))
>> + if (type != null && !this.targetClass.sym.isSubClass(type.tsym,
>> types))
>> this.refs.remove(ExprRef.direct(this.depth));
>> }
>> }
>> @@ -672,7 +672,7 @@ class ThisEscapeAnalyzer extends TreeScanner {
>> if (explicitOuterThis != null) {
>> this.scan(explicitOuterThis);
>> this.refs.removeExprs(this.depth, direct -> outerRefs.add(new
>> OuterRef(direct)));
>> - } else if (this.types.hasOuterClass(type, this.methodClass.type)) {
>> + } else if (type.tsym.isEnclosedBy(this.methodClass.sym)) {
>> if (this.refs.contains(ThisRef.direct()))
>> outerRefs.add(OuterRef.direct());
>> if (this.refs.contains(ThisRef.indirect()))
>> @@ -801,9 +801,8 @@ class ThisEscapeAnalyzer extends TreeScanner {
>> // Explicit outer 'this' reference?
>> final Type selectedType = this.types.erasure(tree.selected.type);
>> if (selectedType.hasTag(CLASS)) {
>> - final Type.ClassType selectedClassType =
>> (Type.ClassType)selectedType;
>> if (tree.name == this.names._this &&
>> - this.types.hasOuterClass(currentClassType,
>> selectedClassType)) {
>> +
>> currentClassType.tsym.isEnclosedBy((ClassSymbol)selectedType.tsym)) {
>> if (this.refs.contains(OuterRef.direct()))
>> this.refs.add(ExprRef.direct(this.depth));
>> if (this.refs.contains(OuterRef.indirect()))
>> @@ -895,9 +894,7 @@ class ThisEscapeAnalyzer extends TreeScanner {
>> final MethodSymbol sym = (MethodSymbol)tree.sym;
>>
>> // Check for implicit 'this' reference
>> - final Type.ClassType currentClassType =
>> (Type.ClassType)this.methodClass.sym.type;
>> - final Type methodOwnerType = sym.owner.type;
>> - if (this.isSubtype(currentClassType, methodOwnerType)) {
>> + if (methodClass.sym.isSubClass(sym.owner, types)) {
>> if (this.refs.contains(ThisRef.direct()))
>> this.refs.add(ExprRef.direct(this.depth));
>> if (this.refs.contains(ThisRef.indirect()))
>> @@ -906,7 +903,7 @@ class ThisEscapeAnalyzer extends TreeScanner {
>> }
>>
>> // Check for implicit outer 'this' reference
>> - if (this.types.hasOuterClass(currentClassType,
>> methodOwnerType)) {
>> + if (methodClass.sym.isEnclosedBy((ClassSymbol)sym.owner)) {
>> if (this.refs.contains(OuterRef.direct()))
>> this.refs.add(ExprRef.direct(this.depth));
>>
>>
>> Btw, I believe a similar trick can be used in
>> TreeInfo.isExplicitThisReference. If I'm correct, `hasOuterClass` should
>> probably be removed as it duplicates already existing functionality.
>>
>> Since I'm bringing this up, as TreeInfo.isExplicitThisReference is only used
>> by the new analyzer, it would be cleaner if it was defined there, at least
>> until it's clear it might be needed by some other client.
>
> Weird. I don't get that build failure.
>
> Neither does github... I have been relying on the github build "Actions"
> succeeding to determine if "the build works" and according to that it is
> building.
>
> I will add the `DISABLED_WARNINGS` in any case.
Thanks for the patch!
The semantics of `hasOuterClass()` returns false if A and B are the same class,
while `isEnclosedBy()` returns true if A and B are the same class.
However, I don't think it would actually matter in practice...
Regardless, I'll add the extra equality comparison and apply your patch and
also the suggestions to integrate `isExplicitThisReference()` and eliminate
`hasOuterClass()`.
-------------
PR: https://git.openjdk.org/jdk/pull/11874
