Skip to content

Commit

Permalink
chore: export report_errors from nargo (#4461)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Related to #3503 

## Summary\*

This PR removes the extra copy of the `report_errors` function.

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
TomAFrench authored Mar 4, 2024
1 parent 8a6b131 commit 17f343b
Show file tree
Hide file tree
Showing 13 changed files with 98 additions and 110 deletions.
14 changes: 12 additions & 2 deletions tooling/lsp/src/requests/profile_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ use std::{

use acvm::acir::circuit::ExpressionWidth;
use async_lsp::{ErrorCode, ResponseError};
use nargo::{artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager};
use nargo::{
artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager,
ops::report_errors,
};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{
file_manager_with_stdlib, CompileOptions, DebugFile, NOIR_ARTIFACT_VERSION_STRING,
Expand Down Expand Up @@ -60,11 +63,18 @@ fn on_profile_run_request_inner(
Some(_package) => {
let expression_width = ExpressionWidth::Bounded { width: 3 };

let (compiled_programs, compiled_contracts) = nargo::ops::compile_workspace(
let compiled_workspace = nargo::ops::compile_workspace(
&workspace_file_manager,
&parsed_files,
&workspace,
&CompileOptions::default(),
);

let (compiled_programs, compiled_contracts) = report_errors(
compiled_workspace,
&workspace_file_manager,
CompileOptions::default().deny_warnings,
CompileOptions::default().silence_warnings,
)
.map_err(|err| ResponseError::new(ErrorCode::REQUEST_FAILED, err))?;

Expand Down
66 changes: 39 additions & 27 deletions tooling/nargo/src/ops/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn compile_workspace(
parsed_files: &ParsedFiles,
workspace: &Workspace,
compile_options: &CompileOptions,
) -> Result<(Vec<CompiledProgram>, Vec<CompiledContract>), CompileError> {
) -> CompilationResult<(Vec<CompiledProgram>, Vec<CompiledContract>)> {
let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace
.into_iter()
.filter(|package| !package.is_library())
Expand All @@ -38,31 +38,20 @@ pub fn compile_workspace(
.map(|package| compile_contract(file_manager, parsed_files, package, compile_options))
.collect();

// Report any warnings/errors which were encountered during compilation.
let compiled_programs: Vec<CompiledProgram> = program_results
.into_iter()
.map(|compilation_result| {
report_errors(
compilation_result,
file_manager,
compile_options.deny_warnings,
compile_options.silence_warnings,
)
})
.collect::<Result<_, _>>()?;
let compiled_contracts: Vec<CompiledContract> = contract_results
.into_iter()
.map(|compilation_result| {
report_errors(
compilation_result,
file_manager,
compile_options.deny_warnings,
compile_options.silence_warnings,
)
})
.collect::<Result<_, _>>()?;

Ok((compiled_programs, compiled_contracts))
// Collate any warnings/errors which were encountered during compilation.
let compiled_programs = collect_errors(program_results);
let compiled_contracts = collect_errors(contract_results);

match (compiled_programs, compiled_contracts) {
(Ok((programs, program_warnings)), Ok((contracts, contract_warnings))) => {
let warnings = [program_warnings, contract_warnings].concat();
Ok(((programs, contracts), warnings))
}
(Err(program_errors), Err(contract_errors)) => {
Err([program_errors, contract_errors].concat())
}
(Err(errors), _) | (_, Err(errors)) => Err(errors),
}
}

pub fn compile_program(
Expand Down Expand Up @@ -107,7 +96,30 @@ pub fn compile_contract(
noirc_driver::compile_contract(&mut context, crate_id, compile_options)
}

pub(crate) fn report_errors<T>(
/// Constructs a single `CompilationResult` for a collection of `CompilationResult`s, merging the set of warnings/errors.
pub fn collect_errors<T>(results: Vec<CompilationResult<T>>) -> CompilationResult<Vec<T>> {
let mut artifacts = Vec::new();
let mut warnings = Vec::new();
let mut errors = Vec::new();

for result in results {
match result {
Ok((new_artifact, new_warnings)) => {
artifacts.push(new_artifact);
warnings.extend(new_warnings);
}
Err(new_errors) => errors.extend(new_errors),
}
}

if errors.is_empty() {
Ok((artifacts, warnings))
} else {
Err(errors)
}
}

pub fn report_errors<T>(
result: CompilationResult<T>,
file_manager: &FileManager,
deny_warnings: bool,
Expand Down
3 changes: 2 additions & 1 deletion tooling/nargo/src/ops/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub use self::compile::{
compile_contract, compile_program, compile_program_with_debug_instrumenter, compile_workspace,
collect_errors, compile_contract, compile_program, compile_program_with_debug_instrumenter,
compile_workspace, report_errors,
};
pub use self::execute::execute_circuit;
pub use self::foreign_calls::{
Expand Down
11 changes: 3 additions & 8 deletions tooling/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use clap::Args;
use fm::FileManager;
use iter_extended::btree_map;
use nargo::{
errors::CompileError, insert_all_files_for_workspace_into_file_manager, package::Package,
parse_all, prepare_package,
errors::CompileError, insert_all_files_for_workspace_into_file_manager, ops::report_errors,
package::Package, parse_all, prepare_package,
};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME};
Expand Down Expand Up @@ -152,12 +152,7 @@ pub(crate) fn check_crate_and_report_errors(
silence_warnings: bool,
) -> Result<(), CompileError> {
let result = check_crate(context, crate_id, deny_warnings, disable_macros);
super::compile_cmd::report_errors(
result,
&context.file_manager,
deny_warnings,
silence_warnings,
)
report_errors(result, &context.file_manager, deny_warnings, silence_warnings)
}

#[cfg(test)]
Expand Down
3 changes: 1 addition & 2 deletions tooling/nargo_cli/src/cli/codegen_verifier_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use super::fs::{create_named_dir, write_to_file};
use super::NargoConfig;
use crate::backends::Backend;
use crate::cli::compile_cmd::report_errors;
use crate::errors::CliError;

use clap::Args;
use nargo::ops::compile_program;
use nargo::ops::{compile_program, report_errors};
use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{file_manager_with_stdlib, CompileOptions, NOIR_ARTIFACT_VERSION_STRING};
Expand Down
82 changes: 25 additions & 57 deletions tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use std::path::Path;

use fm::FileManager;
use nargo::artifacts::program::ProgramArtifact;
use nargo::errors::CompileError;
use nargo::ops::{compile_contract, compile_program};
use nargo::ops::{collect_errors, compile_contract, compile_program, report_errors};
use nargo::package::Package;
use nargo::workspace::Workspace;
use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all};
Expand Down Expand Up @@ -65,11 +64,18 @@ pub(crate) fn run(
.compile_options
.expression_width
.unwrap_or_else(|| backend.get_backend_info_or_default());
let (compiled_program, compiled_contracts) = compile_workspace(
let compiled_workspace = compile_workspace(
&workspace_file_manager,
&parsed_files,
&workspace,
&args.compile_options,
);

let (compiled_programs, compiled_contracts) = report_errors(
compiled_workspace,
&workspace_file_manager,
args.compile_options.deny_warnings,
args.compile_options.silence_warnings,
)?;

let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace
Expand All @@ -80,7 +86,7 @@ pub(crate) fn run(

// Save build artifacts to disk.
let only_acir = args.compile_options.only_acir;
for (package, program) in binary_packages.into_iter().zip(compiled_program) {
for (package, program) in binary_packages.into_iter().zip(compiled_programs) {
let program = nargo::ops::transform_program(program, expression_width);
save_program(program.clone(), &package, &workspace.target_directory_path(), only_acir);
}
Expand All @@ -97,7 +103,7 @@ pub(super) fn compile_workspace(
parsed_files: &ParsedFiles,
workspace: &Workspace,
compile_options: &CompileOptions,
) -> Result<(Vec<CompiledProgram>, Vec<CompiledContract>), CliError> {
) -> CompilationResult<(Vec<CompiledProgram>, Vec<CompiledContract>)> {
let (binary_packages, contract_packages): (Vec<_>, Vec<_>) = workspace
.into_iter()
.filter(|package| !package.is_library())
Expand All @@ -123,31 +129,20 @@ pub(super) fn compile_workspace(
.map(|package| compile_contract(file_manager, parsed_files, package, compile_options))
.collect();

// Report any warnings/errors which were encountered during compilation.
let compiled_programs: Vec<CompiledProgram> = program_results
.into_iter()
.map(|compilation_result| {
report_errors(
compilation_result,
file_manager,
compile_options.deny_warnings,
compile_options.silence_warnings,
)
})
.collect::<Result<_, _>>()?;
let compiled_contracts: Vec<CompiledContract> = contract_results
.into_iter()
.map(|compilation_result| {
report_errors(
compilation_result,
file_manager,
compile_options.deny_warnings,
compile_options.silence_warnings,
)
})
.collect::<Result<_, _>>()?;

Ok((compiled_programs, compiled_contracts))
// Collate any warnings/errors which were encountered during compilation.
let compiled_programs = collect_errors(program_results);
let compiled_contracts = collect_errors(contract_results);

match (compiled_programs, compiled_contracts) {
(Ok((programs, program_warnings)), Ok((contracts, contract_warnings))) => {
let warnings = [program_warnings, contract_warnings].concat();
Ok(((programs, contracts), warnings))
}
(Err(program_errors), Err(contract_errors)) => {
Err([program_errors, contract_errors].concat())
}
(Err(errors), _) | (_, Err(errors)) => Err(errors),
}
}

pub(super) fn save_program(
Expand All @@ -172,30 +167,3 @@ fn save_contract(contract: CompiledContract, package: &Package, circuit_dir: &Pa
circuit_dir,
);
}

/// Helper function for reporting any errors in a `CompilationResult<T>`
/// structure that is commonly used as a return result in this file.
pub(crate) fn report_errors<T>(
result: CompilationResult<T>,
file_manager: &FileManager,
deny_warnings: bool,
silence_warnings: bool,
) -> Result<T, CompileError> {
let (t, warnings) = result.map_err(|errors| {
noirc_errors::reporter::report_all(
file_manager.as_file_map(),
&errors,
deny_warnings,
silence_warnings,
)
})?;

noirc_errors::reporter::report_all(
file_manager.as_file_map(),
&warnings,
deny_warnings,
silence_warnings,
);

Ok(t)
}
3 changes: 1 addition & 2 deletions tooling/nargo_cli/src/cli/debug_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use fm::FileManager;
use nargo::artifacts::debug::DebugArtifact;
use nargo::constants::PROVER_INPUT_FILE;
use nargo::errors::CompileError;
use nargo::ops::{compile_program, compile_program_with_debug_instrumenter};
use nargo::ops::{compile_program, compile_program_with_debug_instrumenter, report_errors};
use nargo::package::Package;
use nargo::workspace::Workspace;
use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all};
Expand All @@ -22,7 +22,6 @@ use noirc_frontend::debug::DebugInstrumenter;
use noirc_frontend::graph::CrateName;
use noirc_frontend::hir::ParsedFiles;

use super::compile_cmd::report_errors;
use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir};
use super::NargoConfig;
use crate::backends::Backend;
Expand Down
3 changes: 1 addition & 2 deletions tooling/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use clap::Args;
use nargo::artifacts::debug::DebugArtifact;
use nargo::constants::PROVER_INPUT_FILE;
use nargo::errors::try_to_diagnose_runtime_error;
use nargo::ops::{compile_program, DefaultForeignCallExecutor};
use nargo::ops::{compile_program, report_errors, DefaultForeignCallExecutor};
use nargo::package::Package;
use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
Expand All @@ -19,7 +19,6 @@ use noirc_frontend::graph::CrateName;
use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir};
use super::NargoConfig;
use crate::backends::Backend;
use crate::cli::compile_cmd::report_errors;
use crate::errors::CliError;

/// Executes a circuit to calculate its return value
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/src/cli/export_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use nargo::errors::CompileError;
use nargo::ops::report_errors;
use noirc_errors::FileDiagnostic;
use noirc_frontend::hir::ParsedFiles;
use rayon::prelude::*;
Expand All @@ -24,7 +25,6 @@ use crate::errors::CliError;

use super::check_cmd::check_crate_and_report_errors;

use super::compile_cmd::report_errors;
use super::fs::program::save_program_to_file;
use super::NargoConfig;

Expand Down
4 changes: 2 additions & 2 deletions tooling/nargo_cli/src/cli/fmt_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{fs::DirEntry, path::Path};

use clap::Args;
use nargo::insert_all_files_for_workspace_into_file_manager;
use nargo::{insert_all_files_for_workspace_into_file_manager, ops::report_errors};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{file_manager_with_stdlib, NOIR_ARTIFACT_VERSION_STRING};
use noirc_errors::CustomDiagnostic;
Expand Down Expand Up @@ -53,7 +53,7 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr
})
.collect();

let _ = super::compile_cmd::report_errors::<()>(
let _ = report_errors::<()>(
Err(errors),
&workspace_file_manager,
false,
Expand Down
11 changes: 9 additions & 2 deletions tooling/nargo_cli/src/cli/info_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use clap::Args;
use iter_extended::vecmap;
use nargo::{
artifacts::debug::DebugArtifact, insert_all_files_for_workspace_into_file_manager,
package::Package, parse_all,
ops::report_errors, package::Package, parse_all,
};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{
Expand Down Expand Up @@ -73,11 +73,18 @@ pub(crate) fn run(
.compile_options
.expression_width
.unwrap_or_else(|| backend.get_backend_info_or_default());
let (compiled_programs, compiled_contracts) = compile_workspace(
let compiled_workspace = compile_workspace(
&workspace_file_manager,
&parsed_files,
&workspace,
&args.compile_options,
);

let (compiled_programs, compiled_contracts) = report_errors(
compiled_workspace,
&workspace_file_manager,
args.compile_options.deny_warnings,
args.compile_options.silence_warnings,
)?;

let compiled_programs = vecmap(compiled_programs, |program| {
Expand Down
Loading

0 comments on commit 17f343b

Please sign in to comment.