Skip to content

Commit

Permalink
Remove invalid reference to this as an enclosing instance for anony…
Browse files Browse the repository at this point in the history
…mous classes in parameters to super/this construtor calls.

Anonymous classes are considered to capture their enclosing instance except when declared in a static method. In constructors those classes capture the enclosing instance unless they appear as a parameter to a super/this constructor call. Since the type model object is constructed independent from the code, anonymous classes that are declared in a parameter to a super/this constructor call are considered as capturing the enclosing instance.

This cl works around the problem by changing the way implicit qualifiers are computed, and in this cases the anonymous class will receive a null enclosing instance instead of a this reference. Such value will never be referenced since both Java and Kotlin forbid referencing `this` impliclitly or explicitly in these situation.

PiperOrigin-RevId: 600956219
  • Loading branch information
rluble authored and copybara-github committed Jan 24, 2024
1 parent 6e38485 commit ff9f7b4
Show file tree
Hide file tree
Showing 17 changed files with 1,005 additions and 109 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@
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.MemberReference;
import com.google.j2cl.transpiler.ast.Method;
import com.google.j2cl.transpiler.ast.NewInstance;
import com.google.j2cl.transpiler.ast.Node;
import java.util.function.Predicate;

/** Resolves implicit qualifiers for instance members and constructors. */
public class ResolveImplicitInstanceQualifiers extends NormalizationPass {
Expand All @@ -30,6 +36,46 @@ public MemberReference rewriteMemberReference(MemberReference memberReference) {
return AstUtils.resolveImplicitQualifier(
memberReference, getCurrentType().getTypeDescriptor());
}

@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.

Member currentMember = getCurrentMember();
if (newInstance
.getTarget()
.getEnclosingTypeDescriptor()
.getTypeDeclaration()
.isAnonymous()
&& newInstance.getQualifier() == null
&& currentMember.isConstructor()) {
Method method = (Method) currentMember;
ExpressionStatement constructorInvocationStatement =
AstUtils.getConstructorInvocationStatement(method);

// 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()) {

return NewInstance.Builder.from(newInstance)
.setQualifier(
currentMember.getDescriptor().getEnclosingTypeDescriptor().getNullValue())
.build();
}
}
return rewriteMemberReference(newInstance);
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package anonymousclass;

import jsinterop.annotations.JsConstructor;

abstract class SomeClass {
public abstract String foo();

Expand Down Expand Up @@ -78,3 +80,20 @@ public String foo() {
}
};
}

// Test case for b/321755877
class JsConstructorClass {
@JsConstructor
JsConstructorClass(Object o) {}

JsConstructorClass() {
this(new Object() {});
}
}

