From 832c88e35b220a7d22f75fe2dcdcfcb89504be17 Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Wed, 6 Sep 2023 16:21:20 -0700 Subject: [PATCH] [WASM] Noop refactor JsImportsGenerator in preparation for modular compilation. PiperOrigin-RevId: 563246908 --- .../backend/wasm/JsImportsGenerator.java | 278 +++++++++--------- .../backend/wasm/WasmOutputsGenerator.java | 8 +- 2 files changed, 150 insertions(+), 136 deletions(-) diff --git a/transpiler/java/com/google/j2cl/transpiler/backend/wasm/JsImportsGenerator.java b/transpiler/java/com/google/j2cl/transpiler/backend/wasm/JsImportsGenerator.java index 843b8093c7..8adb18ea90 100644 --- a/transpiler/java/com/google/j2cl/transpiler/backend/wasm/JsImportsGenerator.java +++ b/transpiler/java/com/google/j2cl/transpiler/backend/wasm/JsImportsGenerator.java @@ -43,10 +43,6 @@ final class JsImportsGenerator { /** Top-level module name in the imports map containing all generated imports. */ public static final String MODULE = "imports"; - public static Imports collectImports(Library library, Problems problems) { - return new JsImportsGenerator(problems).collectJsImports(library); - } - @AutoValue public abstract static class Imports { public abstract ImmutableMap getMethodImports(); @@ -60,43 +56,24 @@ public static Imports create( } } - private final Output output; - private final SourceBuilder builder = new SourceBuilder(); - - private final WasmGenerationEnvironment environment; - private final Problems problems; - - /** A minimal closure generation environment to reuse {@code ClosureTypesGenerator}. */ - private final ClosureGenerationEnvironment closureEnvironment = - new ClosureGenerationEnvironment(ImmutableSet.of(), ImmutableMap.of()) { - @Override - public String aliasForType(TypeDeclaration typeDeclaration) { - return JsMethodImport.getJsTypeName(typeDeclaration); - } - }; - - public JsImportsGenerator( - Output output, WasmGenerationEnvironment environment, Problems problems) { - this.output = output; - this.environment = environment; - this.problems = problems; + /** Collects JavaScript imports that are referenced in the library. */ + public static Imports collectImports(Library library, Problems problems) { + ImmutableSet.Builder moduleImports = ImmutableSet.builder(); + ImmutableMap.Builder methodImports = ImmutableMap.builder(); + library.accept(new ImportCollector(problems, methodImports, moduleImports)); + return Imports.create(methodImports.buildOrThrow(), moduleImports.build()); } - private JsImportsGenerator(Problems problems) { - this.output = null; - this.environment = null; - this.problems = problems; - } - - public void generateOutputs(Library library) { - emitRequires(); - emitJsImports(); - builder.newLine(); // Ends in a new line for human readability. - output.write("imports.txt", builder.build()); + /** Generates the JavaScript code to support the imports. */ + public static void generateOutputs(Output output, Imports imports) { + JsImportsGenerator importsGenerator = new JsImportsGenerator(imports); + importsGenerator.emitRequires(); + importsGenerator.emitJsImports(); + importsGenerator.writeOutput(output); } private void emitRequires() { - environment.getJsImports().getModuleImports().stream() + imports.getModuleImports().stream() .sorted() .forEach( imp -> { @@ -132,7 +109,7 @@ private void emitJsImports() { builder.append(String.format("'%s': ", MODULE)); builder.openBrace(); emitHardCodedJsImports(); - environment.getJsImports().getMethodImports().values().stream() + imports.getMethodImports().values().stream() .distinct() .sorted(comparing(JsMethodImport::getImportKey)) .forEach( @@ -219,101 +196,118 @@ private void emitParameter(Variable parameter) { parameter.getName())); } - private Imports collectJsImports(Library library) { - ImmutableSet.Builder moduleImports = ImmutableSet.builder(); - Map methodImportsByName = new HashMap<>(); - ImmutableMap.Builder methodImports = ImmutableMap.builder(); - library.accept( - new AbstractVisitor() { - @Override - public void exitType(Type type) { - collectModuleImports(type.getTypeDescriptor()); - } - - @Override - public void exitMethod(Method method) { - MethodDescriptor methodDescriptor = method.getDescriptor(); - if (!shouldGenerateImport(methodDescriptor)) { - return; - } - - addMethodImport( - JsMethodImport.newBuilder() - .setBaseImportKey(JsMethodImport.getJsImportName(methodDescriptor)) - .setSignature( - JsMethodImport.computeSignature(methodDescriptor, closureEnvironment)) - .setMethod(method) - .build()); - - // Collect imports for JsDoc. - addModuleImports(methodDescriptor); - } - - private void addMethodImport(JsMethodImport newImport) { - JsMethodImport newOrExistingImport = - methodImportsByName.compute( - newImport.getImportKey(), - (keyUnused, existingImport) -> { - if (existingImport == null) { - return newImport; - } - - if (!JsMethodImport.isCompatible(existingImport, newImport)) { - problems.error( - newImport.getMethod().getSourcePosition(), - "Native methods '%s' and '%s', importing JavaScript method '%s', do not" - + " match. Both or neither must be constructors, static, or JS" - + " properties (b/283986050).", - existingImport.getMethod().getReadableDescription(), - newImport.getMethod().getReadableDescription(), - existingImport.getImportKey()); - } - - if (!newImport.getSignature().equals(existingImport.getSignature()) - // Signature doesn't matter if the method import is emitted as a method - // reference. Exclude these in this check. - && !(newImport.emitAsMethodReference() - && existingImport.emitAsMethodReference())) { - problems.error( - newImport.getMethod().getSourcePosition(), - "Native methods '%s' and '%s', importing JavaScript method '%s', have" - + " different parameter types ('%s' vs '%s'), currently disallowed" - + " due to performance concerns (b/279081023).", - existingImport.getMethod().getReadableDescription(), - newImport.getMethod().getReadableDescription(), - existingImport.getImportKey(), - existingImport.getSignature(), - newImport.getSignature()); - } - return existingImport; - }); - methodImports.put(newImport.getMethod().getDescriptor(), newOrExistingImport); - } - - private void addModuleImports(MethodDescriptor methodDescriptor) { - if (!methodDescriptor.isExtern()) { - moduleImports.add(methodDescriptor.getJsNamespace()); - } - - methodDescriptor.getParameterTypeDescriptors().forEach(this::collectModuleImports); - } - - private void collectModuleImports(TypeDescriptor typeDescriptor) { - if (!(typeDescriptor instanceof DeclaredTypeDescriptor)) { - return; - } - DeclaredTypeDescriptor declaredTypeDescriptor = (DeclaredTypeDescriptor) typeDescriptor; - TypeDeclaration typeDeclaration = declaredTypeDescriptor.getTypeDeclaration(); - if (!typeDeclaration.isNative() || typeDeclaration.isExtern()) { - return; - } - moduleImports.add(typeDeclaration.getEnclosingModule().getQualifiedJsName()); - for (TypeDescriptor t : declaredTypeDescriptor.getTypeArgumentDescriptors()) { - collectModuleImports(t); - } - } - }); - return Imports.create(methodImports.buildOrThrow(), moduleImports.build()); + private void writeOutput(Output output) { + builder.newLine(); // Ends in a new line for human readability. + output.write("imports.txt", builder.build()); + } + + private static class ImportCollector extends AbstractVisitor { + private final Problems problems; + private final ImmutableMap.Builder methodImports; + private final ImmutableSet.Builder moduleImports; + private final Map methodImportsByName = new HashMap<>(); + private final ClosureGenerationEnvironment closureEnvironment = + createNominalClosureEnvironment(); + + public ImportCollector( + Problems problems, + ImmutableMap.Builder methodImports, + ImmutableSet.Builder moduleImports) { + this.problems = problems; + this.methodImports = methodImports; + this.moduleImports = moduleImports; + } + + @Override + public void exitType(Type type) { + collectModuleImports(type.getTypeDescriptor()); + } + + @Override + public void exitMethod(Method method) { + MethodDescriptor methodDescriptor = method.getDescriptor(); + if (!shouldGenerateImport(methodDescriptor)) { + return; + } + + addMethodImport( + JsMethodImport.newBuilder() + .setBaseImportKey(JsMethodImport.getJsImportName(methodDescriptor)) + .setSignature(JsMethodImport.computeSignature(methodDescriptor, closureEnvironment)) + .setMethod(method) + .build()); + + // Collect imports for JsDoc. + addModuleImports(methodDescriptor); + } + + private void addMethodImport(JsMethodImport newImport) { + JsMethodImport newOrExistingImport = + methodImportsByName.compute( + newImport.getImportKey(), + (keyUnused, existingImport) -> { + if (existingImport == null) { + return newImport; + } + + checkForConflicts(newImport, existingImport, problems); + return existingImport; + }); + methodImports.put(newImport.getMethod().getDescriptor(), newOrExistingImport); + } + + private void addModuleImports(MethodDescriptor methodDescriptor) { + if (!methodDescriptor.isExtern()) { + moduleImports.add(methodDescriptor.getJsNamespace()); + } + + methodDescriptor.getParameterTypeDescriptors().forEach(this::collectModuleImports); + } + + private void collectModuleImports(TypeDescriptor typeDescriptor) { + if (!(typeDescriptor instanceof DeclaredTypeDescriptor)) { + return; + } + DeclaredTypeDescriptor declaredTypeDescriptor = (DeclaredTypeDescriptor) typeDescriptor; + TypeDeclaration typeDeclaration = declaredTypeDescriptor.getTypeDeclaration(); + if (!typeDeclaration.isNative() || typeDeclaration.isExtern()) { + return; + } + moduleImports.add(typeDeclaration.getEnclosingModule().getQualifiedJsName()); + for (TypeDescriptor t : declaredTypeDescriptor.getTypeArgumentDescriptors()) { + collectModuleImports(t); + } + } + } + + private static void checkForConflicts( + JsMethodImport newImport, JsMethodImport existingImport, Problems problems) { + if (!JsMethodImport.isCompatible(existingImport, newImport)) { + problems.error( + newImport.getMethod().getSourcePosition(), + "Native methods '%s' and '%s', importing JavaScript method '%s', do not" + + " match. Both or neither must be constructors, static, or JS" + + " properties (b/283986050).", + existingImport.getMethod().getReadableDescription(), + newImport.getMethod().getReadableDescription(), + existingImport.getImportKey()); + } + + if (!newImport.getSignature().equals(existingImport.getSignature()) + // Signature doesn't matter if the method import is emitted as a method + // reference. Exclude these in this check. + && !(newImport.emitAsMethodReference() && existingImport.emitAsMethodReference())) { + problems.error( + newImport.getMethod().getSourcePosition(), + "Native methods '%s' and '%s', importing JavaScript method '%s', have" + + " different parameter types ('%s' vs '%s'), currently disallowed" + + " due to performance concerns (b/279081023).", + existingImport.getMethod().getReadableDescription(), + newImport.getMethod().getReadableDescription(), + existingImport.getImportKey(), + existingImport.getSignature(), + newImport.getSignature()); + } } private static boolean shouldGenerateImport(MethodDescriptor methodDescriptor) { @@ -342,4 +336,24 @@ private static boolean isNativeMethod(MethodDescriptor methodDescriptor) { || (methodDescriptor.getEnclosingTypeDescriptor().isNative() && methodDescriptor.isConstructor()); } + + /** Creates a minimal closure generation environment to reuse {@code ClosureTypesGenerator}. */ + private static ClosureGenerationEnvironment createNominalClosureEnvironment() { + return new ClosureGenerationEnvironment(ImmutableSet.of(), ImmutableMap.of()) { + @Override + public String aliasForType(TypeDeclaration typeDeclaration) { + return JsMethodImport.getJsTypeName(typeDeclaration); + } + }; + } + + private final SourceBuilder builder = new SourceBuilder(); + private final Imports imports; + + /** A minimal closure generation environment to reuse {@code ClosureTypesGenerator}. */ + private final ClosureGenerationEnvironment closureEnvironment = createNominalClosureEnvironment(); + + private JsImportsGenerator(Imports imports) { + this.imports = imports; + } } diff --git a/transpiler/java/com/google/j2cl/transpiler/backend/wasm/WasmOutputsGenerator.java b/transpiler/java/com/google/j2cl/transpiler/backend/wasm/WasmOutputsGenerator.java index 22e66b3872..eba903fb68 100644 --- a/transpiler/java/com/google/j2cl/transpiler/backend/wasm/WasmOutputsGenerator.java +++ b/transpiler/java/com/google/j2cl/transpiler/backend/wasm/WasmOutputsGenerator.java @@ -102,7 +102,7 @@ public void generateModularOutput(Library library) { emitDataSegments(library); outputIfNotEmpty("data.wat"); - generateJsImportsFile(library); + generateJsImportsFile(); } private void outputIfNotEmpty(String path) { @@ -118,7 +118,7 @@ private void outputIfNotEmpty(String path) { public void generateMonolithicOutput(Library library) { copyJavaSources(library); generateWasmModule(library); - generateJsImportsFile(library); + generateJsImportsFile(); } private void copyJavaSources(Library library) { @@ -919,7 +919,7 @@ private void emitEndCodeComment(String commentId) { builder.append(";;; End of code for " + commentId); } - private void generateJsImportsFile(Library library) { - new JsImportsGenerator(output, environment, problems).generateOutputs(library); + private void generateJsImportsFile() { + JsImportsGenerator.generateOutputs(output, environment.getJsImports()); } }