From cb5a15b9dbcfdaac5d656a122c73dca23855307d Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Fri, 27 Oct 2023 19:42:22 +0200 Subject: [PATCH] fix(3300): cache warnings into debug artefacts (#3313) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Savio <72797635+Savio-Sou@users.noreply.github.com> Co-authored-by: José Pedro Sousa Co-authored-by: jfecher Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- Cargo.lock | 2 ++ compiler/noirc_driver/src/contract.rs | 2 ++ compiler/noirc_driver/src/lib.rs | 35 ++++++++++++------------ compiler/noirc_driver/src/program.rs | 2 ++ compiler/noirc_evaluator/Cargo.toml | 3 +- compiler/noirc_evaluator/src/errors.rs | 5 ++-- tooling/nargo/src/artifacts/debug.rs | 4 ++- tooling/nargo/src/ops/test.rs | 2 +- tooling/nargo_cli/src/cli/compile_cmd.rs | 9 ++++-- tooling/nargo_cli/src/cli/debug_cmd.rs | 1 + tooling/nargo_cli/src/cli/execute_cmd.rs | 1 + 11 files changed, 41 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 09bb5276435..09f2708da11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2068,6 +2068,7 @@ dependencies = [ "bitmaps", "rand_core", "rand_xoshiro", + "serde", "sized-chunks", "typenum", "version_check", @@ -2662,6 +2663,7 @@ dependencies = [ "noirc_errors", "noirc_frontend", "num-bigint", + "serde", "thiserror", ] diff --git a/compiler/noirc_driver/src/contract.rs b/compiler/noirc_driver/src/contract.rs index da097d4cb86..d8688780a1f 100644 --- a/compiler/noirc_driver/src/contract.rs +++ b/compiler/noirc_driver/src/contract.rs @@ -5,6 +5,7 @@ use acvm::acir::circuit::Circuit; use fm::FileId; use noirc_abi::{Abi, ContractEvent}; use noirc_errors::debug_info::DebugInfo; +use noirc_evaluator::errors::SsaReport; use super::debug::DebugFile; use crate::program::{deserialize_circuit, serialize_circuit}; @@ -42,6 +43,7 @@ pub struct CompiledContract { pub events: Vec, pub file_map: BTreeMap, + pub warnings: Vec, } /// Each function in the contract will be compiled diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 2afc3216f70..8ec92597a7c 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -10,7 +10,6 @@ use iter_extended::vecmap; use noirc_abi::{AbiParameter, AbiType, ContractEvent}; use noirc_errors::{CustomDiagnostic, FileDiagnostic}; use noirc_evaluator::errors::RuntimeError; -use noirc_evaluator::errors::SsaReport; use noirc_evaluator::{create_circuit, into_abi_params}; use noirc_frontend::graph::{CrateId, CrateName}; use noirc_frontend::hir::def_map::{Contract, CrateDefMap}; @@ -180,10 +179,9 @@ pub fn compile_main( } }; - let (compiled_program, compilation_warnings) = - compile_no_check(context, options, main, cached_program, force_compile) - .map_err(FileDiagnostic::from)?; - let compilation_warnings = vecmap(compilation_warnings, FileDiagnostic::from); + let compiled_program = compile_no_check(context, options, main, cached_program, force_compile) + .map_err(FileDiagnostic::from)?; + let compilation_warnings = vecmap(compiled_program.warnings.clone(), FileDiagnostic::from); if options.deny_warnings && !compilation_warnings.is_empty() { return Err(compilation_warnings); } @@ -267,6 +265,7 @@ fn compile_contract_inner( ) -> Result { let mut functions = Vec::new(); let mut errors = Vec::new(); + let mut warnings = Vec::new(); for contract_function in &contract.functions { let function_id = contract_function.function_id; let is_entry_point = contract_function.is_entry_point; @@ -282,12 +281,13 @@ fn compile_contract_inner( } let function = match compile_no_check(context, options, function_id, None, true) { - Ok((function, _warnings)) => function, + Ok(function) => function, Err(new_error) => { errors.push(FileDiagnostic::from(new_error)); continue; } }; + warnings.extend(function.warnings); let modifiers = context.def_interner.function_modifiers(&function_id); let func_type = modifiers .contract_function_type @@ -323,6 +323,7 @@ fn compile_contract_inner( functions, file_map, noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(), + warnings, }) } else { Err(errors) @@ -340,7 +341,7 @@ pub fn compile_no_check( main_function: FuncId, cached_program: Option, force_compile: bool, -) -> Result<(CompiledProgram, Vec), RuntimeError> { +) -> Result { let program = monomorphize(main_function, &context.def_interner); let hash = fxhash::hash64(&program); @@ -350,7 +351,7 @@ pub fn compile_no_check( if !(force_compile || options.print_acir || options.show_brillig || options.show_ssa) { if let Some(cached_program) = cached_program { if hash == cached_program.hash { - return Ok((cached_program, Vec::new())); + return Ok(cached_program); } } } @@ -360,15 +361,13 @@ pub fn compile_no_check( let file_map = filter_relevant_files(&[debug.clone()], &context.file_manager); - Ok(( - CompiledProgram { - hash, - circuit, - debug, - abi, - file_map, - noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(), - }, + Ok(CompiledProgram { + hash, + circuit, + debug, + abi, + file_map, + noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(), warnings, - )) + }) } diff --git a/compiler/noirc_driver/src/program.rs b/compiler/noirc_driver/src/program.rs index fe991567180..a940f6b20b8 100644 --- a/compiler/noirc_driver/src/program.rs +++ b/compiler/noirc_driver/src/program.rs @@ -5,6 +5,7 @@ use fm::FileId; use base64::Engine; use noirc_errors::debug_info::DebugInfo; +use noirc_evaluator::errors::SsaReport; use serde::{de::Error as DeserializationError, ser::Error as SerializationError}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; @@ -24,6 +25,7 @@ pub struct CompiledProgram { pub abi: noirc_abi::Abi, pub debug: DebugInfo, pub file_map: BTreeMap, + pub warnings: Vec, } pub(crate) fn serialize_circuit(circuit: &Circuit, s: S) -> Result diff --git a/compiler/noirc_evaluator/Cargo.toml b/compiler/noirc_evaluator/Cargo.toml index d70c9d3179e..c9f5f28478b 100644 --- a/compiler/noirc_evaluator/Cargo.toml +++ b/compiler/noirc_evaluator/Cargo.toml @@ -16,4 +16,5 @@ fxhash.workspace = true iter-extended.workspace = true thiserror.workspace = true num-bigint = "0.4" -im = "15.1" +im = { version = "15.1", features = ["serde"] } +serde.workspace = true diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index 42381d6441e..2429ca9e194 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -13,6 +13,7 @@ use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; use crate::ssa::ir::{dfg::CallStack, types::NumericType}; +use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Clone, Error)] pub enum RuntimeError { @@ -53,7 +54,7 @@ fn format_failed_constraint(message: &Option) -> String { } } -#[derive(Debug)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub enum SsaReport { Warning(InternalWarning), } @@ -78,7 +79,7 @@ impl From for FileDiagnostic { } } -#[derive(Debug, PartialEq, Eq, Clone, Error)] +#[derive(Debug, PartialEq, Eq, Clone, Error, Serialize, Deserialize)] pub enum InternalWarning { #[error("Returning a constant value is not allowed")] ReturnConstant { call_stack: CallStack }, diff --git a/tooling/nargo/src/artifacts/debug.rs b/tooling/nargo/src/artifacts/debug.rs index d2e2eb9f311..19d8b88e641 100644 --- a/tooling/nargo/src/artifacts/debug.rs +++ b/tooling/nargo/src/artifacts/debug.rs @@ -1,6 +1,7 @@ use codespan_reporting::files::{Error, Files, SimpleFile}; use noirc_driver::DebugFile; use noirc_errors::{debug_info::DebugInfo, Location}; +use noirc_evaluator::errors::SsaReport; use serde::{Deserialize, Serialize}; use std::{ collections::{BTreeMap, BTreeSet}, @@ -15,6 +16,7 @@ use fm::{FileId, FileManager, PathString}; pub struct DebugArtifact { pub debug_symbols: Vec, pub file_map: BTreeMap, + pub warnings: Vec, } impl DebugArtifact { @@ -43,7 +45,7 @@ impl DebugArtifact { ); } - Self { debug_symbols, file_map } + Self { debug_symbols, file_map, warnings: Vec::new() } } /// Given a location, returns its file's source code diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 6e0e553abf3..d2ef2659e4d 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -23,7 +23,7 @@ pub fn run_test( ) -> TestStatus { let program = compile_no_check(context, config, test_function.get_id(), None, false); match program { - Ok((program, _)) => { + Ok(program) => { // Run the backend to ensure the PWG evaluates functions like std::hash::pedersen, // otherwise constraints involving these expressions will not error. let circuit_execution = diff --git a/tooling/nargo_cli/src/cli/compile_cmd.rs b/tooling/nargo_cli/src/cli/compile_cmd.rs index 039fee5dbc4..d018cf79651 100644 --- a/tooling/nargo_cli/src/cli/compile_cmd.rs +++ b/tooling/nargo_cli/src/cli/compile_cmd.rs @@ -188,6 +188,7 @@ fn compile_program( noir_version: preprocessed_program.noir_version, debug: debug_artifact.debug_symbols.remove(0), file_map: debug_artifact.file_map, + warnings: debug_artifact.warnings, }) } else { None @@ -261,8 +262,11 @@ fn save_program(program: CompiledProgram, package: &Package, circuit_dir: &Path) save_program_to_file(&preprocessed_program, &package.name, circuit_dir); - let debug_artifact = - DebugArtifact { debug_symbols: vec![program.debug], file_map: program.file_map }; + let debug_artifact = DebugArtifact { + debug_symbols: vec![program.debug], + file_map: program.file_map, + warnings: program.warnings, + }; let circuit_name: String = (&package.name).into(); save_debug_artifact_to_file(&debug_artifact, &circuit_name, circuit_dir); } @@ -275,6 +279,7 @@ fn save_contract(contract: CompiledContract, package: &Package, circuit_dir: &Pa let debug_artifact = DebugArtifact { debug_symbols: contract.functions.iter().map(|function| function.debug.clone()).collect(), file_map: contract.file_map, + warnings: contract.warnings, }; let preprocessed_functions = vecmap(contract.functions, |func| PreprocessedContractFunction { diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 1f2daa6caae..2c46af96bbb 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -112,6 +112,7 @@ pub(crate) fn debug_program( let debug_artifact = DebugArtifact { debug_symbols: vec![compiled_program.debug.clone()], file_map: compiled_program.file_map.clone(), + warnings: compiled_program.warnings.clone(), }; noir_debugger::debug_circuit( diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 1819c9a6f06..3f05b5b75f2 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -113,6 +113,7 @@ pub(crate) fn execute_program( let debug_artifact = DebugArtifact { debug_symbols: vec![compiled_program.debug.clone()], file_map: compiled_program.file_map.clone(), + warnings: compiled_program.warnings.clone(), }; if let Some(diagnostic) = try_to_diagnose_runtime_error(&err, &compiled_program.debug) {