Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
andreaTP committed Aug 14, 2024
1 parent 4c2851f commit 087f886
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 60 deletions.
2 changes: 1 addition & 1 deletion readmes/wasm/expected/parser-base.result
Original file line number Diff line number Diff line change
@@ -1 +1 @@
target_features
target_features
Original file line number Diff line number Diff line change
Expand Up @@ -958,8 +958,7 @@ 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()
if (instance.module().codeSection().isRequiresDataCount()
&& instance.module().dataCountSection().isEmpty()) {
throw new MalformedException("data count section required");
}
Expand Down
2 changes: 1 addition & 1 deletion wasm/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ System.out.println("First custom section: " + customSection.name());

<!--
```java
writeResultFile("parser-base.result", customSection.name());
writeResultFile("parser-base.result", customSection.name() + "\n");
```
-->

Expand Down
4 changes: 3 additions & 1 deletion wasm/src/main/java/com/dylibso/chicory/wasm/Module.java
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,13 @@ public Builder setDataSection(DataSection ds) {
}

public Builder setDataCountSection(DataCountSection dcs) {
this.dataCountSection = Optional.of(dcs);
this.dataCountSection = Optional.ofNullable(dcs);
return this;
}

public Builder addCustomSection(String name, CustomSection cs) {
requireNonNull(name);
requireNonNull(cs);
this.customSections.put(name, cs);
return this;
}
Expand Down
74 changes: 36 additions & 38 deletions wasm/src/main/java/com/dylibso/chicory/wasm/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ private CustomSection parseCustomSection(
private static TypeSection parseTypeSection(ByteBuffer buffer) {

var typeCount = readVarUInt32(buffer);
TypeSection.Builder typeSectionBuilder = TypeSection.builder();
TypeSection.Builder typeSection = TypeSection.builder();

// Parse individual types in the type section
for (int i = 0; i < typeCount; i++) {
Expand Down Expand Up @@ -417,16 +417,16 @@ private static TypeSection parseTypeSection(ByteBuffer buffer) {
returns[j] = ValueType.forId((int) readVarUInt32(buffer));
}

typeSectionBuilder.addFunctionType(FunctionType.of(params, returns));
typeSection.addFunctionType(FunctionType.of(params, returns));
}

return typeSectionBuilder.build();
return typeSection.build();
}

private static ImportSection parseImportSection(ByteBuffer buffer) {

var importCount = readVarUInt32(buffer);
ImportSection.Builder importSectionBuilder = ImportSection.builder();
ImportSection.Builder importSection = ImportSection.builder();

// Parse individual imports in the import section
for (int i = 0; i < importCount; i++) {
Expand All @@ -444,7 +444,7 @@ private static ImportSection parseImportSection(ByteBuffer buffer) {
if (moduleName.isEmpty() && importName.isEmpty()) {
throw new MalformedException("malformed import kind");
}
importSectionBuilder.addImport(
importSection.addImport(
new FunctionImport(
moduleName, importName, (int) readVarUInt32(buffer)));
break;
Expand All @@ -464,7 +464,7 @@ private static ImportSection parseImportSection(ByteBuffer buffer) {
? new Limits(min, readVarUInt32(buffer))
: new Limits(min);

importSectionBuilder.addImport(
importSection.addImport(
new TableImport(moduleName, importName, tableType, limits));
break;
}
Expand All @@ -483,42 +483,41 @@ private static ImportSection parseImportSection(ByteBuffer buffer) {
readVarUInt32(buffer)))
: new MemoryLimits(min, MemoryLimits.MAX_PAGES);

importSectionBuilder.addImport(
new MemoryImport(moduleName, importName, limits));
importSection.addImport(new MemoryImport(moduleName, importName, limits));
break;
}
case GLOBAL:
var globalValType = ValueType.forId((int) readVarUInt32(buffer));
var globalMut = MutabilityType.forId(readByte(buffer));
importSectionBuilder.addImport(
importSection.addImport(
new GlobalImport(moduleName, importName, globalMut, globalValType));
break;
default:
throw new MalformedException("malformed import kind");
}
}

return importSectionBuilder.build();
return importSection.build();
}

private static FunctionSection parseFunctionSection(ByteBuffer buffer) {

var functionCount = readVarUInt32(buffer);
FunctionSection.Builder functionSectionBuilder = FunctionSection.builder();
FunctionSection.Builder functionSection = FunctionSection.builder();

// Parse individual functions in the function section
for (int i = 0; i < functionCount; i++) {
var typeIndex = readVarUInt32(buffer);
functionSectionBuilder.addFunctionType((int) typeIndex);
functionSection.addFunctionType((int) typeIndex);
}

return functionSectionBuilder.build();
return functionSection.build();
}

private static TableSection parseTableSection(ByteBuffer buffer) {

var tableCount = readVarUInt32(buffer);
TableSection.Builder tableSectionBuilder = TableSection.builder();
TableSection.Builder tableSection = TableSection.builder();

// Parse individual tables in the tables section
for (int i = 0; i < tableCount; i++) {
Expand All @@ -529,24 +528,24 @@ private static TableSection parseTableSection(ByteBuffer buffer) {
}
var min = readVarUInt32(buffer);
var limits = limitType > 0 ? new Limits(min, readVarUInt32(buffer)) : new Limits(min);
tableSectionBuilder.addTable(new Table(tableType, limits));
tableSection.addTable(new Table(tableType, limits));
}

return tableSectionBuilder.build();
return tableSection.build();
}

private static MemorySection parseMemorySection(ByteBuffer buffer) {

var memoryCount = readVarUInt32(buffer);
MemorySection.Builder memorySectionBuilder = MemorySection.builder();
MemorySection.Builder memorySection = MemorySection.builder();

// Parse individual memories in the memory section
for (int i = 0; i < memoryCount; i++) {
var limits = parseMemoryLimits(buffer);
memorySectionBuilder.addMemory(new Memory(limits));
memorySection.addMemory(new Memory(limits));
}

return memorySectionBuilder.build();
return memorySection.build();
}

private static MemoryLimits parseMemoryLimits(ByteBuffer buffer) {
Expand All @@ -568,33 +567,33 @@ private static MemoryLimits parseMemoryLimits(ByteBuffer buffer) {
private static GlobalSection parseGlobalSection(ByteBuffer buffer) {

var globalCount = readVarUInt32(buffer);
GlobalSection.Builder globalSectionBuilder = GlobalSection.builder();
GlobalSection.Builder globalSection = GlobalSection.builder();

// Parse individual globals
for (int i = 0; i < globalCount; i++) {
var valueType = ValueType.forId((int) readVarUInt32(buffer));
var mutabilityType = MutabilityType.forId(readByte(buffer));
var init = parseExpression(buffer);
globalSectionBuilder.addGlobal(new Global(valueType, mutabilityType, List.of(init)));
globalSection.addGlobal(new Global(valueType, mutabilityType, List.of(init)));
}

return globalSectionBuilder.build();
return globalSection.build();
}

private static ExportSection parseExportSection(ByteBuffer buffer) {

var exportCount = readVarUInt32(buffer);
ExportSection.Builder exportSectionBuilder = ExportSection.builder();
ExportSection.Builder exportSection = ExportSection.builder();

// Parse individual functions in the function section
for (int i = 0; i < exportCount; i++) {
var name = readName(buffer, false);
var exportType = ExternalType.byId((int) readVarUInt32(buffer));
var index = (int) readVarUInt32(buffer);
exportSectionBuilder.addExport(new Export(name, index, exportType));
exportSection.addExport(new Export(name, index, exportType));
}

return exportSectionBuilder.build();
return exportSection.build();
}

private static StartSection parseStartSection(ByteBuffer buffer) {
Expand All @@ -605,16 +604,16 @@ private static ElementSection parseElementSection(ByteBuffer buffer, long sectio
var initialPosition = buffer.position();

var elementCount = readVarUInt32(buffer);
ElementSection.Builder elementSectionBuilder = ElementSection.builder();
ElementSection.Builder elementSection = ElementSection.builder();

for (var i = 0; i < elementCount; i++) {
elementSectionBuilder.addElement(parseSingleElement(buffer));
elementSection.addElement(parseSingleElement(buffer));
}
if (buffer.position() != initialPosition + sectionSize) {
throw new MalformedException("section size mismatch");
}

return elementSectionBuilder.build();
return elementSection.build();
}

private static Element parseSingleElement(ByteBuffer buffer) {
Expand Down Expand Up @@ -722,7 +721,7 @@ private static CodeSection parseCodeSection(ByteBuffer buffer) {
var funcBodyCount = readVarUInt32(buffer);

var root = new ControlTree();
var codeSectionBuilder = CodeSection.builder();
var codeSection = CodeSection.builder();

// Parse individual function bodies in the code section
for (int i = 0; i < funcBodyCount; i++) {
Expand All @@ -745,7 +744,7 @@ private static CodeSection parseCodeSection(ByteBuffer buffer) {
switch (instruction.opcode()) {
case MEMORY_INIT:
case DATA_DROP:
codeSectionBuilder.setRequiresDataCount(true);
codeSection.setRequiresDataCount(true);
}

// depth control
Expand Down Expand Up @@ -870,41 +869,40 @@ private static CodeSection parseCodeSection(ByteBuffer buffer) {
} while (!lastInstruction);

var functionBody = new FunctionBody(locals, instructions);
codeSectionBuilder.addFunctionBody(functionBody);
codeSection.addFunctionBody(functionBody);
}

return codeSectionBuilder.build();
return codeSection.build();
}

private static DataSection parseDataSection(ByteBuffer buffer) {

var dataSegmentCount = readVarUInt32(buffer);
DataSection.Builder dataSectionBuilder = DataSection.builder();
DataSection.Builder dataSection = DataSection.builder();

for (var i = 0; i < dataSegmentCount; i++) {
var mode = readVarUInt32(buffer);
if (mode == 0) {
var offset = parseExpression(buffer);
byte[] data = new byte[(int) readVarUInt32(buffer)];
readBytes(buffer, data);
dataSectionBuilder.addDataSegment(new ActiveDataSegment(0, List.of(offset), data));
dataSection.addDataSegment(new ActiveDataSegment(0, List.of(offset), data));
} else if (mode == 1) {
byte[] data = new byte[(int) readVarUInt32(buffer)];
readBytes(buffer, data);
dataSectionBuilder.addDataSegment(new PassiveDataSegment(data));
dataSection.addDataSegment(new PassiveDataSegment(data));
} else if (mode == 2) {
var memoryId = readVarUInt32(buffer);
var offset = parseExpression(buffer);
byte[] data = new byte[(int) readVarUInt32(buffer)];
readBytes(buffer, data);
dataSectionBuilder.addDataSegment(
new ActiveDataSegment(memoryId, List.of(offset), data));
dataSection.addDataSegment(new ActiveDataSegment(memoryId, List.of(offset), data));
} else {
throw new ChicoryException("Failed to parse data segment with data mode: " + mode);
}
}

return dataSectionBuilder.build();
return dataSection.build();
}

private static DataCountSection parseDataCountSection(ByteBuffer buffer) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
package com.dylibso.chicory.wasm.types;

import static java.util.Objects.requireNonNull;

import com.dylibso.chicory.wasm.Parser;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.ToIntFunction;

/**
* The "name" custom section.
*/
public class NameCustomSection extends CustomSection {

private final String moduleName;
private final Optional<String> moduleName;
private final List<NameEntry> funcNames;
private final List<ListEntry<NameEntry>> localNames;
private final List<ListEntry<NameEntry>> labelNames;
Expand All @@ -27,7 +29,7 @@ public class NameCustomSection extends CustomSection {
* Construct a section instance from the specified contents.
*/
private NameCustomSection(
String moduleName,
Optional<String> moduleName,
List<NameEntry> funcNames,
List<ListEntry<NameEntry>> localNames,
List<ListEntry<NameEntry>> labelNames,
Expand All @@ -38,15 +40,15 @@ private NameCustomSection(
List<NameEntry> dataNames,
List<NameEntry> tagNames) {
this.moduleName = moduleName;
this.funcNames = List.copyOf(funcNames);
this.localNames = List.copyOf(localNames);
this.labelNames = List.copyOf(labelNames);
this.tableNames = List.copyOf(tableNames);
this.memoryNames = List.copyOf(memoryNames);
this.globalNames = List.copyOf(globalNames);
this.elementNames = List.copyOf(elementNames);
this.dataNames = List.copyOf(dataNames);
this.tagNames = List.copyOf(tagNames);
this.funcNames = List.copyOf(requireNonNull(funcNames));
this.localNames = List.copyOf(requireNonNull(localNames));
this.labelNames = List.copyOf(requireNonNull(labelNames));
this.tableNames = List.copyOf(requireNonNull(tableNames));
this.memoryNames = List.copyOf(requireNonNull(memoryNames));
this.globalNames = List.copyOf(requireNonNull(globalNames));
this.elementNames = List.copyOf(requireNonNull(elementNames));
this.dataNames = List.copyOf(requireNonNull(dataNames));
this.tagNames = List.copyOf(requireNonNull(tagNames));
}

/**
Expand Down Expand Up @@ -130,7 +132,7 @@ public static NameCustomSection parse(byte[] bytes) {
}

return new NameCustomSection(
moduleName,
Optional.ofNullable(moduleName),
funcNames,
localNames,
labelNames,
Expand All @@ -147,9 +149,9 @@ public String name() {
}

/**
* @return the module name, or <code>null</code> if none is set
* @return the optional module name
*/
public String moduleName() {
public Optional<String> moduleName() {
return moduleName;
}

Expand Down Expand Up @@ -279,7 +281,7 @@ private static String twoLevelSearch(
}

private static String oneLevelStore(List<NameEntry> list, int storeIdx, String name) {
Objects.requireNonNull(name);
requireNonNull(name);
int idx = binarySearch(list, storeIdx, NameEntry::index);
if (idx < 0) {
// insert
Expand All @@ -293,7 +295,7 @@ private static String oneLevelStore(List<NameEntry> list, int storeIdx, String n

private static String twoLevelStore(
List<ListEntry<NameEntry>> listList, int groupIdx, int subIdx, String name) {
Objects.requireNonNull(name);
requireNonNull(name);
int fi = binarySearch(listList, groupIdx, ListEntry::index);
ListEntry<NameEntry> subList;
if (fi < 0) {
Expand Down

0 comments on commit 087f886

Please sign in to comment.