Skip to content

Commit

Permalink
Fix the handling the resolution of the implicit qualifier in anonymou…
Browse files Browse the repository at this point in the history
…s class creation.

In the Kotlin fronted there can be arbitrary code before the super/this constructor call which can include anonymous class instantiation. Those instantiations can not capture the enclosing instance.

PiperOrigin-RevId: 601290089
  • Loading branch information
rluble authored and copybara-github committed Jan 25, 2024
1 parent 6b1e11e commit c1c8251
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,65 +16,110 @@
import com.google.j2cl.transpiler.ast.AbstractRewriter;
import com.google.j2cl.transpiler.ast.AstUtils;
import com.google.j2cl.transpiler.ast.CompilationUnit;
import com.google.j2cl.transpiler.ast.ExpressionStatement;
import com.google.j2cl.transpiler.ast.Member;
import com.google.j2cl.transpiler.ast.Expression;
import com.google.j2cl.transpiler.ast.MemberReference;
import com.google.j2cl.transpiler.ast.Method;
import com.google.j2cl.transpiler.ast.MethodCall;
import com.google.j2cl.transpiler.ast.NewInstance;
import com.google.j2cl.transpiler.ast.Node;
import java.util.function.Predicate;
import com.google.j2cl.transpiler.ast.Type;

