[api] Make Module and Sections immutable with Builders (#460)

This is part of the effort toward #441 --------- Co-authored-by: David Phillips <[email protected]>
dylibso · Aug 14, 2024 · bd17cf3 · bd17cf3
1 parent bef1801
commit bd17cf3
Show file tree

Hide file tree

Showing 44 changed files with 737 additions and 775 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -91,6 +91,13 @@ jobs:
           script: aot/README.md
         env:
           JBANG_REPO: "${{ github.workspace }}/repository"
+      # Test WASM Readme
+      - name: jbang
+        uses: jbangdev/jbang-action@9f8c55e0a2b6b297711162b20c209c5e07076e59 # tag=v0.117.1
+        with:
+          script: wasm/README.md
+        env:
+          JBANG_REPO: "${{ github.workspace }}/repository"
 
   test-results:
     name: Test Results

diff --git a/README.md b/README.md
@@ -114,11 +114,12 @@ Now let's load this module and instantiate it:
 import com.dylibso.chicory.runtime.ExportFunction;
 import com.dylibso.chicory.wasm.types.Value;
 import com.dylibso.chicory.wasm.Module;
+import com.dylibso.chicory.wasm.Parser;
 import com.dylibso.chicory.runtime.Instance;
 import java.io.File;
 
 // point this to your path on disk
-Module module = Module.builder(new File("./factorial.wasm")).build();
+Module module = Parser.parse(new File("./factorial.wasm"));
 Instance instance = Instance.builder(module).build();
 ```
 
@@ -171,7 +172,7 @@ copyFileFromWasmCorpus("count_vowels.rs.wasm", "count_vowels.wasm");
 Build and instantiate this module:
 
 ```java
-Instance instance = Instance.builder(Module.builder(new File("./count_vowels.wasm")).build()).build();
+Instance instance = Instance.builder(Parser.parse(new File("./count_vowels.wasm"))).build();
 ExportFunction countVowels = instance.export("count_vowels");
 ```
 
@@ -278,7 +279,7 @@ Now we just need to pass this host function in during our instantiation phase:
 ```java
 import com.dylibso.chicory.runtime.HostImports;
 var imports = new HostImports(new HostFunction[] {func});
-var instance = Instance.builder(Module.builder(new File("./logger.wasm")).build()).withHostImports(imports).build();
+var instance = Instance.builder(Parser.parse(new File("./logger.wasm"))).withHostImports(imports).build();
 var logIt = instance.export("logIt");
 logIt.apply();
 // should print "Hello, World!" 10 times

diff --git a/aot/README.md b/aot/README.md
@@ -14,10 +14,10 @@ To enable use the AotMachine factory when building the module:
 ```java
 // ...
 
-import com.dylibso.chicory.wasm.Module;
+import com.dylibso.chicory.wasm.Parser;
 import com.dylibso.chicory.runtime.Instance;
 import com.dylibso.chicory.aot.AotMachine;
 // ...
 var is = ClassLoader.getSystemClassLoader().getResourceAsStream("compiled/basic.c.wasm");
-Instance.builder(Module.builder(is).build()).withMachineFactory(AotMachine::new).build();
+Instance.builder(Parser.parse(is)).withMachineFactory(AotMachine::new).build();
 ```
diff --git a/aot/src/test/java/com/dylibso/chicory/approvals/ApprovalTest.java b/aot/src/test/java/com/dylibso/chicory/approvals/ApprovalTest.java
@@ -2,7 +2,7 @@
 
 import com.dylibso.chicory.aot.AotMachine;
 import com.dylibso.chicory.runtime.Instance;
-import com.dylibso.chicory.wasm.Module;
+import com.dylibso.chicory.wasm.Parser;
 import java.io.ByteArrayOutputStream;
 import java.io.PrintWriter;
 import org.approvaltests.Approvals;
@@ -65,10 +65,9 @@ public void verifyTrap() {
     private static void verifyGeneratedBytecode(String name) {
         var instance =
                 Instance.builder(
-                                Module.builder(
-                                                ClassLoader.getSystemClassLoader()
-                                                        .getResourceAsStream("compiled/" + name))
-                                        .build())
+                                Parser.parse(
+                                        ClassLoader.getSystemClassLoader()
+                                                .getResourceAsStream("compiled/" + name)))
                         .withImportValidation(false)
                         .withMachineFactory(AotMachine::new)
                         .withStart(false)

diff --git a/aot/src/test/java/com/dylibso/chicory/testing/TestModule.java b/aot/src/test/java/com/dylibso/chicory/testing/TestModule.java
@@ -5,7 +5,7 @@
 import com.dylibso.chicory.runtime.Instance;
 import com.dylibso.chicory.wabt.Wat2Wasm;
 import com.dylibso.chicory.wasm.Module;
-import com.dylibso.chicory.wasm.ModuleType;
+import com.dylibso.chicory.wasm.Parser;
 import com.dylibso.chicory.wasm.exceptions.MalformedException;
 import java.io.File;
 
@@ -21,10 +21,6 @@ public TestModule(Module module) {
         this.module = module;
     }
 
-    public static TestModule of(File file) {
-        return of(file, ModuleType.BINARY);
-    }
-
     public static TestModule of(Module module) {
         return new TestModule(module);
     }
@@ -54,18 +50,18 @@ public static TestModule of(Module module) {
                     + "alignment "
                     + "multiple start sections";
 
-    public static TestModule of(File file, ModuleType moduleType) {
-        if (moduleType == ModuleType.TEXT) {
+    public static TestModule of(File file) {
+        if (file.getName().endsWith(".wat")) {
             byte[] parsed;
             try {
                 parsed = Wat2Wasm.parse(file);
             } catch (Exception e) {
                 throw new MalformedException(
                         e.getMessage() + HACK_MATCH_ALL_MALFORMED_EXCEPTION_TEXT);
             }
-            return of(Module.builder(parsed).build());
+            return of(Parser.parse(parsed));
         }
-        return of(Module.builder(file).build());
+        return of(Parser.parse(file));
     }
 
     public Instance build() {

diff --git a/cli/src/main/java/com/dylibso/chicory/cli/Cli.java b/cli/src/main/java/com/dylibso/chicory/cli/Cli.java
@@ -5,7 +5,7 @@
 import com.dylibso.chicory.runtime.Instance;
 import com.dylibso.chicory.wasi.WasiOptions;
 import com.dylibso.chicory.wasi.WasiPreview1;
-import com.dylibso.chicory.wasm.Module;
+import com.dylibso.chicory.wasm.Parser;
 import com.dylibso.chicory.wasm.types.Value;
 import java.io.ByteArrayInputStream;
 import java.io.File;
@@ -57,7 +57,7 @@ public void run() {
             throw new RuntimeException(e);
         }
         var logger = new SystemLogger();
-        var module = Module.builder(file).build();
+        var module = Parser.parse(file);
         var imports =
                 wasi
                         ? new HostImports(

diff --git a/fuzz/src/test/java/com/dylibso/chicory/fuzz/FuzzTest.java b/fuzz/src/test/java/com/dylibso/chicory/fuzz/FuzzTest.java
@@ -8,7 +8,7 @@
 import com.dylibso.chicory.log.Logger;
 import com.dylibso.chicory.log.SystemLogger;
 import com.dylibso.chicory.runtime.Instance;
-import com.dylibso.chicory.wasm.Module;
+import com.dylibso.chicory.wasm.Parser;
 import com.dylibso.chicory.wasm.types.ExternalType;
 import java.io.File;
 import org.junit.jupiter.api.BeforeEach;
@@ -29,7 +29,7 @@ File generateTestData(String prefix, int num, InstructionType... instructionType
             targetWasm =
                     smith.run(prefix + num, "test.wasm", new InstructionTypes(instructionTypes));
 
-            var exportSection = Module.builder(targetWasm).build().exportSection();
+            var exportSection = Parser.parse(targetWasm).exportSection();
             atLeastOneExportedFunction = false;
             for (int i = 0; i < exportSection.exportCount(); i++) {
                 if (exportSection.getExport(i).exportType() == ExternalType.FUNCTION) {
@@ -59,7 +59,7 @@ public void numericOnlyFuzz(RepetitionInfo repetitionInfo) throws Exception {
         var targetWasm =
                 generateTestData(
                         "numeric-", repetitionInfo.getCurrentRepetition(), InstructionType.NUMERIC);
-        var module = Module.builder(targetWasm).build();
+        var module = Parser.parse(targetWasm);
         var instance = Instance.builder(module).withInitialize(true).withStart(false).build();
 
         var results = testModule(targetWasm, module, instance);
@@ -76,7 +76,7 @@ public void tableOnlyFuzz(RepetitionInfo repetitionInfo) throws Exception {
         var targetWasm =
                 generateTestData(
                         "table-", repetitionInfo.getCurrentRepetition(), InstructionType.TABLE);
-        var module = Module.builder(targetWasm).build();
+        var module = Parser.parse(targetWasm);
         var instance = Instance.builder(module).withInitialize(true).withStart(false).build();
 
         var results = testModule(targetWasm, module, instance);

diff --git a/fuzz/src/test/java/com/dylibso/chicory/fuzz/RegressionTest.java b/fuzz/src/test/java/com/dylibso/chicory/fuzz/RegressionTest.java
@@ -4,7 +4,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import com.dylibso.chicory.runtime.Instance;
-import com.dylibso.chicory.wasm.Module;
+import com.dylibso.chicory.wasm.Parser;
 import java.io.File;
 import java.util.Arrays;
 import java.util.stream.Stream;
@@ -24,7 +24,7 @@ private static Stream<Arguments> crashFolders() {
     @MethodSource("crashFolders")
     void regressionTests(File folder) throws Exception {
         var targetWasm = new File(folder.getAbsolutePath() + "/test.wasm");
-        var module = Module.builder(targetWasm).build();
+        var module = Parser.parse(targetWasm);
         var instance = Instance.builder(module).withInitialize(true).withStart(false).build();
 
         var results = testModule(targetWasm, module, instance, false);

diff --git a/fuzz/src/test/java/com/dylibso/chicory/fuzz/SingleReproTest.java b/fuzz/src/test/java/com/dylibso/chicory/fuzz/SingleReproTest.java
@@ -3,7 +3,7 @@
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 
 import com.dylibso.chicory.runtime.Instance;
-import com.dylibso.chicory.wasm.Module;
+import com.dylibso.chicory.wasm.Parser;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import org.junit.jupiter.api.Test;
@@ -28,7 +28,7 @@ void singleReproducer() throws Exception {
         var targetWasm =
                 smith.run(seed.substring(0, Math.min(seed.length(), 32)), "test.wasm", types);
 
-        var module = Module.builder(targetWasm).build();
+        var module = Parser.parse(targetWasm);
         var instance = Instance.builder(module).withInitialize(true).withStart(false).build();
 
         testModule(targetWasm, module, instance);

diff --git a/jmh/src/main/java/com/dylibso/chicory/bench/BenchmarkFactorialExecution.java b/jmh/src/main/java/com/dylibso/chicory/bench/BenchmarkFactorialExecution.java
@@ -3,7 +3,7 @@
 import com.dylibso.chicory.aot.AotMachine;
 import com.dylibso.chicory.runtime.ExportFunction;
 import com.dylibso.chicory.runtime.Instance;
-import com.dylibso.chicory.wasm.Module;
+import com.dylibso.chicory.wasm.Parser;
 import com.dylibso.chicory.wasm.types.Value;
 import java.io.File;
 import java.util.concurrent.TimeUnit;
@@ -38,11 +38,11 @@ public class BenchmarkFactorialExecution {
 
     @Setup
     public void setup() {
-        var factorialInt = Instance.builder(Module.builder(ITERFACT).build()).build();
+        var factorialInt = Instance.builder(Parser.parse(ITERFACT)).build();
         iterFactInt = factorialInt.export("iterFact");
 
         var factorialAot =
-                Instance.builder(Module.builder(ITERFACT).build())
+                Instance.builder(Parser.parse(ITERFACT))
                         .withMachineFactory(AotMachine::new)
                         .build();
         iterFactAot = factorialAot.export("iterFact");

diff --git a/jmh/src/main/java/com/dylibso/chicory/bench/BenchmarkParsing.java b/jmh/src/main/java/com/dylibso/chicory/bench/BenchmarkParsing.java
@@ -1,6 +1,6 @@
 package com.dylibso.chicory.bench;
 
-import com.dylibso.chicory.wasm.Module;
+import com.dylibso.chicory.wasm.Parser;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
@@ -41,6 +41,6 @@ public void setup() throws IOException {
     @Benchmark
     @BenchmarkMode(Mode.Throughput)
     public void benchmark(Blackhole bh) {
-        bh.consume(Module.builder(memoryMappedFile).build());
+        bh.consume(Parser.parse(memoryMappedFile));
     }
 }
diff --git a/readmes/wasm/expected/parser-base.result b/readmes/wasm/expected/parser-base.result
@@ -0,0 +1 @@
+target_features
diff --git a/readmes/wasm/expected/parser-listener.result b/readmes/wasm/expected/parser-listener.result
@@ -0,0 +1,8 @@
+.debug_abbrev
+.debug_info
+.debug_ranges
+.debug_str
+.debug_line
+name
+producers
+target_features
diff --git a/runtime-tests/src/test/java/com/dylibso/chicory/testing/TestModule.java b/runtime-tests/src/test/java/com/dylibso/chicory/testing/TestModule.java
@@ -4,7 +4,7 @@
 import com.dylibso.chicory.runtime.Instance;
 import com.dylibso.chicory.wabt.Wat2Wasm;
 import com.dylibso.chicory.wasm.Module;
-import com.dylibso.chicory.wasm.ModuleType;
+import com.dylibso.chicory.wasm.Parser;
 import com.dylibso.chicory.wasm.exceptions.MalformedException;
 import java.io.File;
 
@@ -15,10 +15,6 @@ public class TestModule {
 
     private HostImports imports;
 
-    public static TestModule of(File file) {
-        return of(file, ModuleType.BINARY);
-    }
-
     public static TestModule of(Module module) {
         return new TestModule(module);
     }
@@ -48,18 +44,18 @@ public static TestModule of(Module module) {
                     + "alignment "
                     + "multiple start sections";
 
-    public static TestModule of(File file, ModuleType moduleType) {
-        if (moduleType == ModuleType.TEXT) {
+    public static TestModule of(File file) {
+        if (file.getName().endsWith(".wat")) {
             byte[] parsed;
             try {
                 parsed = Wat2Wasm.parse(file);
             } catch (Exception e) {
                 throw new MalformedException(
                         e.getMessage() + HACK_MATCH_ALL_MALFORMED_EXCEPTION_TEXT);
             }
-            return of(Module.builder(parsed).build());
+            return of(Parser.parse(parsed));
         }
-        return of(Module.builder(file).build());
+        return of(Parser.parse(file));
     }
 
     public TestModule(Module module) {

diff --git a/runtime/src/main/java/com/dylibso/chicory/runtime/Instance.java b/runtime/src/main/java/com/dylibso/chicory/runtime/Instance.java
@@ -627,8 +627,9 @@ private static void validateModule(Module module) {
             if (functionSectionSize != codeSectionSize) {
                 throw new MalformedException("function and code section have inconsistent lengths");
             }
-            if (module.dataCountSection() != null
-                    && dataSectionSize != module.dataCountSection().dataCount()) {
+            if (module.dataCountSection()
+                    .map(dcs -> dcs.dataCount() != dataSectionSize)
+                    .orElse(false)) {
                 throw new MalformedException(
                         "data count and data section have inconsistent lengths");
             }
@@ -813,15 +814,13 @@ public Instance build() {
                     mapHostImports(
                             imports,
                             (hostImports == null) ? new HostImports() : hostImports,
-                            (module.memorySection() != null)
-                                    ? module.memorySection().memoryCount()
-                                    : 0);
+                            module.memorySection().map(m -> m.memoryCount()).orElse(0));
 
-            if (module.startSection() != null) {
+            if (module.startSection().isPresent()) {
                 var export =
                         new Export(
                                 START_FUNCTION_NAME,
-                                (int) module.startSection().startIndex(),
+                                (int) module.startSection().get().startIndex(),
                                 ExternalType.FUNCTION);
                 exports.put(START_FUNCTION_NAME, export);
             }
@@ -839,8 +838,8 @@ public Instance build() {
             Element[] elements = module.elementSection().elements();
 
             Memory memory = null;
-            if (module.memorySection() != null) {
-                var memories = module.memorySection();
+            if (module.memorySection().isPresent()) {
+                var memories = module.memorySection().get();
                 if (memories.memoryCount() > 0) {
                     memory = new Memory(memories.getMemory(0).memoryLimits());
                 }
@@ -912,9 +911,7 @@ public Instance build() {
                     case MEMORY:
                         {
                             var memoryCount =
-                                    (module.memorySection() == null)
-                                            ? 0
-                                            : module.memorySection().memoryCount();
+                                    module.memorySection().map(m -> m.memoryCount()).orElse(0);
                             if (e.index() >= memoryCount + memoryImportsOffset) {
                                 throw new InvalidException("unknown memory " + e);
                             }

diff --git a/runtime/src/main/java/com/dylibso/chicory/runtime/TypeValidator.java b/runtime/src/main/java/com/dylibso/chicory/runtime/TypeValidator.java
@@ -958,9 +958,8 @@ public void validate(
 
         // to satisfy the check mentioned in the NOTE
         // https://webassembly.github.io/spec/core/binary/modules.html#data-count-section
-        if (instance.module().codeSection() != null
-                && instance.module().codeSection().isRequiresDataCount()
-                && instance.module().dataCountSection() == null) {
+        if (instance.module().codeSection().isRequiresDataCount()
+                && instance.module().dataCountSection().isEmpty()) {
             throw new MalformedException("data count section required");
         }
     }