[2/x] Translate basic wallet Wasm component #131
Merged
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
greenhat
force-pushed
the
greenhat/i109-basic-wallet-trans
branch
from
9920e70 to
e650342
Compare
greenhat
force-pushed
the
greenhat/i109-basic-wallet-trans
branch
2 times, most recently
from
1883b7a to
422b8f7
Compare
greenhat
force-pushed
the
greenhat/wasmcm-itest-support
branch
from
fe0cbde to
2d39974
Compare
greenhat
force-pushed
the
greenhat/i109-basic-wallet-trans
branch
from
cce1f5b to
5c2cc0d
Compare
greenhat
changed the title
bitwalker
force-pushed
the
greenhat/wasmcm-itest-support
branch
from
2d39974 to
1960cfd
Compare
bitwalker
reviewed
Mar 15, 2024
hir-type/src/lib.rs
Outdated
| @@ -65,6 +65,8 @@ pub enum Type { | |||
| Struct(StructType), | |||
| /// A vector of fixed size | |||
| Array(Box<Type>, usize), | |||
| /// A tuple of values of the given types | |||
There was a problem hiding this comment.
Do we actually need a Tuple type? A tuple is basically just an anonymous struct, there is no meaningful difference between them. I'd rather avoid introducing additional type-level abstractions if we can avoid it, since it requires handling that type in various places throughout the compiler, and we don't really want to do that unless it is actually semantically significant.
If there is a specific reason why StructType is insufficient, let me know, but for now I'm going to leave it out of the rebased changes and use structs instead.
There was a problem hiding this comment.
Probably, not. I cannot find any reason why it cannot be represented as a struct.
Swap function calls to imported functions with the external function passed as module arguments.
according to the instantiation order
in the final stage of the translation to make the overall algorithm and steps more clear and concise. Raise errors when an particular item translation is not(yet) implemented.
temporary use of u64 for Felt in WIT.
bitwalker
force-pushed
the
greenhat/i109-basic-wallet-trans
branch
from
5c2cc0d to
eb3c0c9
Compare
bitwalker
approved these changes
Mar 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Close #109
This PR is stacked on top of #127 and should be merged after it
This PR implements missing pieces in the Wasm CM translation needed for basic wallet example translation.
Wasm tables translation is not yet implemented, along with the
call_indirectop translation (ignored) and are extracted into #133.This PR adds:
miden_hir::Type::Tupleto represent Wasm CM tuple types;FuncEnvironmentforcallop translation;ComponentTranslatorto make the overall algorithm and steps more clear and concise;Type::splitimplementation forTupleis temporarily plugged withtodo!. Let me know if you want me to implement it.