From e1796abe5707a3d39b0c9cc5c42cf0db086c7955 Mon Sep 17 00:00:00 2001 From: Denys Zadorozhnyi Date: Fri, 11 Oct 2024 16:09:47 +0300 Subject: [PATCH] chore: clean up todos --- frontend-wasm/src/miden_abi/mod.rs | 6 +++--- .../integration/src/rust_masm_tests/rust_sdk.rs | 4 ---- tools/cargo-miden/src/lib.rs | 17 ++++++----------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/frontend-wasm/src/miden_abi/mod.rs b/frontend-wasm/src/miden_abi/mod.rs index 51bcbc41..d5662316 100644 --- a/frontend-wasm/src/miden_abi/mod.rs +++ b/frontend-wasm/src/miden_abi/mod.rs @@ -61,8 +61,9 @@ fn miden_stdlib_function_type(module_id: Symbol, function_id: Symbol) -> Functio /// Restore module and function names of the intrinsics and Miden SDK functions /// that were renamed to satisfy the Wasm Component Model requirements. /// -/// Returns the pre-renamed module and function names or does nothing if the -/// function is not an intrinsic or Miden SDK function +/// Returns the pre-renamed (expected at the linking stage) module and function +/// names or given `wasm_module_id` and `wasm_function_id` ids if the function +/// is not an intrinsic or Miden SDK function pub fn recover_imported_masm_function_id( wasm_module_id: &str, wasm_function_id: &str, @@ -94,7 +95,6 @@ pub fn recover_imported_masm_function_id( }; // Since `hash-1to1` is an invalid name in Wasm CM (dashed part cannot start with a digit), // we need to translate the CM name to the one that is expected at the linking stage - // TODO: look into why dashed part cannot start with a digit? let function_id = if wasm_function_id == "hash-one-to-one" { "hash_1to1".to_string() } else if wasm_function_id == "hash-two-to-one" { diff --git a/tests/integration/src/rust_masm_tests/rust_sdk.rs b/tests/integration/src/rust_masm_tests/rust_sdk.rs index 66d79d41..fc3950a4 100644 --- a/tests/integration/src/rust_masm_tests/rust_sdk.rs +++ b/tests/integration/src/rust_masm_tests/rust_sdk.rs @@ -38,8 +38,4 @@ fn rust_sdk_basic_wallet() { let artifact_name = test.artifact_name().to_string(); test.expect_wasm(expect_file![format!("../../expected/rust_sdk/{artifact_name}.wat")]); test.expect_ir(expect_file![format!("../../expected/rust_sdk/{artifact_name}.hir")]); - // test.expect_masm(expect_file![format!("../../expected/rust_sdk/{artifact_name}.masm")]); - // TODO: fix flaky test, "exec."_ZN19miden_sdk_tx_kernel9add_asset17h6f4cff304c095ffc" is - // changing the suffix on every n-th run test.expect_masm(expect_file![format!("../../ - // expected/{project_name}/{artifact_name}.masm" )]); } diff --git a/tools/cargo-miden/src/lib.rs b/tools/cargo-miden/src/lib.rs index 1057f971..137e764c 100644 --- a/tools/cargo-miden/src/lib.rs +++ b/tools/cargo-miden/src/lib.rs @@ -100,7 +100,6 @@ pub enum OutputType { /// Runs the cargo-miden command /// The arguments are expected to start with `["cargo", "miden", ...]` followed by a subcommand with options -// TODO: Use Report instead of anyhow? pub fn run(args: T, build_output_type: OutputType) -> anyhow::Result> where T: Iterator, @@ -173,7 +172,6 @@ where } for package in packages.iter_mut() { - // TODO: do we want/need to explicitly specify the package version? package.metadata.section.bindings.with = [ ("miden:base/core-types@1.0.0/felt", "miden_sdk::Felt"), ("miden:base/core-types@1.0.0/word", "miden_sdk::Word"), @@ -192,11 +190,9 @@ where // skip functions that are provided by the Miden SDK and/or intrinsics // only function names (no CM path) package.metadata.section.bindings.skip = vec![ - // TODO: not unique enough! - // If it's a proper component import or user export it would be skipped! - // Option 1: make "unique" intrinsics/SDK function names by including interface - // name in the function name - // Option 2: add a full CM path option in wit-bindgen + // Our function names can clash with user's function names leading to + // skipping the bindings generation of the user's function names + // see https://github.com/0xPolygonMiden/compiler/issues/341 "remove-asset", "create-note", "heap-base", @@ -208,7 +204,6 @@ where .into_iter() .map(|s| s.to_string()) .collect(); - // TODO: add namespace/package name support in the `skip` field in wit-bindgen? } let mut spawn_args: Vec<_> = args.into_iter().collect(); @@ -248,10 +243,10 @@ where ) .await })?; - // TODO: analyze `packages` and find the ones that don't have a WIT component and get Wasm binary (core module) for them with our own version of run_cargo_command if wasm_outputs.is_empty() { - // crates that don't have a WIT component are ignored by the `cargo-component` run_cargo_command - // build them with our own version of run_cargo_command + // crates that don't have a WIT component are ignored by the + // `cargo-component` run_cargo_command and return no outputs. + // Build them with our own version of run_cargo_command wasm_outputs = run_cargo_command_for_non_component( &config, subcommand.as_deref(),