Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Oct 27, 2023
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions compiler/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down
35 changes: 17 additions & 18 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -323,6 +323,7 @@ fn compile_contract_inner(
functions,
file_map,
noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(),
warnings,
})
} else {
Err(errors)
Expand All @@ -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);
Expand All @@ -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);
}
}
}
Expand All @@ -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,
))
})
}
2 changes: 2 additions & 0 deletions compiler/noirc_driver/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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>
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 3 additions & 2 deletions compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -53,7 +54,7 @@ fn format_failed_constraint(message: &Option<String>) -> String {
}
}

#[derive(Debug)]
#[derive(Debug, Clone, Serialize, Deserialize)]
pub enum SsaReport {
Warning(InternalWarning),
}
Expand All @@ -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 },
Expand Down
4 changes: 3 additions & 1 deletion tooling/nargo/src/artifacts/debug.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo/src/ops/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
9 changes: 7 additions & 2 deletions tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions tooling/nargo_cli/src/cli/debug_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions tooling/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down