Skip to content

Commit 7255073

Browse files
Googlercopybara-github
Googler
authored and
copybara-github
committed
[WASM] JsInterop - reorder AddJsExportBridgesWasm - to be before PropagateCompileTimeConstants, because the latter pass removes read-only fields which we still need to generate bridges for
This changes how we handle constructors in Wasm jsinterop bridge generation. Instead of looking at the $create factory method generated by `NormalizeInstantiationThroughFactoryMethods`, we generate bridges for constructors directly (this will later be changed by NormalizeInstantiationThroughFactoryMethods to call the factory) PiperOrigin-RevId: 888206677
1 parent a26346a commit 7255073

File tree

7 files changed

+165
-115
lines changed

7 files changed

+165
-115
lines changed

‎transpiler/java/com/google/j2cl/transpiler/ast/AstUtils.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,6 +1387,11 @@ public static boolean needsWasmJsExport(MemberDescriptor memberDescriptor) {
13871387
return memberDescriptor.canBeReferencedExternally()
13881388
// TODO(b/481799839): Consider "@Wasm native ..." methods.
13891389
&& !memberDescriptor.isNative()
1390+
// Exclude native js constructors.
1391+
// TODO(b/264676817): Consider refactoring to have MethodDescriptor.isNative return true
1392+
// for native constructors, or exposing isNativeConstructor from MethodDescriptor.
1393+
&& !(memberDescriptor.isConstructor()
1394+
&& memberDescriptor.getEnclosingTypeDescriptor().isNative())
13901395
// JS members that override an already exported JS member don't need a bridge in the
13911396
// overriding type. The bridge in the overridden method does a polymorphic dispatch.
13921397
// Also exclude the generated export bridges themselves.

‎transpiler/java/com/google/j2cl/transpiler/ast/WasmExportBridgesUtils.java‎

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package com.google.j2cl.transpiler.ast;
1717

18+
import static com.google.common.base.Preconditions.checkState;
1819
import static com.google.common.collect.ImmutableList.toImmutableList;
1920

2021
import com.google.common.collect.ImmutableList;
@@ -50,17 +51,7 @@ public static Method generateBridge(
5051
.setMethodDescriptor(bridgeMethodDescriptor)
5152
.setParameters(parameters)
5253
.addStatements(
53-
convertReturnIfNeeded(
54-
AstUtils.createForwardingStatement(
55-
sourcePosition,
56-
/* qualifier= */ methodDescriptor.isStatic()
57-
? null
58-
: new ThisReference(methodDescriptor.getEnclosingTypeDescriptor()),
59-
methodDescriptor,
60-
/* isStaticDispatch= */ methodDescriptor.isStatic(),
61-
arguments,
62-
returnType),
63-
returnType))
54+
createBridgeTargetInvocation(methodDescriptor, sourcePosition, arguments, returnType))
6455
.setJsDocDescription(
6556
origin.isWasmEntryPoint() ? "Wasm entry point forwarding method." : null)
6657
.setSourcePosition(sourcePosition)
@@ -89,6 +80,36 @@ private static Statement convertReturnIfNeeded(
8980
return statement;
9081
}
9182

83+
private static Statement createBridgeTargetInvocation(
84+
MethodDescriptor methodDescriptor,
85+
SourcePosition sourcePosition,
86+
List<Expression> arguments,
87+
TypeDescriptor returnTypeDescriptor) {
88+
if (methodDescriptor.isConstructor()) {
89+
checkState(
90+
returnTypeDescriptor.isSameBaseType(methodDescriptor.getEnclosingTypeDescriptor()));
91+
return ReturnStatement.newBuilder()
92+
.setExpression(
93+
NewInstance.Builder.from(methodDescriptor)
94+
.setArguments(AstUtils.maybePackageVarargs(methodDescriptor, arguments))
95+
.build())
96+
.setSourcePosition(sourcePosition)
97+
.build();
98+
}
99+
100+
return convertReturnIfNeeded(
101+
AstUtils.createForwardingStatement(
102+
sourcePosition,
103+
/* qualifier= */ methodDescriptor.isStatic()
104+
? null
105+
: new ThisReference(methodDescriptor.getEnclosingTypeDescriptor()),
106+
methodDescriptor,
107+
/* isStaticDispatch= */ methodDescriptor.isStatic(),
108+
arguments,
109+
returnTypeDescriptor),
110+
returnTypeDescriptor);
111+
}
112+
92113
private static MethodDescriptor createBridgeDescriptor(
93114
MethodDescriptor descriptor, MethodDescriptor.MethodOrigin origin) {
94115
MethodDescriptor.Builder builder =
@@ -99,10 +120,17 @@ private static MethodDescriptor createBridgeDescriptor(
99120
.updateParameterTypeDescriptors(
100121
descriptor.getParameterTypeDescriptors().stream()
101122
.map(WasmExportBridgesUtils::replaceStringWithNativeString)
102-
.collect(toImmutableList()));
103-
if (!origin.isWasmEntryPoint()) {
104-
builder.setOriginalJsInfo(
105-
descriptor.getJsInfo().toBuilder().setJsName(descriptor.getSimpleJsName()).build());
123+
.collect(toImmutableList()))
124+
// Copy over the JsInfo from the descriptor. This allows the bridge to retain the JsInfo
125+
// if, for example, it is inherited; we otherwise lose the inherited JsInfo because we
126+
// lose override information. It also preserves the CONSTRUCTOR member type if the
127+
// original member is a constructor.
128+
// TODO(b/493656775): Consider calling `makeBridge` here instead and getting JsInfo from
129+
// the mangling descriptor/bridge origin.
130+
.setOriginalJsInfo(descriptor.getJsInfo());
131+
if (descriptor.isConstructor()) {
132+
// Change constructors to static factory methods.
133+
builder.setStatic(true).setConstructor(false);
106134
}
107135
return builder.build();
108136
}

‎transpiler/java/com/google/j2cl/transpiler/backend/Backend.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -509,8 +509,14 @@ public ImmutableList<Supplier<NormalizationPass>> getPassFactories(BackendOption
509509
RewriteUnaryExpressions::new,
510510
AddSwitchExpressionsExhaustivenessCheck::new,
511511
NormalizeSwitchConstructs::new,
512+
() ->
513+
new AddJsExportBridgesWasm(
514+
/* enableCustomDescriptorsJsInterop= */ options
515+
.getEnableWasmCustomDescriptorsJsInterop()),
512516
// Propagate constants needs to run after NormalizeSwitchStatements since it introduces
513517
// field references to constant fields.
518+
// It must also run after AddJsExportBridgesWasm so we don't remove fields for which it
519+
// needs to generate bridges.
514520
PropagateCompileTimeConstants::new,
515521
StaticallyEvaluateStringConcatenation::new,
516522
StaticallyEvaluateStringComparison::new,
@@ -547,10 +553,6 @@ public ImmutableList<Supplier<NormalizationPass>> getPassFactories(BackendOption
547553
// extracted. After extracting qualifiers, we must again normalize multi-expressions.
548554
ExtractNonIdempotentExpressions::new,
549555
NormalizeMultiExpressions::new,
550-
() ->
551-
new AddJsExportBridgesWasm(
552-
/* enableCustomDescriptorsJsInterop= */ options
553-
.getEnableWasmCustomDescriptorsJsInterop()),
554556
ImplementFinallyViaControlFlow::new,
555557

556558
// Needs to run at the end as the types in the ast will be invalid after the pass.

‎transpiler/java/com/google/j2cl/transpiler/backend/wasm/SummaryBuilder.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,14 @@ private JsInfo getJsInfo(Type type) {
152152
JsMemberInfo.newBuilder()
153153
.setKind(
154154
switch (methodDescriptor.getJsInfo().getJsMemberType()) {
155-
case METHOD ->
156-
methodDescriptor.getOrigin().isWasmJsConstructorExport()
157-
? JsMemberInfo.Kind.CONSTRUCTOR
158-
: JsMemberInfo.Kind.METHOD;
155+
case CONSTRUCTOR -> JsMemberInfo.Kind.CONSTRUCTOR;
156+
case METHOD -> JsMemberInfo.Kind.METHOD;
159157
// Should not happen, we should be handling all types.
160158
// TODO(b/458472428) Consider removing default when all types are handled.
161-
default -> throw new AssertionError();
159+
default ->
160+
throw new AssertionError(
161+
"Unexpected JsMemberType: "
162+
+ methodDescriptor.getJsInfo().getJsMemberType().name());
162163
})
163164
.setWasmName(environment.getMethodImplementationName(methodDescriptor))
164165
.setJsName(methodDescriptor.getSimpleJsName())

‎transpiler/java/com/google/j2cl/transpiler/passes/AddJsExportBridgesWasm.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,17 @@ public void applyTo(Library library) {
6969

7070
private static boolean shouldGenerateBridge(MethodDescriptor methodDescriptor) {
7171
return AstUtils.needsWasmJsExport(methodDescriptor)
72-
// We don't expose constructors directly. Instead, the factory method is exported.
73-
&& !methodDescriptor.isConstructor()
7472
// TODO(b/458472428): Support JsProperty/Getter/Setter.
75-
&& methodDescriptor.isJsMethod();
73+
&& (methodDescriptor.isJsConstructor() || methodDescriptor.isJsMethod());
7674
}
7775

7876
private static MethodDescriptor.MethodOrigin getBridgeOrigin(MethodDescriptor descriptor) {
79-
return descriptor.getOrigin() == MethodDescriptor.MethodOrigin.SYNTHETIC_FACTORY_FOR_CONSTRUCTOR
80-
? MethodDescriptor.MethodOrigin.SYNTHETIC_WASM_JS_CONSTRUCTOR_EXPORT
81-
: MethodDescriptor.MethodOrigin.SYNTHETIC_WASM_JS_METHOD_EXPORT;
77+
return switch (descriptor.getJsInfo().getJsMemberType()) {
78+
case CONSTRUCTOR -> MethodDescriptor.MethodOrigin.SYNTHETIC_WASM_JS_CONSTRUCTOR_EXPORT;
79+
case METHOD -> MethodDescriptor.MethodOrigin.SYNTHETIC_WASM_JS_METHOD_EXPORT;
80+
default ->
81+
throw new AssertionError(
82+
"Unexpected JsMemberType: " + descriptor.getJsInfo().getJsMemberType().name());
83+
};
8284
}
8385
}

‎transpiler/javatests/com/google/j2cl/readable/java/jsmethod/output_wasm/contents.wat.txt‎

Lines changed: 45 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,21 @@
376376
)
377377
(elem declare func $m_m__java_lang_Object__void_$pp_jsmethod@jsmethod.JsMethodExample.Base)
378378

379+
;;; void Base.m(T arg0)
380+
(func $js_export_m__java_lang_Object__void_$pp_jsmethod@jsmethod.JsMethodExample.Base
381+
(type $function.js_export_m__java_lang_Object__void_$pp_jsmethod)
382+
(param $this.untyped (ref $java.lang.Object))
383+
(param $arg0 (ref null $java.lang.Object))
384+
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:38:9
385+
(local $this (ref null $jsmethod.JsMethodExample.Base))
386+
(local.set $this (ref.cast (ref $jsmethod.JsMethodExample.Base) (local.get $this.untyped)))
387+
(block
388+
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:38:9
389+
(call_ref $function.m_m__java_lang_Object__void_$pp_jsmethod (ref.as_non_null (local.get $this))(local.get $arg0)(struct.get $jsmethod.JsMethodExample.Base.vtable $m_m__java_lang_Object__void_$pp_jsmethod (ref.get_desc $jsmethod.JsMethodExample.Base(local.get $this))))
390+
)
391+
)
392+
(elem declare func $js_export_m__java_lang_Object__void_$pp_jsmethod@jsmethod.JsMethodExample.Base)
393+
379394
;;; void Base.$clinit()
380395
(func $$clinit__void_<once>_@jsmethod.JsMethodExample.Base
381396
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:36:24
@@ -421,21 +436,6 @@
421436
)
422437
(elem declare func $$getClassImpl__java_lang_Class@jsmethod.JsMethodExample.Base)
423438

424-
;;; void Base.m(T arg0)
425-
(func $js_export_m__java_lang_Object__void_$pp_jsmethod@jsmethod.JsMethodExample.Base
426-
(type $function.js_export_m__java_lang_Object__void_$pp_jsmethod)
427-
(param $this.untyped (ref $java.lang.Object))
428-
(param $arg0 (ref null $java.lang.Object))
429-
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:38:9
430-
(local $this (ref null $jsmethod.JsMethodExample.Base))
431-
(local.set $this (ref.cast (ref $jsmethod.JsMethodExample.Base) (local.get $this.untyped)))
432-
(block
433-
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:38:9
434-
(call_ref $function.m_m__java_lang_Object__void_$pp_jsmethod (ref.as_non_null (local.get $this))(local.get $arg0)(struct.get $jsmethod.JsMethodExample.Base.vtable $m_m__java_lang_Object__void_$pp_jsmethod (ref.get_desc $jsmethod.JsMethodExample.Base(local.get $this))))
435-
)
436-
)
437-
(elem declare func $js_export_m__java_lang_Object__void_$pp_jsmethod@jsmethod.JsMethodExample.Base)
438-
439439
;;; void InterfaceExposingJsMethods.m()
440440
(func $m_m__void@jsmethod.JsMethodExample.InterfaceExposingJsMethods
441441
(param $this (ref null $java.lang.Object))
@@ -894,6 +894,21 @@
894894
)
895895
(elem declare func $m_m__java_lang_Object__void_$pp_jsmethod@jsmethod.JsMethodExample.SubGenericsJsType)
896896

897+
;;; void SubGenericsJsType.m(T arg0)
898+
(func $js_export_m__java_lang_Object__void@jsmethod.JsMethodExample.SubGenericsJsType
899+
(type $function.js_export_m__java_lang_Object__void)
900+
(param $this.untyped (ref $java.lang.Object))
901+
(param $arg0 (ref null $java.lang.Object))
902+
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:61:16
903+
(local $this (ref null $jsmethod.JsMethodExample.SubGenericsJsType))
904+
(local.set $this (ref.cast (ref $jsmethod.JsMethodExample.SubGenericsJsType) (local.get $this.untyped)))
905+
(block
906+
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:61:16
907+
(call_ref $function.m_m__java_lang_Object__void (ref.as_non_null (local.get $this))(local.get $arg0)(struct.get $jsmethod.JsMethodExample.SubGenericsJsType.vtable $m_m__java_lang_Object__void (ref.get_desc $jsmethod.JsMethodExample.SubGenericsJsType(local.get $this))))
908+
)
909+
)
910+
(elem declare func $js_export_m__java_lang_Object__void@jsmethod.JsMethodExample.SubGenericsJsType)
911+
897912
;;; void SubGenericsJsType.$clinit()
898913
(func $$clinit__void_<once>_@jsmethod.JsMethodExample.SubGenericsJsType
899914
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:60:15
@@ -939,21 +954,6 @@
939954
)
940955
(elem declare func $$getClassImpl__java_lang_Class@jsmethod.JsMethodExample.SubGenericsJsType)
941956

942-
;;; void SubGenericsJsType.m(T arg0)
943-
(func $js_export_m__java_lang_Object__void@jsmethod.JsMethodExample.SubGenericsJsType
944-
(type $function.js_export_m__java_lang_Object__void)
945-
(param $this.untyped (ref $java.lang.Object))
946-
(param $arg0 (ref null $java.lang.Object))
947-
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:61:16
948-
(local $this (ref null $jsmethod.JsMethodExample.SubGenericsJsType))
949-
(local.set $this (ref.cast (ref $jsmethod.JsMethodExample.SubGenericsJsType) (local.get $this.untyped)))
950-
(block
951-
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:61:16
952-
(call_ref $function.m_m__java_lang_Object__void (ref.as_non_null (local.get $this))(local.get $arg0)(struct.get $jsmethod.JsMethodExample.SubGenericsJsType.vtable $m_m__java_lang_Object__void (ref.get_desc $jsmethod.JsMethodExample.SubGenericsJsType(local.get $this))))
953-
)
954-
)
955-
(elem declare func $js_export_m__java_lang_Object__void@jsmethod.JsMethodExample.SubGenericsJsType)
956-
957957
;;; SubJsTypeWithRenamedJsMethod SubJsTypeWithRenamedJsMethod.$create()
958958
(func $$create__@jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod
959959
(result (ref null $jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod))
@@ -1009,6 +1009,21 @@
10091009
)
10101010
(elem declare func $m_m__java_lang_Object__void_$pp_jsmethod@jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod)
10111011

1012+
;;; void SubJsTypeWithRenamedJsMethod.m(NativeString arg0)
1013+
(func $js_export_m__java_lang_String_NativeString__void@jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod
1014+
(type $function.js_export_m__java_lang_String_NativeString__void)
1015+
(param $this.untyped (ref $java.lang.Object))
1016+
(param $arg0 (ref null string))
1017+
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:67:16
1018+
(local $this (ref null $jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod))
1019+
(local.set $this (ref.cast (ref $jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod) (local.get $this.untyped)))
1020+
(block
1021+
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:67:16
1022+
(call_ref $function.m_m__java_lang_String__void (ref.as_non_null (local.get $this))(call $m_fromJsString__java_lang_String_NativeString__java_lang_String@java.lang.String (local.get $arg0))(struct.get $jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod.vtable $m_m__java_lang_String__void (ref.get_desc $jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod(local.get $this))))
1023+
)
1024+
)
1025+
(elem declare func $js_export_m__java_lang_String_NativeString__void@jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod)
1026+
10121027
;;; void SubJsTypeWithRenamedJsMethod.$clinit()
10131028
(func $$clinit__void_<once>_@jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod
10141029
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:65:15
@@ -1054,21 +1069,6 @@
10541069
)
10551070
(elem declare func $$getClassImpl__java_lang_Class@jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod)
10561071

1057-
;;; void SubJsTypeWithRenamedJsMethod.m(NativeString arg0)
1058-
(func $js_export_m__java_lang_String_NativeString__void@jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod
1059-
(type $function.js_export_m__java_lang_String_NativeString__void)
1060-
(param $this.untyped (ref $java.lang.Object))
1061-
(param $arg0 (ref null string))
1062-
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:67:16
1063-
(local $this (ref null $jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod))
1064-
(local.set $this (ref.cast (ref $jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod) (local.get $this.untyped)))
1065-
(block
1066-
;;@ transpiler/javatests/com/google/j2cl/readable/java/jsmethod/readable-j2wasm.js/jsmethod/JsMethodExample.java:67:16
1067-
(call_ref $function.m_m__java_lang_String__void (ref.as_non_null (local.get $this))(call $m_fromJsString__java_lang_String_NativeString__java_lang_String@java.lang.String (local.get $arg0))(struct.get $jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod.vtable $m_m__java_lang_String__void (ref.get_desc $jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod(local.get $this))))
1068-
)
1069-
)
1070-
(elem declare func $js_export_m__java_lang_String_NativeString__void@jsmethod.JsMethodExample.SubJsTypeWithRenamedJsMethod)
1071-
10721072
;;; ExposesOverrideableJsMethod ExposesOverrideableJsMethod.$create()
10731073
(func $$create__@jsmethod.JsMethodExample.ExposesOverrideableJsMethod
10741074
(result (ref null $jsmethod.JsMethodExample.ExposesOverrideableJsMethod))

0 commit comments

Comments
 (0)