From 235ef78a734b63f29b9b784aa04d23900e9eb0f5 Mon Sep 17 00:00:00 2001 From: maciektr Date: Mon, 26 Aug 2024 08:51:55 -0700 Subject: [PATCH] Add `unstable-add-statements-code-locations-debug-info` flag (#1538) Co-authored-by: Maksymilian Demitraszek --- .../src/compilation/test_collector.rs | 44 +++- .../src/metadata.rs | 9 + .../tests/test.rs | 37 +++ scarb/src/compiler/compilers/test.rs | 4 +- scarb/src/compiler/helpers.rs | 3 + scarb/src/core/manifest/compiler_config.rs | 10 + scarb/src/core/manifest/toml_manifest.rs | 12 + scarb/tests/build.rs | 214 ++++++++++++++++++ website/docs/reference/manifest.md | 22 +- 9 files changed, 342 insertions(+), 13 deletions(-) diff --git a/extensions/scarb-snforge-test-collector/src/compilation/test_collector.rs b/extensions/scarb-snforge-test-collector/src/compilation/test_collector.rs index a33bf699c..0863c1465 100644 --- a/extensions/scarb-snforge-test-collector/src/compilation/test_collector.rs +++ b/extensions/scarb-snforge-test-collector/src/compilation/test_collector.rs @@ -21,6 +21,7 @@ use cairo_lang_sierra::extensions::NamedType; use cairo_lang_sierra::ids::GenericTypeId; use cairo_lang_sierra::program::{GenericArg, ProgramArtifact}; use cairo_lang_sierra_generator::db::SierraGenGroup; +use cairo_lang_sierra_generator::program_generator::SierraProgramWithDebug; use cairo_lang_sierra_generator::replace_ids::replace_sierra_ids_in_program; use cairo_lang_starknet::starknet_plugin_suite; use cairo_lang_test_plugin::test_plugin_suite; @@ -143,16 +144,9 @@ pub fn collect_tests( .context("Compilation failed without any diagnostics") .context("Failed to get sierra program")?; - let debug_annotations = if compilation_unit.unstable_add_statements_functions_debug_info() { - Some(Annotations::from( - sierra_program - .debug_info - .statements_locations - .extract_statements_functions(db), - )) - } else { - None - }; + let debug_annotations: Option = + maybe_build_debug_annotations(compilation_unit, &sierra_program, db); + let debug_info = debug_annotations.map(|annotations| DebugInfo { type_names: Default::default(), executables: Default::default(), @@ -208,6 +202,36 @@ pub fn collect_tests( )) } +fn maybe_build_debug_annotations( + compilation_unit: &CompilationUnit, + sierra_program: &Arc, + db: &mut RootDatabase, +) -> Option { + if !compilation_unit.unstable_add_statements_functions_debug_info() + && !compilation_unit.unstable_add_statements_code_locations_debug_info() + { + return None; + }; + let mut debug_annotations: Annotations = Annotations::default(); + if compilation_unit.unstable_add_statements_functions_debug_info() { + debug_annotations.extend(Annotations::from( + sierra_program + .debug_info + .statements_locations + .extract_statements_functions(db), + )); + } + if compilation_unit.unstable_add_statements_code_locations_debug_info() { + debug_annotations.extend(Annotations::from( + sierra_program + .debug_info + .statements_locations + .extract_statements_source_code_locations(db), + )); + } + Some(debug_annotations) +} + fn build_test_details(function_finder: &FunctionFinder, test_name: &str) -> Result { let func = function_finder.find_function(test_name)?; diff --git a/extensions/scarb-snforge-test-collector/src/metadata.rs b/extensions/scarb-snforge-test-collector/src/metadata.rs index bce47c81e..2c6ad87f5 100644 --- a/extensions/scarb-snforge-test-collector/src/metadata.rs +++ b/extensions/scarb-snforge-test-collector/src/metadata.rs @@ -166,6 +166,15 @@ impl CompilationUnit<'_> { .unwrap_or(false) } + pub fn unstable_add_statements_code_locations_debug_info(&self) -> bool { + self.unit_metadata + .compiler_config + .as_object() + .and_then(|config| config.get("unstable_add_statements_code_locations_debug_info")) + .and_then(|value| value.as_bool()) + .unwrap_or(false) + } + pub fn main_package_source_root(&self) -> Utf8PathBuf { self.main_package_metadata.source_root().to_path_buf() } diff --git a/extensions/scarb-snforge-test-collector/tests/test.rs b/extensions/scarb-snforge-test-collector/tests/test.rs index 98f114144..b64b1dbb8 100644 --- a/extensions/scarb-snforge-test-collector/tests/test.rs +++ b/extensions/scarb-snforge-test-collector/tests/test.rs @@ -1,6 +1,7 @@ use assert_fs::prelude::*; use assert_fs::TempDir; use cairo_lang_sierra::program::StatementIdx; +use cairo_lang_sierra_generator::statements_code_locations::SourceCodeSpan; use std::collections::HashMap; use scarb_test_support::fsx::ChildPathEx; @@ -473,6 +474,42 @@ fn generates_statements_functions_mappings() { assert!(serde_json::from_value::>>(mappings.clone()).is_ok()); } +#[test] +fn generates_statements_code_locations_mappings() { + let t = TempDir::new().unwrap(); + + ProjectBuilder::start() + .name("forge_test") + .lib_cairo(SIMPLE_TEST) + .manifest_extra(indoc! {r#" + [cairo] + unstable-add-statements-code-locations-debug-info = true + "#}) + .build(&t); + + Scarb::quick_snapbox() + .arg("snforge-test-collector") + .current_dir(&t) + .assert() + .success(); + + let snforge_sierra = t + .child("target/dev/snforge/forge_test.snforge_sierra.json") + .read_to_string(); + + let json: Value = serde_json::from_str(&snforge_sierra).unwrap(); + + let mappings = &json[0]["sierra_program"]["debug_info"]["annotations"] + ["github.com/software-mansion/cairo-coverage"]["statements_code_locations"]; + + assert!( + serde_json::from_value::>>( + mappings.clone() + ) + .is_ok() + ); +} + #[test] fn features_test_build_success() { let t = TempDir::new().unwrap(); diff --git a/scarb/src/compiler/compilers/test.rs b/scarb/src/compiler/compilers/test.rs index 0f056964c..a517a6104 100644 --- a/scarb/src/compiler/compilers/test.rs +++ b/scarb/src/compiler/compilers/test.rs @@ -46,7 +46,9 @@ impl Compiler for TestCompiler { add_statements_functions: unit .compiler_config .unstable_add_statements_functions_debug_info, - add_statements_code_locations: false, + add_statements_code_locations: unit + .compiler_config + .unstable_add_statements_code_locations_debug_info, }; let allow_warnings = unit.compiler_config.allow_warnings; compile_test_prepared_db(db, config, main_crate_ids, test_crate_ids, allow_warnings)? diff --git a/scarb/src/compiler/helpers.rs b/scarb/src/compiler/helpers.rs index 6cf585c46..d54940054 100644 --- a/scarb/src/compiler/helpers.rs +++ b/scarb/src/compiler/helpers.rs @@ -63,6 +63,9 @@ pub fn build_compiler_config<'c>( add_statements_functions: unit .compiler_config .unstable_add_statements_functions_debug_info, + add_statements_code_locations: unit + .compiler_config + .unstable_add_statements_code_locations_debug_info, ..CompilerConfig::default() } } diff --git a/scarb/src/core/manifest/compiler_config.rs b/scarb/src/core/manifest/compiler_config.rs index 1e0a5d2b8..2f85f0b84 100644 --- a/scarb/src/core/manifest/compiler_config.rs +++ b/scarb/src/core/manifest/compiler_config.rs @@ -26,6 +26,12 @@ pub struct ManifestCompilerConfig { /// Used by [cairo-profiler](https://github.com/software-mansion/cairo-profiler). /// This feature is unstable and is subject to change. pub unstable_add_statements_functions_debug_info: bool, + /// Add a mapping between sierra statement indexes and code location in cairo code + /// to debug info. A statement index maps to a vector consisting of a code location which caused the + /// statement to be generated and all code location that were inlined or generated along the way. + /// Used by [cairo-coverage](https://github.com/software-mansion/cairo-coverage). + /// This feature is unstable and is subject to change. + pub unstable_add_statements_code_locations_debug_info: bool, // Inlining strategy. pub inlining_strategy: InliningStrategy, } @@ -47,6 +53,7 @@ impl DefaultForProfile for ManifestCompilerConfig { allow_warnings: true, enable_gas: true, unstable_add_statements_functions_debug_info: false, + unstable_add_statements_code_locations_debug_info: false, inlining_strategy: InliningStrategy::default(), } } @@ -61,6 +68,9 @@ impl From for TomlCairo { unstable_add_statements_functions_debug_info: Some( config.unstable_add_statements_functions_debug_info, ), + unstable_add_statements_code_locations_debug_info: Some( + config.unstable_add_statements_code_locations_debug_info, + ), inlining_strategy: Some(config.inlining_strategy), } } diff --git a/scarb/src/core/manifest/toml_manifest.rs b/scarb/src/core/manifest/toml_manifest.rs index 80ace84aa..8fd3f9601 100644 --- a/scarb/src/core/manifest/toml_manifest.rs +++ b/scarb/src/core/manifest/toml_manifest.rs @@ -321,6 +321,12 @@ pub struct TomlCairo { /// Used by [cairo-profiler](https://github.com/software-mansion/cairo-profiler). /// This feature is unstable and is subject to change. pub unstable_add_statements_functions_debug_info: Option, + /// Add a mapping between sierra statement indexes and lines in cairo code + /// to debug info. A statement index maps to a vector consisting of a line which caused the + /// statement to be generated and all lines that were inlined or generated along the way. + /// Used by [cairo-coverage](https://github.com/software-mansion/cairo-coverage). + /// This feature is unstable and is subject to change. + pub unstable_add_statements_code_locations_debug_info: Option, /// Inlining strategy. pub inlining_strategy: Option, } @@ -856,6 +862,12 @@ impl TomlManifest { compiler_config.unstable_add_statements_functions_debug_info = unstable_add_statements_functions_debug_info; } + if let Some(unstable_add_statements_code_locations_debug_info) = + cairo.unstable_add_statements_code_locations_debug_info + { + compiler_config.unstable_add_statements_code_locations_debug_info = + unstable_add_statements_code_locations_debug_info; + } } Ok(compiler_config) } diff --git a/scarb/tests/build.rs b/scarb/tests/build.rs index 28827472b..0282b2198 100644 --- a/scarb/tests/build.rs +++ b/scarb/tests/build.rs @@ -15,6 +15,8 @@ use scarb_test_support::contracts::BALANCE_CONTRACT; use scarb_test_support::fsx::ChildPathEx; use scarb_test_support::project_builder::{Dep, DepBuilder, ProjectBuilder}; use scarb_test_support::workspace_builder::WorkspaceBuilder; +use serde::Deserialize; +use serde::Serialize; #[test] fn compile_simple() { @@ -1208,6 +1210,129 @@ fn add_statements_functions_debug_info() { ); } +#[derive(Serialize, Deserialize)] +pub struct SourceCodeLocation { + pub line: usize, + pub col: usize, +} + +#[derive(Serialize, Deserialize)] +pub struct SourceCodeSpan { + pub start: SourceCodeLocation, + pub end: SourceCodeLocation, +} + +#[test] +fn add_statements_code_locations_debug_info() { + let t = TempDir::new().unwrap(); + ProjectBuilder::start() + .name("hello") + .lib_cairo(indoc! {r##" + #[starknet::interface] + pub trait IHelloStarknet { + fn increase_balance(ref self: TContractState, amount: felt252); + fn get_balance(self: @TContractState) -> felt252; + } + + #[starknet::contract] + mod HelloStarknet { + #[storage] + struct Storage { + balance: felt252, + } + + #[abi(embed_v0)] + impl HelloStarknetImpl of super::IHelloStarknet { + fn increase_balance(ref self: ContractState, amount: felt252) { + assert(amount != 0, 'Amount cannot be 0'); + self.balance.write(self.balance.read() + amount); + } + + fn get_balance(self: @ContractState) -> felt252 { + self.balance.read() + } + } + } + + fn foo(mut shape: Span) -> usize { + let mut result: usize = 1; + + loop { + match shape.pop_front() { + Option::Some(item) => { result *= *item; }, + Option::None => { break; } + }; + }; + + result + } + + fn main() -> usize { + foo(array![1, 2].span()) + } + "##}) + .manifest_extra(indoc! {r#" + [lib] + casm = true + + [[target.starknet-contract]] + casm = true + + [cairo] + unstable-add-statements-code-locations-debug-info = true + "#}) + .dep_starknet() + .build(&t); + Scarb::quick_snapbox() + .arg("build") + .current_dir(&t) + .assert() + .success(); + + let lib_sierra_string = t.child("target/dev/hello.sierra.json").read_to_string(); + let contract_sierra_string = t + .child("target/dev/hello_HelloStarknet.contract_class.json") + .read_to_string(); + let lib_sierra = serde_json::from_str::(&lib_sierra_string).unwrap(); + let contract_sierra = serde_json::from_str::(&contract_sierra_string).unwrap(); + + let debug_info = lib_sierra + .into_v1() + .unwrap() + .debug_info + .expect("Expected debug info to exist"); + let mappings = debug_info + .annotations + .get("github.com/software-mansion/cairo-coverage") + .expect("Expected cairo-coverage annotations to exist") + .get("statements_code_locations") + .expect("Expected statements_code_locations info to exist"); + assert!( + serde_json::from_value::>>( + mappings.clone() + ) + .is_ok(), + "Expected statements_code_locations info to be a map" + ); + + let debug_info = contract_sierra + .sierra_program_debug_info + .expect("Expected debug info to exist"); + let mappings = debug_info + .annotations + .get("github.com/software-mansion/cairo-coverage") + .expect("Expected cairo-coverage annotations to exist") + .get("statements_code_locations") + .expect("Expected statements_code_locations info to exist"); + assert!( + serde_json::from_value::>>( + mappings.clone() + ) + .is_ok(), + "Expected statements_code_locations info to be a map" + ); +} + #[test] fn add_statements_functions_debug_info_to_tests() { let t = TempDir::new().unwrap(); @@ -1293,3 +1418,92 @@ fn add_statements_functions_debug_info_to_tests() { "Expected statements_functions info to be a map" ); } + +#[test] +fn add_statements_code_locations_debug_info_to_tests() { + let t = TempDir::new().unwrap(); + ProjectBuilder::start() + .name("hello") + .lib_cairo(indoc! {r##" + #[starknet::interface] + pub trait IHelloStarknet { + fn increase_balance(ref self: TContractState, amount: felt252); + fn get_balance(self: @TContractState) -> felt252; + } + + #[starknet::contract] + mod HelloStarknet { + #[storage] + struct Storage { + balance: felt252, + } + + #[abi(embed_v0)] + impl HelloStarknetImpl of super::IHelloStarknet { + fn increase_balance(ref self: ContractState, amount: felt252) { + assert(amount != 0, 'Amount cannot be 0'); + self.balance.write(self.balance.read() + amount); + } + + fn get_balance(self: @ContractState) -> felt252 { + self.balance.read() + } + } + } + + fn foo(mut shape: Span) -> usize { + let mut result: usize = 1; + + loop { + match shape.pop_front() { + Option::Some(item) => { result *= *item; }, + Option::None => { break; } + }; + }; + + result + } + + fn main() -> usize { + foo(array![1, 2].span()) + } + "##}) + .manifest_extra(indoc! {r#" + [[target.starknet-contract]] + + [cairo] + unstable-add-statements-code-locations-debug-info = true + "#}) + .dep_starknet() + .build(&t); + Scarb::quick_snapbox() + .arg("build") + .arg("--test") + .current_dir(&t) + .assert() + .success(); + + let lib_sierra_string = t + .child("target/dev/hello_unittest.test.sierra.json") + .read_to_string(); + let lib_sierra = serde_json::from_str::(&lib_sierra_string).unwrap(); + + let debug_info = lib_sierra + .into_v1() + .unwrap() + .debug_info + .expect("Expected debug info to exist"); + let mappings = debug_info + .annotations + .get("github.com/software-mansion/cairo-coverage") + .expect("Expected cairo-coverage annotations to exist") + .get("statements_code_locations") + .expect("Expected statements_code_locations info to exist"); + assert!( + serde_json::from_value::>>( + mappings.clone() + ) + .is_ok(), + "Expected statements_code_locations info to be a map" + ); +} diff --git a/website/docs/reference/manifest.md b/website/docs/reference/manifest.md index 860fc85b0..36a8e14ee 100644 --- a/website/docs/reference/manifest.md +++ b/website/docs/reference/manifest.md @@ -307,8 +307,8 @@ If `avoid` strategy is set, the compiler will only inline function annotated wit > [!WARNING] > This is highly experimental and unstable feature intended to be used > by [cairo-profiler](https://github.com/software-mansion/cairo-profiler). -> It may slow down the compilation - it is advised not to use it for other purposes than mentioned in -> [cairo-profiler](https://github.com/software-mansion/cairo-profiler) documentation. +> It may slow down the compilation - it is advised not to use it for other purposes than running +> [cairo-profiler](https://github.com/software-mansion/cairo-profiler). If enabled, during the project compilation Scarb will a add mapping between Sierra statement indexes and vectors of fully qualified paths of Cairo functions to debug info. A statement index maps to a vector consisting of a function @@ -320,6 +320,24 @@ By default, this flag is disabled. unstable-add-statements-functions-debug-info = true ``` +### `unstable-add-statements-code-locations-debug-info` + +> [!WARNING] +> This is highly experimental and unstable feature intended to be used +> by [cairo-coverage](https://github.com/software-mansion/cairo-coverage). +> It may slow down the compilation - it is advised not to use it for other purposes than running +> [cairo-coverage](https://github.com/software-mansion/cairo-coverage). + +If enabled, during the project compilation Scarb will add a mapping between Sierra statement indexes and locations in +the code to debug info. A statement index maps to a vector consisting of code fragment which caused the statement to be +generated and all code fragments that were inlined or generated along the way. +By default, this flag is disabled. + +```toml +[cairo] +unstable-add-statements-code-locations-debug-info = true +``` + ## `[profile]` See [Profiles](./profiles) page.