From 2a511c2358400f11248c8643f7e46306bb574782 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 4 Oct 2023 11:30:06 -0700 Subject: [PATCH] [WASM] Change PropagateConstants to process JsEnum constants separately. There is an issue around ordering of PropagateConstants, string concatenation passes, and JsEnum boxing/unboxing passes: - PropagateConstants must run before StaticallyEvaluateStringConcatenation, which must run before ImplementStringConcatenation. - InsertJsEnumBoxingAndUnboxingConversions must run after ImplementStringConcatenation but before PropagateConstants, because PropagateConstants removes type information around JsEnums. To account for this, we split PropagateConstants to handle JsEnums separately, so that we can run the non-JsEnum part before JsEnum boxing/unboxing, and the JsEnum part afterwards. Note: This change is a no-op for the output PiperOrigin-RevId: 570753214 --- .../google/j2cl/transpiler/ast/AstUtils.java | 12 ++++++ .../j2cl/transpiler/backend/Backend.java | 3 ++ .../transpiler/passes/NormalizeJsEnums.java | 13 +----- .../transpiler/passes/PropagateConstants.java | 22 +++++++++- .../passes/PropagateJsEnumConstants.java | 41 +++++++++++++++++++ 5 files changed, 77 insertions(+), 14 deletions(-) create mode 100644 transpiler/java/com/google/j2cl/transpiler/passes/PropagateJsEnumConstants.java diff --git a/transpiler/java/com/google/j2cl/transpiler/ast/AstUtils.java b/transpiler/java/com/google/j2cl/transpiler/ast/AstUtils.java index 6cf3a9574e..33a733fe8e 100644 --- a/transpiler/java/com/google/j2cl/transpiler/ast/AstUtils.java +++ b/transpiler/java/com/google/j2cl/transpiler/ast/AstUtils.java @@ -1015,6 +1015,18 @@ public static boolean isJsEnumCustomValueField(MemberDescriptor memberDescriptor && memberDescriptor.getEnclosingTypeDescriptor().isJsEnum(); } + public static FieldDescriptor createJsEnumConstantFieldDescriptor( + FieldDescriptor fieldDescriptor) { + TypeDescriptor enumValueType = + getJsEnumValueFieldType(fieldDescriptor.getEnclosingTypeDescriptor().getTypeDeclaration()); + return FieldDescriptor.Builder.from(fieldDescriptor) + .setTypeDescriptor(enumValueType) + .setFinal(true) + .setCompileTimeConstant(true) + .setEnumConstant(true) + .build(); + } + /** Returns the value field for a JsEnum. */ public static TypeDescriptor getJsEnumValueFieldType(TypeDeclaration typeDeclaration) { FieldDescriptor valueFieldDescriptor = getJsEnumValueFieldDescriptor(typeDeclaration); diff --git a/transpiler/java/com/google/j2cl/transpiler/backend/Backend.java b/transpiler/java/com/google/j2cl/transpiler/backend/Backend.java index 92745afe25..ed80882460 100644 --- a/transpiler/java/com/google/j2cl/transpiler/backend/Backend.java +++ b/transpiler/java/com/google/j2cl/transpiler/backend/Backend.java @@ -142,6 +142,7 @@ import com.google.j2cl.transpiler.passes.OptimizeAutoValue; import com.google.j2cl.transpiler.passes.OptimizeEnums; import com.google.j2cl.transpiler.passes.PropagateConstants; +import com.google.j2cl.transpiler.passes.PropagateJsEnumConstants; import com.google.j2cl.transpiler.passes.PropagateNullabilityJ2kt; import com.google.j2cl.transpiler.passes.RecoverShortcutBooleanOperator; import com.google.j2cl.transpiler.passes.RemoveAssertStatements; @@ -411,6 +412,7 @@ public ImmutableList> getPassFactories(BackendOption // Propagate constants needs to run after NormalizeSwitchStatements since it introduces // field references to constant fields. PropagateConstants::new, + PropagateJsEnumConstants::new, StaticallyEvaluateStringConcatenation::new, StaticallyEvaluateStringComparison::new, ImplementStringConcatenation::new, @@ -544,6 +546,7 @@ public ImmutableList> getPassFactories(BackendOption // Propagate constants needs to run after NormalizeSwitchStatements since it introduces // field references to constant fields. PropagateConstants::new, + PropagateJsEnumConstants::new, StaticallyEvaluateStringConcatenation::new, StaticallyEvaluateStringComparison::new, ImplementStringConcatenation::new, diff --git a/transpiler/java/com/google/j2cl/transpiler/passes/NormalizeJsEnums.java b/transpiler/java/com/google/j2cl/transpiler/passes/NormalizeJsEnums.java index 3055a87a8f..f8d187e807 100644 --- a/transpiler/java/com/google/j2cl/transpiler/passes/NormalizeJsEnums.java +++ b/transpiler/java/com/google/j2cl/transpiler/passes/NormalizeJsEnums.java @@ -25,7 +25,6 @@ import com.google.j2cl.transpiler.ast.Expression; import com.google.j2cl.transpiler.ast.Field; import com.google.j2cl.transpiler.ast.FieldAccess; -import com.google.j2cl.transpiler.ast.FieldDescriptor; import com.google.j2cl.transpiler.ast.FieldDescriptor.FieldOrigin; import com.google.j2cl.transpiler.ast.JsDocCastExpression; import com.google.j2cl.transpiler.ast.MemberReference; @@ -106,18 +105,8 @@ public Node rewriteMethod(Method method) { * invocation. */ private static Field createJsEnumConstant(Field field) { - TypeDescriptor enumValueType = - AstUtils.getJsEnumValueFieldType( - field.getDescriptor().getEnclosingTypeDescriptor().getTypeDeclaration()); - FieldDescriptor closureEnumConstantFieldDescriptor = - FieldDescriptor.Builder.from(field.getDescriptor()) - .setTypeDescriptor(enumValueType) - .setFinal(true) - .setCompileTimeConstant(true) - .setEnumConstant(true) - .build(); return Field.Builder.from(field) - .setDescriptor(closureEnumConstantFieldDescriptor) + .setDescriptor(AstUtils.createJsEnumConstantFieldDescriptor(field.getDescriptor())) .setInitializer(getJsEnumConstantValue(field)) .build(); } diff --git a/transpiler/java/com/google/j2cl/transpiler/passes/PropagateConstants.java b/transpiler/java/com/google/j2cl/transpiler/passes/PropagateConstants.java index 82534abc30..b4560b119e 100644 --- a/transpiler/java/com/google/j2cl/transpiler/passes/PropagateConstants.java +++ b/transpiler/java/com/google/j2cl/transpiler/passes/PropagateConstants.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkState; import com.google.j2cl.transpiler.ast.AbstractRewriter; +import com.google.j2cl.transpiler.ast.AstUtils; import com.google.j2cl.transpiler.ast.Expression; import com.google.j2cl.transpiler.ast.Field; import com.google.j2cl.transpiler.ast.FieldAccess; @@ -71,7 +72,10 @@ private void rewriteCompileTimeConstantFieldReferences( new AbstractRewriter() { @Override public Node rewriteFieldAccess(FieldAccess fieldAccess) { - FieldDescriptor target = fieldAccess.getTarget(); + if (!shouldPropagateConstant(fieldAccess.getTarget())) { + return fieldAccess; + } + FieldDescriptor target = getConstantFieldDescriptor(fieldAccess); Expression literal = literalsByField.get(target.getDeclarationDescriptor()); if (literal == null) { return fieldAccess; @@ -84,7 +88,15 @@ public Node rewriteFieldAccess(FieldAccess fieldAccess) { }); } - private static boolean isCompileTimeConstant(Field field) { + /** Gets the relevant field descriptor for the specified {@link FieldAccess}. */ + protected FieldDescriptor getConstantFieldDescriptor(FieldAccess fieldAccess) { + return fieldAccess.getTarget(); + } + + private boolean isCompileTimeConstant(Field field) { + if (!shouldPropagateConstant(field.getDescriptor())) { + return false; + } return field.isCompileTimeConstant() // Consider final static fields that are initialized to literals to be compile time // constants. These might be driven from TypeLiterals or System.getProperty calls and it @@ -94,4 +106,10 @@ private static boolean isCompileTimeConstant(Field field) { && (field.getInitializer() instanceof TypeLiteral || field.getInitializer() instanceof StringLiteral)); } + + protected boolean shouldPropagateConstant(FieldDescriptor fieldDescriptor) { + // Skip JsEnum constants, which must be handled separately. + return !(fieldDescriptor.isEnumConstant() + && AstUtils.isNonNativeJsEnum(fieldDescriptor.getEnclosingTypeDescriptor())); + } } diff --git a/transpiler/java/com/google/j2cl/transpiler/passes/PropagateJsEnumConstants.java b/transpiler/java/com/google/j2cl/transpiler/passes/PropagateJsEnumConstants.java new file mode 100644 index 0000000000..864653aaf5 --- /dev/null +++ b/transpiler/java/com/google/j2cl/transpiler/passes/PropagateJsEnumConstants.java @@ -0,0 +1,41 @@ +/* + * Copyright 2023 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.j2cl.transpiler.passes; + +import com.google.j2cl.transpiler.ast.AstUtils; +import com.google.j2cl.transpiler.ast.FieldAccess; +import com.google.j2cl.transpiler.ast.FieldDescriptor; + +// TODO(b/303316598): This approach needs more thought for modular. With modular transpilation, we +// don't have enough information about JsEnum to propagate, in particular, the AST which includes +// the constant field values. +/** Propagates JsEnum constant fields. */ +public class PropagateJsEnumConstants extends PropagateConstants { + + @Override + protected boolean shouldPropagateConstant(FieldDescriptor fieldDescriptor) { + return fieldDescriptor.isEnumConstant() + && AstUtils.isNonNativeJsEnum(fieldDescriptor.getEnclosingTypeDescriptor()); + } + + // TODO(b/303309109) This is a workaround for NormalizeJsEnums not updating field references to + // point to the correct descriptor. Ideally, this shouldn't be needed. Clean up after rethinking + // JsEnum normalization passes. + @Override + protected FieldDescriptor getConstantFieldDescriptor(FieldAccess fieldAccess) { + return AstUtils.createJsEnumConstantFieldDescriptor(fieldAccess.getTarget()); + } +}