/** Resolves implicit qualifiers for instance members and constructors. */
public class ResolveImplicitInstanceQualifiers extends NormalizationPass {
@Override
public void applyTo(CompilationUnit compilationUnit) {

compilationUnit.accept(
new AbstractRewriter() {
@Override
public MemberReference rewriteMemberReference(MemberReference memberReference) {
return AstUtils.resolveImplicitQualifier(
memberReference, getCurrentType().getTypeDescriptor());
}
fixupAnonymousClassQualifiersInConstructors(compilationUnit);
resolveImplicitInstanceQualifiers(compilationUnit);
}

/**
* Prevents the resolution of the implicit qualifiers in anonymous inner classes to this if they
* occur before the super/this constructor call.
*/
private static void fixupAnonymousClassQualifiersInConstructors(CompilationUnit compilationUnit) {
// Anonymous classes are considered to capture the enclosing instance unless they
// appear in a static method. But when declared inside a constructor they might only
// capture the enclosing instance only if they don't appear before the super/this
// constructor call invocation.
// To workaround that we continue to construct anonymous inner classes as if they
// capture the enclosing instance, but pass `null` as the instance if the instantiation
// happens before the super/this constructor call.
compilationUnit
.streamTypes()
.flatMap(t -> t.getConstructors().stream())
.forEach(
c -> {
MethodCall delegatedConstructorCall = AstUtils.getConstructorInvocation(c);
if (delegatedConstructorCall == null
|| !delegatedConstructorCall.getTarget().isJsConstructor()) {
return;
}

fixDelegatedConstructorCall(c, delegatedConstructorCall);
});
}

private static void fixDelegatedConstructorCall(
Method constructor, MethodCall delegatedConstructorCall) {
constructor.accept(
new AbstractRewriter() {
@Override
public Node rewriteNewInstance(NewInstance newInstance) {
// Anonymous classes are considered to capture the enclosing instance unless they
// appear in a static method. But when declared inside a constructor they might only
// capture the enclosing instance only if they don't appear before the super/this
// constructor call invocation.
// To workaround that we continue to construct anonymous inner classes as if they
// capture the enclosing instance, but pass `null` as the instance if the instantiation
// happens before the super/this constructor call.
if (newInstance.getQualifier() != null) {
return newInstance;
}

Member currentMember = getCurrentMember();
if (newInstance
.getTarget()
.getEnclosingTypeDescriptor()
.getTypeDeclaration()
.isAnonymous()
&& newInstance.getQualifier() == null
&& currentMember.isConstructor()) {
Method method = (Method) currentMember;
ExpressionStatement constructorInvocationStatement =
AstUtils.getConstructorInvocationStatement(method);
.getTarget()
.getEnclosingTypeDescriptor()
.getTypeDeclaration()
.isAnonymous()) {
// This is an anonymous class instantiation with an implicit qualifier
// happening in a constructor before the super/this call.
// Pass null as the enclosing instance instead of a this reference.
return NewInstance.Builder.from(newInstance)
.setQualifier(
constructor.getDescriptor().getEnclosingTypeDescriptor().getNullValue())
.build();
}
return newInstance;
}

@Override
public boolean shouldProcessType(Type type) {
// Do not recurse into nested types.
return false;
}

// Special treat anonymous class creation when declared in a parameter of a
// super/this constructor invocation.
boolean inSuperOrThisCall =
getParent(Predicate.isEqual(constructorInvocationStatement)) != null;
if (inSuperOrThisCall
// Restrict the handling to JsConstructor super/this calls. These are the only
// cases where the generated code is not correct for JS.
&& AstUtils.getConstructorInvocation(method).getTarget().isJsConstructor()) {
private boolean foundDelegatedConstructorCall = false;

return NewInstance.Builder.from(newInstance)
.setQualifier(
currentMember.getDescriptor().getEnclosingTypeDescriptor().getNullValue())
.build();
}
@Override
public Node rewriteExpression(Expression expression) {
if (expression == delegatedConstructorCall) {
// All the sub nodes have been processed and we found the super constructor
// call. Anonymous classes instantiated after this point can capture the
// enclosing instance.
foundDelegatedConstructorCall = true;
}
return rewriteMemberReference(newInstance);
return expression;
}

@Override
public boolean shouldProcessNode(Node node) {
// Skip all processing after the constructor call was found.
return !foundDelegatedConstructorCall;
}
});
}

private static void resolveImplicitInstanceQualifiers(CompilationUnit compilationUnit) {
compilationUnit.accept(
new AbstractRewriter() {
@Override
public MemberReference rewriteMemberReference(MemberReference memberReference) {
return AstUtils.resolveImplicitQualifier(
memberReference, getCurrentType().getTypeDescriptor());
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,15 @@ class JsConstructorClass {
JsConstructorClass(Object o) {}

JsConstructorClass() {
this(new Object() {});
this(trueVar ? new Object() {} : null);
}

static boolean trueVar = true;
}

class JsConstructorSubclass extends JsConstructorClass {
@JsConstructor
JsConstructorSubclass() {
super(new Object() {});
super(trueVar ? new Object() {} : null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,27 @@ class JsConstructorClass extends j_l_Object {
/** @nodts @return {!JsConstructorClass} */
static $create__() {
JsConstructorClass.$clinit();
let $instance = new JsConstructorClass($1.$create__anonymousclass_JsConstructorClass(null));
let $instance = new JsConstructorClass(JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass ? $1.$create__anonymousclass_JsConstructorClass(null) : null);
$instance.$ctor__anonymousclass_JsConstructorClass__void();
return $instance;
}
//Initialization from constructor 'JsConstructorClass()'.
/** @nodts */
$ctor__anonymousclass_JsConstructorClass__void() {}
/** @nodts @return {boolean} */
static get f_trueVar__anonymousclass_JsConstructorClass() {
return (JsConstructorClass.$clinit(), JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass);
}
/** @nodts */
static set f_trueVar__anonymousclass_JsConstructorClass(/** boolean */ value) {
(JsConstructorClass.$clinit(), JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass = value);
}
/** @nodts */
static $clinit() {
JsConstructorClass.$clinit = () =>{};
JsConstructorClass.$loadModules();
j_l_Object.$clinit();
JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass = true;
}
/** @nodts @return {boolean} */
static $isInstance(/** ? */ instance) {
Expand All @@ -45,6 +54,8 @@ class JsConstructorClass extends j_l_Object {
$1 = goog.module.get('anonymousclass.JsConstructorClass.$1$impl');
}
}
/**@private {boolean} @nodts*/
JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass = false;
$Util.$setClassMetadata(JsConstructorClass, 'anonymousclass.JsConstructorClass');

exports = JsConstructorClass;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,18 @@
[{}] => [this.$ctor__java_lang_Object__void();] "anonymousclass.JsConstructorClass.<init>"
[JsConstructorClass] => [$create__]
[JsConstructorClass] => [JsConstructorClass.$clinit();] "anonymousclass.JsConstructorClass.<synthetic: ctor_create>"
[JsConstructorClass] => [let $instance = new JsConstructorClass($1.$create__anonymousclass_JsConstructorClass(null));] "anonymousclass.JsConstructorClass.<synthetic: ctor_create>"
[JsConstructorClass] => [let $instance = new JsConstructorClass(JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass ? $1.$create__anonymousclass_JsConstructorClass(null) : null);] "anonymousclass.JsConstructorClass.<synthetic: ctor_create>"
[JsConstructorClass] => [$instance.$ctor__anonymousclass_JsConstructorClass__void();] "anonymousclass.JsConstructorClass.<synthetic: ctor_create>"
[JsConstructorClass] => [return $instance;] "anonymousclass.JsConstructorClass.<synthetic: ctor_create>"
[JsConstructorClass] => [$ctor__anonymousclass_JsConstructorClass__void]
[static boolean trueVar = true;] => [get f_trueVar__anonymousclass_JsConstructorClass]
[static boolean trueVar = true;] => [return (JsConstructorClass.$clinit(), JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass);] "anonymousclass.JsConstructorClass.<synthetic: getter>"
[static boolean trueVar = true;] => [set f_trueVar__anonymousclass_JsConstructorClass]
[static boolean trueVar = true;] => [(JsConstructorClass.$clinit(), JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass = value);] "anonymousclass.JsConstructorClass.<synthetic: setter>"
[JsConstructorClass] => [$clinit]
[JsConstructorClass] => [JsConstructorClass.$clinit = () =>{};] "anonymousclass.JsConstructorClass.<clinit>"
[JsConstructorClass] => [JsConstructorClass.$loadModules();] "anonymousclass.JsConstructorClass.<clinit>"
[JsConstructorClass] => [j_l_Object.$clinit();] "anonymousclass.JsConstructorClass.<clinit>"
[static boolean trueVar = true;] => [JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass = true;] "anonymousclass.JsConstructorClass.<clinit>"
[JsConstructorClass] => [/**@private {boolean} @nodts*/
JsConstructorClass.$static_trueVar__anonymousclass_JsConstructorClass = false;]
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class JsConstructorSubclass extends JsConstructorClass {

constructor() {
JsConstructorSubclass.$clinit();
super($1.$create__anonymousclass_JsConstructorSubclass(null));
super(JsConstructorClass.f_trueVar__anonymousclass_JsConstructorClass ? $1.$create__anonymousclass_JsConstructorSubclass(null) : null);
this.$ctor__anonymousclass_JsConstructorSubclass__void();
}
/** @nodts */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[JsConstructorSubclass] => [JsConstructorSubclass]
[JsConstructorSubclass] => [constructor]
[JsConstructorSubclass] => [JsConstructorSubclass.$clinit();] "anonymousclass.JsConstructorSubclass.<init>"
[JsConstructorSubclass] => [super($1.$create__anonymousclass_JsConstructorSubclass(null));] "anonymousclass.JsConstructorSubclass.<init>"
[JsConstructorSubclass] => [super(JsConstructorClass.f_trueVar__anonymousclass_JsConstructorClass ? $1.$create__anonymousclass_JsConstructorSubclass(null) : null);] "anonymousclass.JsConstructorSubclass.<init>"
[JsConstructorSubclass] => [this.$ctor__anonymousclass_JsConstructorSubclass__void();] "anonymousclass.JsConstructorSubclass.<init>"
[JsConstructorSubclass] => [$ctor__anonymousclass_JsConstructorSubclass__void]
[JsConstructorSubclass] => [$clinit]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package anonymousclass
import javaemul.lang.*
import jsinterop.annotations.JsConstructor
import kotlin.Any
import kotlin.Boolean
import kotlin.Int
import kotlin.OptIn
import kotlin.String
Expand Down Expand Up @@ -115,10 +116,15 @@ open class JsConstructorClass {
@JsConstructor
internal constructor(o: Any?)

internal constructor(): this(object : Any() {})
internal constructor(): this(if (JsConstructorClass.trueVar_pp_anonymousclass) object : Any() {} else null)

companion object {
@JvmField
internal var trueVar_pp_anonymousclass: Boolean = true
}
}

open class JsConstructorSubclass: JsConstructorClass {
@JsConstructor
internal constructor(): super(object : Any() {})
internal constructor(): super(if (JsConstructorClass.trueVar_pp_anonymousclass) object : Any() {} else null)
}
Loading

0 comments on commit c1c8251

Please sign in to comment.