fix(3300): cache warnings into debug artefacts (#3313)

Co-authored-by: Savio <[email protected]> Co-authored-by: José Pedro Sousa <[email protected]> Co-authored-by: jfecher <[email protected]> Co-authored-by: Tom French <[email protected]>
noir-lang · Oct 27, 2023 · cb5a15b · cb5a15b
1 parent 31c493c
commit cb5a15b
Show file tree

Hide file tree

Showing 11 changed files with 41 additions and 25 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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<ContractEvent>,
 
     pub file_map: BTreeMap<FileId, DebugFile>,
+    pub warnings: Vec<SsaReport>,
 }
 
 /// Each function in the contract will be compiled

diff --git 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<CompiledContract, ErrorsAndWarnings> {
     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<CompiledProgram>,
     force_compile: bool,
-) -> Result<(CompiledProgram, Vec<SsaReport>), RuntimeError> {
+) -> Result<CompiledProgram, RuntimeError> {
     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
@@ -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<FileId, DebugFile>,
+    pub warnings: Vec<SsaReport>,
 }
 
 pub(crate) fn serialize_circuit<S>(circuit: &Circuit, s: S) -> Result<S::Ok, S::Error>

diff --git 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
@@ -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>) -> String {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
 pub enum SsaReport {
     Warning(InternalWarning),
 }
@@ -78,7 +79,7 @@ impl From<SsaReport> 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
@@ -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<DebugInfo>,
     pub file_map: BTreeMap<FileId, DebugFile>,
+    pub warnings: Vec<SsaReport>,
 }
 
 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
@@ -23,7 +23,7 @@ pub fn run_test<B: BlackBoxFunctionSolver>(
 ) -> 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
@@ -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
@@ -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
@@ -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) {