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

chore: remove FileDiagnostic #7546

Merged
merged 2 commits into from
Feb 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 23 additions & 29 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use clap::Args;
use fm::{FileId, FileManager};
use iter_extended::vecmap;
use noirc_abi::{AbiParameter, AbiType, AbiValue};
use noirc_errors::{CustomDiagnostic, DiagnosticKind, FileDiagnostic};
use noirc_errors::{CustomDiagnostic, DiagnosticKind};
use noirc_evaluator::brillig::BrilligOptions;
use noirc_evaluator::create_program;
use noirc_evaluator::errors::RuntimeError;
Expand Down Expand Up @@ -227,8 +227,8 @@ impl From<RuntimeError> for CompileError {
}
}

impl From<CompileError> for FileDiagnostic {
fn from(error: CompileError) -> FileDiagnostic {
impl From<CompileError> for CustomDiagnostic {
fn from(error: CompileError) -> CustomDiagnostic {
match error {
CompileError::RuntimeError(err) => err.into(),
CompileError::MonomorphizationError(err) => err.into(),
Expand All @@ -237,10 +237,10 @@ impl From<CompileError> for FileDiagnostic {
}

/// Helper type used to signify where only warnings are expected in file diagnostics
pub type Warnings = Vec<FileDiagnostic>;
pub type Warnings = Vec<CustomDiagnostic>;

/// Helper type used to signify where errors or warnings are expected in file diagnostics
pub type ErrorsAndWarnings = Vec<FileDiagnostic>;
pub type ErrorsAndWarnings = Vec<CustomDiagnostic>;

/// Helper type for connecting a compilation artifact to the errors or warnings which were produced during compilation.
pub type CompilationResult<T> = Result<(T, Warnings), ErrorsAndWarnings>;
Expand Down Expand Up @@ -350,20 +350,16 @@ pub fn check_crate(
) -> CompilationResult<()> {
let diagnostics = CrateDefMap::collect_defs(crate_id, context, options.frontend_options());
let crate_files = context.crate_files(&crate_id);
let warnings_and_errors: Vec<FileDiagnostic> = diagnostics
.into_iter()
.map(|error| {
let location = error.location();
let diagnostic = CustomDiagnostic::from(&error);
diagnostic.in_file(location.file)
})
let warnings_and_errors: Vec<CustomDiagnostic> = diagnostics
.iter()
.map(CustomDiagnostic::from)
.filter(|diagnostic| {
// We filter out any warnings if they're going to be ignored later on to free up memory.
!options.silence_warnings || diagnostic.diagnostic.kind != DiagnosticKind::Warning
!options.silence_warnings || diagnostic.kind != DiagnosticKind::Warning
})
.filter(|error| {
// Only keep warnings from the crate we are checking
if error.diagnostic.is_warning() { crate_files.contains(&error.file_id) } else { true }
if error.is_warning() { crate_files.contains(&error.file) } else { true }
})
.collect();

Expand Down Expand Up @@ -401,16 +397,16 @@ pub fn compile_main(
// TODO(#2155): This error might be a better to exist in Nargo
let err = CustomDiagnostic::from_message(
"cannot compile crate into a program as it does not contain a `main` function",
)
.in_file(FileId::default());
FileId::default(),
);
vec![err]
})?;

let compiled_program =
compile_no_check(context, options, main, cached_program, options.force_compile)
.map_err(FileDiagnostic::from)?;
.map_err(|error| vec![CustomDiagnostic::from(error)])?;

let compilation_warnings = vecmap(compiled_program.warnings.clone(), FileDiagnostic::from);
let compilation_warnings = vecmap(compiled_program.warnings.clone(), CustomDiagnostic::from);
if options.deny_warnings && !compilation_warnings.is_empty() {
return Err(compilation_warnings);
}
Expand Down Expand Up @@ -439,14 +435,16 @@ pub fn compile_contract(
let mut errors = warnings;

if contracts.len() > 1 {
let err = CustomDiagnostic::from_message("Packages are limited to a single contract")
.in_file(FileId::default());
let err = CustomDiagnostic::from_message(
"Packages are limited to a single contract",
FileId::default(),
);
return Err(vec![err]);
} else if contracts.is_empty() {
let err = CustomDiagnostic::from_message(
"cannot compile crate into a contract as it does not contain any contracts",
)
.in_file(FileId::default());
FileId::default(),
);
return Err(vec![err]);
};

Expand Down Expand Up @@ -483,12 +481,8 @@ pub fn compile_contract(
}

/// True if there are (non-warning) errors present and we should halt compilation
fn has_errors(errors: &[FileDiagnostic], deny_warnings: bool) -> bool {
if deny_warnings {
!errors.is_empty()
} else {
errors.iter().any(|error| error.diagnostic.is_error())
}
fn has_errors(errors: &[CustomDiagnostic], deny_warnings: bool) -> bool {
if deny_warnings { !errors.is_empty() } else { errors.iter().any(|error| error.is_error()) }
}

/// Compile all of the functions associated with a Noir contract.
Expand Down Expand Up @@ -525,7 +519,7 @@ fn compile_contract_inner(
let function = match compile_no_check(context, &options, function_id, None, true) {
Ok(function) => function,
Err(new_error) => {
errors.push(FileDiagnostic::from(new_error));
errors.push(new_error.into());
continue;
}
};
Expand Down
8 changes: 4 additions & 4 deletions compiler/noirc_driver/tests/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ contract Bar {}";

assert_eq!(
errors,
vec![
CustomDiagnostic::from_message("Packages are limited to a single contract")
.in_file(FileId::default())
],
vec![CustomDiagnostic::from_message(
"Packages are limited to a single contract",
FileId::default()
)],
"stdlib is producing warnings"
);

Expand Down
18 changes: 0 additions & 18 deletions compiler/noirc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,3 @@ mod position;
pub mod reporter;
pub use position::{Located, Location, Position, Span, Spanned};
pub use reporter::{CustomDiagnostic, DiagnosticKind};

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct FileDiagnostic {
pub file_id: fm::FileId,
pub diagnostic: CustomDiagnostic,
}

impl FileDiagnostic {
pub fn new(file_id: fm::FileId, diagnostic: CustomDiagnostic) -> FileDiagnostic {
FileDiagnostic { file_id, diagnostic }
}
}

impl From<FileDiagnostic> for Vec<FileDiagnostic> {
fn from(value: FileDiagnostic) -> Self {
vec![value]
}
}
22 changes: 11 additions & 11 deletions compiler/noirc_errors/src/reporter.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use std::io::IsTerminal;

use crate::{FileDiagnostic, Location, Span};
use crate::{Location, Span};
use codespan_reporting::diagnostic::{Diagnostic, Label};
use codespan_reporting::files::Files;
use codespan_reporting::term;
use codespan_reporting::term::termcolor::{ColorChoice, StandardStream};

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct CustomDiagnostic {
pub file: fm::FileId,
pub message: String,
pub secondaries: Vec<CustomLabel>,
pub notes: Vec<String>,
Expand Down Expand Up @@ -35,8 +36,9 @@ pub struct ReportedErrors {
}

impl CustomDiagnostic {
pub fn from_message(msg: &str) -> CustomDiagnostic {
pub fn from_message(msg: &str, file: fm::FileId) -> CustomDiagnostic {
Self {
file,
message: msg.to_owned(),
secondaries: Vec::new(),
notes: Vec::new(),
Expand All @@ -54,6 +56,7 @@ impl CustomDiagnostic {
kind: DiagnosticKind,
) -> CustomDiagnostic {
CustomDiagnostic {
file: secondary_location.file,
message: primary_message,
secondaries: vec![CustomLabel::new(secondary_message, secondary_location)],
notes: Vec::new(),
Expand Down Expand Up @@ -109,6 +112,7 @@ impl CustomDiagnostic {
secondary_location: Location,
) -> CustomDiagnostic {
CustomDiagnostic {
file: secondary_location.file,
message: primary_message,
secondaries: vec![CustomLabel::new(secondary_message, secondary_location)],
notes: Vec::new(),
Expand All @@ -119,10 +123,6 @@ impl CustomDiagnostic {
}
}

pub fn in_file(self, file_id: fm::FileId) -> FileDiagnostic {
FileDiagnostic::new(file_id, self)
}

pub fn with_call_stack(mut self, call_stack: Vec<Location>) -> Self {
self.call_stack = call_stack;
self
Expand Down Expand Up @@ -185,16 +185,16 @@ impl CustomLabel {
/// of diagnostics that were errors.
pub fn report_all<'files>(
files: &'files impl Files<'files, FileId = fm::FileId>,
diagnostics: &[FileDiagnostic],
diagnostics: &[CustomDiagnostic],
deny_warnings: bool,
silence_warnings: bool,
) -> ReportedErrors {
// Report warnings before any errors
let (warnings_and_bugs, mut errors): (Vec<_>, _) =
diagnostics.iter().partition(|item| !item.diagnostic.is_error());
diagnostics.iter().partition(|item| !item.is_error());

let (warnings, mut bugs): (Vec<_>, _) =
warnings_and_bugs.iter().partition(|item| item.diagnostic.is_warning());
warnings_and_bugs.iter().partition(|item| item.is_warning());
let mut diagnostics = if silence_warnings { Vec::new() } else { warnings };
diagnostics.append(&mut bugs);
diagnostics.append(&mut errors);
Expand All @@ -205,14 +205,14 @@ pub fn report_all<'files>(
ReportedErrors { error_count }
}

impl FileDiagnostic {
impl CustomDiagnostic {
/// Print the report; return true if it was an error.
pub fn report<'files>(
&self,
files: &'files impl Files<'files, FileId = fm::FileId>,
deny_warnings: bool,
) -> bool {
report(files, &self.diagnostic, deny_warnings)
report(files, self, deny_warnings)
}
}

Expand Down
33 changes: 16 additions & 17 deletions compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//!
//! An Error of the latter is an error in the implementation of the compiler
use iter_extended::vecmap;
use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic, Location};
use noirc_errors::{CustomDiagnostic, Location};
use noirc_frontend::signed_field::SignedField;
use thiserror::Error;

Expand Down Expand Up @@ -73,8 +73,8 @@ pub enum SsaReport {
Bug(InternalBug),
}

impl From<SsaReport> for FileDiagnostic {
fn from(error: SsaReport) -> FileDiagnostic {
impl From<SsaReport> for CustomDiagnostic {
fn from(error: SsaReport) -> CustomDiagnostic {
match error {
SsaReport::Warning(warning) => {
let message = warning.to_string();
Expand All @@ -87,10 +87,10 @@ impl From<SsaReport> for FileDiagnostic {
},
};
let call_stack = vecmap(call_stack, |location| location);
let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
let location = call_stack.last().expect("Expected RuntimeError to have a location");
let diagnostic = Diagnostic::simple_warning(message, secondary_message, *location);
diagnostic.with_call_stack(call_stack).in_file(file_id)
let diagnostic =
CustomDiagnostic::simple_warning(message, secondary_message, *location);
diagnostic.with_call_stack(call_stack)
}
SsaReport::Bug(bug) => {
let message = bug.to_string();
Expand All @@ -104,10 +104,10 @@ impl From<SsaReport> for FileDiagnostic {
InternalBug::AssertFailed { call_stack } => ("As a result, the compiled circuit is ensured to fail. Other assertions may also fail during execution".to_string(), call_stack)
};
let call_stack = vecmap(call_stack, |location| location);
let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
let location = call_stack.last().expect("Expected RuntimeError to have a location");
let diagnostic = Diagnostic::simple_bug(message, secondary_message, *location);
diagnostic.with_call_stack(call_stack).in_file(file_id)
let diagnostic =
CustomDiagnostic::simple_bug(message, secondary_message, *location);
diagnostic.with_call_stack(call_stack)
}
}
}
Expand Down Expand Up @@ -181,20 +181,19 @@ impl RuntimeError {
}
}

impl From<RuntimeError> for FileDiagnostic {
fn from(error: RuntimeError) -> FileDiagnostic {
impl From<RuntimeError> for CustomDiagnostic {
fn from(error: RuntimeError) -> CustomDiagnostic {
let call_stack = vecmap(error.call_stack(), |location| *location);
let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
let diagnostic = error.into_diagnostic();
diagnostic.with_call_stack(call_stack).in_file(file_id)
diagnostic.with_call_stack(call_stack)
}
}

impl RuntimeError {
fn into_diagnostic(self) -> Diagnostic {
fn into_diagnostic(self) -> CustomDiagnostic {
match self {
RuntimeError::InternalError(cause) => {
Diagnostic::simple_error(
CustomDiagnostic::simple_error(
"Internal Consistency Evaluators Errors: \n
This is likely a bug. Consider opening an issue at https://github.com/noir-lang/noir/issues".to_owned(),
cause.to_string(),
Expand All @@ -206,7 +205,7 @@ impl RuntimeError {
let location =
self.call_stack().last().expect("Expected RuntimeError to have a location");

Diagnostic::simple_error(
CustomDiagnostic::simple_error(
primary_message,
"If attempting to fetch the length of a slice, try converting to an array. Slices only use dynamic lengths.".to_string(),
*location,
Expand All @@ -217,7 +216,7 @@ impl RuntimeError {
let location =
self.call_stack().last().unwrap_or_else(|| panic!("Expected RuntimeError to have a location. Error message: {message}"));

Diagnostic::simple_error(message, String::new(), *location)
CustomDiagnostic::simple_error(message, String::new(), *location)
}
}
}
Expand Down
5 changes: 0 additions & 5 deletions compiler/noirc_frontend/src/hir/def_collector/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::ast::{Ident, ItemVisibility, Path, UnsupportedNumericGenericType};
use crate::hir::resolution::import::PathResolutionError;
use crate::hir::type_check::generics::TraitGenerics;

use noirc_errors::FileDiagnostic;
use noirc_errors::{CustomDiagnostic as Diagnostic, Location};
use thiserror::Error;

Expand Down Expand Up @@ -76,10 +75,6 @@ pub enum DefCollectorErrorKind {
}

impl DefCollectorErrorKind {
pub fn into_file_diagnostic(&self, file: fm::FileId) -> FileDiagnostic {
Diagnostic::from(self).in_file(file)
}

pub fn location(&self) -> Location {
match self {
DefCollectorErrorKind::Duplicate { first_def: ident, .. }
Expand Down
6 changes: 1 addition & 5 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use acvm::FieldElement;
pub use noirc_errors::Span;
use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic, Location};
use noirc_errors::{CustomDiagnostic as Diagnostic, Location};
use thiserror::Error;

use crate::{
Expand Down Expand Up @@ -195,10 +195,6 @@ pub enum ResolverError {
}

impl ResolverError {
pub fn into_file_diagnostic(&self, file: fm::FileId) -> FileDiagnostic {
Diagnostic::from(self).in_file(file)
}

pub fn location(&self) -> Location {
match self {
ResolverError::DuplicateDefinition { first_location: location, .. }
Expand Down
Loading
Loading