Rename ExternalValues to ImportValues; order of params in ImportFunction,HostFunction.
#589
Conversation
This file contains hidden or 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
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
evacchi
changed the title
ExternalValues to ImportValues; order of params in ImportFunction,HostFunction.
evacchi
added a commit
that referenced
this pull request
Oct 23, 2024
Reworded a few sections, this will need changes if #589 gets merged. Notably: - The `HostFunction` section now comes first, so I removed the reference to "using _again_ memory" because this is the first time we are seeing it - I have also rounded up the section to introduce and clarify the concept of "host", "guest" - [x] the description of the `HostFunction` should change to reflect #589 - [x] we should also update this same section to use the `Store` from the start - [x] we might describe later the alternate method (passing in "imports"/"external values" directly) for low-level access EDIT: I did all the last 3 above as #589 was merged in the meantime --------- Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #527
Hopefully last big rename (related to this, at least) before the RC 😅
After chatting with @andreaTP, we agreed that
ExternalValues, from the perspective of an end user, was confusing naming ("external to what?"). We came up withImportedValues.To be honest, my original objection with
HostImportswas about them beingHost, not `Imports" :)<module,name>).ExportFunction)we do "lose" the parallel with the spec, but this is less of an issue, because we only use these
ImportValuesas, duh, values imported byInstances. We never returnImportValuefrom anexport*()-kind API, so everything should workOrder of Parameters for External/HostFunction
In this PR I am also changing, again, the order of the parameters for
ImportFunctionbecause the old constructor "broke" the signature into two pieces, i.e. the handle was between the names and the types:vs:
which I think is clearer. This change is in the last 2 commits, for easier review.