-
Notifications
You must be signed in to change notification settings - Fork 35
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rename HostImports to ExternalValues (#527)
a.k.a. **The Big Rename™️**. This follows an offline discussion with @andreaTP. The idea would be to finalize naming before the future 1.0 release. The class `HostImports` and the related dependencies `HostFunction`, `HostTable`, `HostGlobal`, `HostMemory` are currently being used with two different purposes: 1. allowing a **host**, that is, the application that embeds the Wasm runtime, to define such values programmatically using the **host** language (especially true for functions) 2. allowing **the runtime** to link together two different module instances, connecting their "import" requirements to the "exports" of other instances. However, name "Host<something>" makes most sense for the first case, while it feels slightly off and ambiguous in the second case: are those Host-defined values, or are those just values belonging to another instance? The spec in this regard is not ambiguous, and generally refers to [**External Types**](https://webassembly.github.io/spec/core/syntax/types.html#external-types) and [**External Values**](https://webassembly.github.io/spec/core/exec/runtime.html#syntax-externval). Specifically: - **External Types** classify **imports** and **external values**. Notice [we are already using the term `ExternalType`](https://github.com/dylibso/chicory/blob/dc6959dbf20544517374d93e1f7c255d316ecdaa/wasm/src/main/java/com/dylibso/chicory/wasm/types/ExternalType.java#L13) under package `com.dylibso.chicory.wasm.types`. - I think **External Values** identifies more correctly what we are currently calling **HostImports**; i.e. instances of the respective types (instances of functions, of globals, of memories or of tables), capturing the fact that these values are just "externally defined", not necessarily by **the host**, but possibly by an instance of another module. Thus, with this change, I propose the following renames: - `FromHost` -> `ExternalValue` - `FromHost.FromHostType` -> `ExternalValue.Type` notice that we could also reuse `com.dylibso.chicory.wasm.types. ExternalType`, but this would require exposing to the end users package `com.dylibso.chicory.wasm.types` from package `com.dylibso.chicory.runtime` - `HostImports` -> `ExternalValues` - `HostGlobal` -> `ExternalGlobal` - `HostTable` -> `ExternalTable` - `HostMemory` -> `ExternalMemory` - `HostFunction` still exists, as a *subtype* of `ExternalFunction`: this allows to keep a distinction that might be useful in the future; in fact, the constructor to `ExternalFunction` could be even package-local. Notice that, I have not re-defined `Host{Global,Table,Memory}` as subtypes of `External{Global,Table,Memory}`, because I feel like there is no meaningful distinction: - the implementation for the "host" version does not conceptually differs from the "wasm" version - on the other hand, `HostFunction`s implement logic using the host language, while `ExternalFunction`s are otherwise just the exported image of an instance of a `Function` defined in a Wasm module. So, even though their interface might be the same, they are different concepts. Even if we decided to drop the concrete type `HostFunction` altogether, it might still make sense to expose a factory `ExternalFunction.createHostFunction() for an `ExternalFunction` because the term "host function" is generally known to refer to a host-defined function. It might be now feel slightly more awkward to import host functions directly through an `Instance.Builder`; e.g.: ``` var wasi = new WasiPreview1(logger, wasiOpts); var hostFunctions = new ExternalValues(wasi.toHostFunctions()); var inst = Instance.builder(Parser.parse(new File("greet-wasi.wasm"))) .withExternalValues(hostFunctions).build(); ``` but consider that, on the other hand, the `Instance.Builder` interface is a lower-level interface, so it might be acceptable for it to expose "lower-level" naming; in the future, I would expect users to interact with the runtime using higher level interfaces, such as the `Store`. In this case, users would write: ``` var wasi = new WasiPreview1(logger, wasiOpts); var store = new Store(); store.addFunctions(wasi.toHostFunctions()); var inst = store.instantiate("main", Parser.parse(new File("greet-wasi.wasm"))); ``` ### Notes Will follow-up with another, conceptually unrelated rename, but with the same goal of improved clarity; i.e.: - `fieldName()` -> `symbolName()` reason being `fieldName()` to me sounds too much like "name of a property", that is, it makes me think "Global" but we also use it for "Function"s. "Symbol" is more generic and should avoid ambiguity. Signed-off-by: Edoardo Vacchi <[email protected]> --------- Signed-off-by: Edoardo Vacchi <[email protected]>
- Loading branch information
Showing
32 changed files
with
560 additions
and
545 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.