class JsConstructorSubclass extends JsConstructorClass {
@JsConstructor
JsConstructorSubclass() {
super(new Object() {});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ package(

readable_example(
srcs = glob(["*.java"]),
deps = ["//third_party:gwt-jsinterop-annotations-j2cl"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
goog.module('anonymousclass.JsConstructorClass.$1$impl');

const j_l_Object = goog.require('java.lang.Object$impl');
const $Util = goog.require('nativebootstrap.Util$impl');

let JsConstructorClass = goog.forwardDeclare('anonymousclass.JsConstructorClass$impl');

class $1 extends j_l_Object {
/** @protected @nodts */
constructor() {
super();
/**@type {JsConstructorClass} @nodts*/
this.$outer_this__anonymousclass_JsConstructorClass_1;
}
/** @nodts @return {!$1} */
static $create__anonymousclass_JsConstructorClass(/** JsConstructorClass */ $outer_this) {
$1.$clinit();
let $instance = new $1();
$instance.$ctor__anonymousclass_JsConstructorClass_1__anonymousclass_JsConstructorClass__void($outer_this);
return $instance;
}
/** @nodts */
$ctor__anonymousclass_JsConstructorClass_1__anonymousclass_JsConstructorClass__void(/** JsConstructorClass */ $outer_this) {
this.$outer_this__anonymousclass_JsConstructorClass_1 = $outer_this;
this.$ctor__java_lang_Object__void();
}
/** @nodts */
static $clinit() {
$1.$clinit = () =>{};
$1.$loadModules();
j_l_Object.$clinit();
}
/** @nodts @return {boolean} */
static $isInstance(/** ? */ instance) {
return instance instanceof $1;
}

/** @nodts */
static $loadModules() {}
}
$Util.$setClassMetadata($1, 'anonymousclass.JsConstructorClass$1');

exports = $1;

//# sourceMappingURL=JsConstructorClass$1.js.map
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
goog.module('anonymousclass.JsConstructorClass.$1');

goog.require('anonymousclass.JsConstructorClass');
goog.require('java.lang.Object');
goog.require('nativebootstrap.Util');

const $1 = goog.require('anonymousclass.JsConstructorClass.$1$impl');
exports = $1;
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[{}] => [$1]
[{}] => [constructor]
[{}] => [super();] "anonymousclass.JsConstructorClass$1.<synthetic: ctor_js>"
[{}] => [/**@type {JsConstructorClass} @nodts*/
this.$outer_this__anonymousclass_JsConstructorClass_1;] "anonymousclass.JsConstructorClass$1.<synthetic: ctor_js>"
[{}] => [$create__anonymousclass_JsConstructorClass]
[{}] => [$1.$clinit();] "anonymousclass.JsConstructorClass$1.<synthetic: ctor_create>"
[{}] => [let $instance = new $1();] "anonymousclass.JsConstructorClass$1.<synthetic: ctor_create>"
[{}] => [$instance.$ctor__anonymousclass_JsConstructorClass_1__anonymousclass_JsConstructorClass__void($outer_this);] "anonymousclass.JsConstructorClass$1.<synthetic: ctor_create>"
[{}] => [return $instance;] "anonymousclass.JsConstructorClass$1.<synthetic: ctor_create>"
[{}] => [$ctor__anonymousclass_JsConstructorClass_1__anonymousclass_JsConstructorClass__void]
[{}] => [this.$outer_this__anonymousclass_JsConstructorClass_1 = $outer_this;] "anonymousclass.JsConstructorClass$1.<init>"
[{}] => [this.$ctor__java_lang_Object__void();] "anonymousclass.JsConstructorClass$1.<init>"
[{}] => [$clinit]
[{}] => [$1.$clinit = () =>{};] "anonymousclass.JsConstructorClass$1.<clinit>"
[{}] => [$1.$loadModules();] "anonymousclass.JsConstructorClass$1.<clinit>"
[{}] => [j_l_Object.$clinit();] "anonymousclass.JsConstructorClass$1.<clinit>"
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
goog.module('anonymousclass.JsConstructorClass$impl');

const j_l_Object = goog.require('java.lang.Object$impl');
const $Util = goog.require('nativebootstrap.Util$impl');

let $1 = goog.forwardDeclare('anonymousclass.JsConstructorClass.$1$impl');

class JsConstructorClass extends j_l_Object {
//JsConstructor 'JsConstructorClass(Object)'.

constructor(/** * */ o) {
JsConstructorClass.$clinit();
super();
this.$ctor__anonymousclass_JsConstructorClass__java_lang_Object__void(o);
}
//Initialization from constructor 'JsConstructorClass(Object)'.
/** @nodts */
$ctor__anonymousclass_JsConstructorClass__java_lang_Object__void(/** * */ o) {
this.$ctor__java_lang_Object__void();
}
//Factory method corresponding to constructor 'JsConstructorClass()'.
/** @nodts @return {!JsConstructorClass} */
static $create__() {
JsConstructorClass.$clinit();
let $instance = new JsConstructorClass($1.$create__anonymousclass_JsConstructorClass(null));
$instance.$ctor__anonymousclass_JsConstructorClass__void();
return $instance;
}
//Initialization from constructor 'JsConstructorClass()'.
/** @nodts */
$ctor__anonymousclass_JsConstructorClass__void() {}
/** @nodts */
static $clinit() {
JsConstructorClass.$clinit = () =>{};
JsConstructorClass.$loadModules();
j_l_Object.$clinit();
}
/** @nodts @return {boolean} */
static $isInstance(/** ? */ instance) {
return instance instanceof JsConstructorClass;
}

/** @nodts */
static $loadModules() {
$1 = goog.module.get('anonymousclass.JsConstructorClass.$1$impl');
}
}
$Util.$setClassMetadata(JsConstructorClass, 'anonymousclass.JsConstructorClass');

exports = JsConstructorClass;

//# sourceMappingURL=JsConstructorClass.js.map
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
goog.module('anonymousclass.JsConstructorClass');

goog.require('anonymousclass.JsConstructorClass.$1');
goog.require('java.lang.Object');
goog.require('nativebootstrap.Util');

const JsConstructorClass = goog.require('anonymousclass.JsConstructorClass$impl');
exports = JsConstructorClass;
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
[JsConstructorClass] => [JsConstructorClass]
[JsConstructorClass] => [constructor]
[o] => [o] "o"
[JsConstructorClass] => [JsConstructorClass.$clinit();] "anonymousclass.JsConstructorClass.<init>"
[JsConstructorClass] => [super();] "anonymousclass.JsConstructorClass.<init>"
[JsConstructorClass] => [this.$ctor__anonymousclass_JsConstructorClass__java_lang_Object__void(o);] "anonymousclass.JsConstructorClass.<init>"
[JsConstructorClass] => [$ctor__anonymousclass_JsConstructorClass__java_lang_Object__void]
[o] => [o] "o"
[{}] => [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] => [$instance.$ctor__anonymousclass_JsConstructorClass__void();] "anonymousclass.JsConstructorClass.<synthetic: ctor_create>"
[JsConstructorClass] => [return $instance;] "anonymousclass.JsConstructorClass.<synthetic: ctor_create>"
[JsConstructorClass] => [$ctor__anonymousclass_JsConstructorClass__void]
[JsConstructorClass] => [$clinit]
[JsConstructorClass] => [JsConstructorClass.$clinit = () =>{};] "anonymousclass.JsConstructorClass.<clinit>"
[JsConstructorClass] => [JsConstructorClass.$loadModules();] "anonymousclass.JsConstructorClass.<clinit>"
[JsConstructorClass] => [j_l_Object.$clinit();] "anonymousclass.JsConstructorClass.<clinit>"
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
goog.module('anonymousclass.JsConstructorSubclass.$1$impl');

const j_l_Object = goog.require('java.lang.Object$impl');
const $Util = goog.require('nativebootstrap.Util$impl');

let JsConstructorSubclass = goog.forwardDeclare('anonymousclass.JsConstructorSubclass$impl');

class $1 extends j_l_Object {
/** @protected @nodts */
constructor() {
super();
/**@type {JsConstructorSubclass} @nodts*/
this.$outer_this__anonymousclass_JsConstructorSubclass_1;
}
/** @nodts @return {!$1} */
static $create__anonymousclass_JsConstructorSubclass(/** JsConstructorSubclass */ $outer_this) {
$1.$clinit();
let $instance = new $1();
$instance.$ctor__anonymousclass_JsConstructorSubclass_1__anonymousclass_JsConstructorSubclass__void($outer_this);
return $instance;
}
/** @nodts */
$ctor__anonymousclass_JsConstructorSubclass_1__anonymousclass_JsConstructorSubclass__void(/** JsConstructorSubclass */ $outer_this) {
this.$outer_this__anonymousclass_JsConstructorSubclass_1 = $outer_this;
this.$ctor__java_lang_Object__void();
}
/** @nodts */
static $clinit() {
$1.$clinit = () =>{};
$1.$loadModules();
j_l_Object.$clinit();
}
/** @nodts @return {boolean} */
static $isInstance(/** ? */ instance) {
return instance instanceof $1;
}

/** @nodts */
static $loadModules() {}
}
$Util.$setClassMetadata($1, 'anonymousclass.JsConstructorSubclass$1');

exports = $1;

//# sourceMappingURL=JsConstructorSubclass$1.js.map
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
goog.module('anonymousclass.JsConstructorSubclass.$1');

goog.require('anonymousclass.JsConstructorSubclass');
goog.require('java.lang.Object');
goog.require('nativebootstrap.Util');

const $1 = goog.require('anonymousclass.JsConstructorSubclass.$1$impl');
exports = $1;
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[{}] => [$1]
[{}] => [constructor]
[{}] => [super();] "anonymousclass.JsConstructorSubclass$1.<synthetic: ctor_js>"
[{}] => [/**@type {JsConstructorSubclass} @nodts*/
this.$outer_this__anonymousclass_JsConstructorSubclass_1;] "anonymousclass.JsConstructorSubclass$1.<synthetic: ctor_js>"
[{}] => [$create__anonymousclass_JsConstructorSubclass]
[{}] => [$1.$clinit();] "anonymousclass.JsConstructorSubclass$1.<synthetic: ctor_create>"
[{}] => [let $instance = new $1();] "anonymousclass.JsConstructorSubclass$1.<synthetic: ctor_create>"
[{}] => [$instance.$ctor__anonymousclass_JsConstructorSubclass_1__anonymousclass_JsConstructorSubclass__void($outer_this);] "anonymousclass.JsConstructorSubclass$1.<synthetic: ctor_create>"
[{}] => [return $instance;] "anonymousclass.JsConstructorSubclass$1.<synthetic: ctor_create>"
[{}] => [$ctor__anonymousclass_JsConstructorSubclass_1__anonymousclass_JsConstructorSubclass__void]
[{}] => [this.$outer_this__anonymousclass_JsConstructorSubclass_1 = $outer_this;] "anonymousclass.JsConstructorSubclass$1.<init>"
[{}] => [this.$ctor__java_lang_Object__void();] "anonymousclass.JsConstructorSubclass$1.<init>"
[{}] => [$clinit]
[{}] => [$1.$clinit = () =>{};] "anonymousclass.JsConstructorSubclass$1.<clinit>"
[{}] => [$1.$loadModules();] "anonymousclass.JsConstructorSubclass$1.<clinit>"
[{}] => [j_l_Object.$clinit();] "anonymousclass.JsConstructorSubclass$1.<clinit>"
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
goog.module('anonymousclass.JsConstructorSubclass$impl');

const JsConstructorClass = goog.require('anonymousclass.JsConstructorClass$impl');
const $Util = goog.require('nativebootstrap.Util$impl');

let $1 = goog.forwardDeclare('anonymousclass.JsConstructorSubclass.$1$impl');

class JsConstructorSubclass extends JsConstructorClass {

constructor() {
JsConstructorSubclass.$clinit();
super($1.$create__anonymousclass_JsConstructorSubclass(null));
this.$ctor__anonymousclass_JsConstructorSubclass__void();
}
/** @nodts */
$ctor__anonymousclass_JsConstructorSubclass__void() {}
/** @nodts */
static $clinit() {
JsConstructorSubclass.$clinit = () =>{};
JsConstructorSubclass.$loadModules();
JsConstructorClass.$clinit();
}
/** @nodts @return {boolean} */
static $isInstance(/** ? */ instance) {
return instance instanceof JsConstructorSubclass;
}

/** @nodts */
static $loadModules() {
$1 = goog.module.get('anonymousclass.JsConstructorSubclass.$1$impl');
}
}
$Util.$setClassMetadata(JsConstructorSubclass, 'anonymousclass.JsConstructorSubclass');

exports = JsConstructorSubclass;

//# sourceMappingURL=JsConstructorSubclass.js.map
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
goog.module('anonymousclass.JsConstructorSubclass');

goog.require('anonymousclass.JsConstructorClass');
goog.require('anonymousclass.JsConstructorSubclass.$1');
goog.require('nativebootstrap.Util');

const JsConstructorSubclass = goog.require('anonymousclass.JsConstructorSubclass$impl');
exports = JsConstructorSubclass;
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[JsConstructorSubclass] => [JsConstructorSubclass]
[JsConstructorSubclass] => [constructor]
[JsConstructorSubclass] => [JsConstructorSubclass.$clinit();] "anonymousclass.JsConstructorSubclass.<init>"
[JsConstructorSubclass] => [super($1.$create__anonymousclass_JsConstructorSubclass(null));] "anonymousclass.JsConstructorSubclass.<init>"
[JsConstructorSubclass] => [this.$ctor__anonymousclass_JsConstructorSubclass__void();] "anonymousclass.JsConstructorSubclass.<init>"
[JsConstructorSubclass] => [$ctor__anonymousclass_JsConstructorSubclass__void]
[JsConstructorSubclass] => [$clinit]
[JsConstructorSubclass] => [JsConstructorSubclass.$clinit = () =>{};] "anonymousclass.JsConstructorSubclass.<clinit>"
[JsConstructorSubclass] => [JsConstructorSubclass.$loadModules();] "anonymousclass.JsConstructorSubclass.<clinit>"
[JsConstructorSubclass] => [JsConstructorClass.$clinit();] "anonymousclass.JsConstructorSubclass.<clinit>"
Loading

0 comments on commit ff9f7b4

Please sign in to comment.