feat: let all compiler errors carry a Location instead of a Span

compiler/noirc_driver/src/abi_gen.rs

@@ -6,7 +6,7 @@ use iter_extended::vecmap;
 use noirc_abi::{
     Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue, AbiVisibility, Sign,
 };
-use noirc_errors::Span;
+use noirc_errors::Location;
 use noirc_evaluator::ErrorType;
 use noirc_frontend::ast::{Signedness, Visibility};
 use noirc_frontend::TypeBinding;
@@ -42,19 +42,19 @@ pub(super) fn gen_abi(
 }
 
 // Get the Span of the root crate's main function, or else a dummy span if that fails
-fn get_main_function_span(context: &Context) -> Span {
+fn get_main_function_location(context: &Context) -> Location {
     if let Some(func_id) = context.get_main_function(context.root_crate_id()) {
-        context.function_meta(&func_id).location.span
+        context.function_meta(&func_id).location
     } else {
-        Span::default()
+        Location::dummy()
     }
 }
 
 fn build_abi_error_type(context: &Context, typ: ErrorType) -> AbiErrorType {
     match typ {
         ErrorType::Dynamic(typ) => {
             if let Type::FmtString(len, item_types) = typ {
-                let span = get_main_function_span(context);
+                let span = get_main_function_location(context);
                 let length = len.evaluate_to_u32(span).expect("Cannot evaluate fmt length");
                 let Type::Tuple(item_types) = item_types.as_ref() else {
                     unreachable!("FmtString items must be a tuple")
@@ -74,7 +74,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
     match typ {
         Type::FieldElement => AbiType::Field,
         Type::Array(size, typ) => {
-            let span = get_main_function_span(context);
+            let span = get_main_function_location(context);
             let length = size
                 .evaluate_to_u32(span)
                 .expect("Cannot have variable sized arrays as a parameter to main");
@@ -103,7 +103,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
         }
         Type::Bool => AbiType::Boolean,
         Type::String(size) => {
-            let span = get_main_function_span(context);
+            let span = get_main_function_location(context);
             let size = size
                 .evaluate_to_u32(span)
                 .expect("Cannot have variable sized strings as a parameter to main");

compiler/noirc_driver/src/lib.rs

@@ -352,9 +352,10 @@ pub fn check_crate(
     let crate_files = context.crate_files(&crate_id);
     let warnings_and_errors: Vec<FileDiagnostic> = diagnostics
         .into_iter()
-        .map(|(error, file_id)| {
+        .map(|error| {
+            let location = error.location();
             let diagnostic = CustomDiagnostic::from(&error);
-            diagnostic.in_file(file_id)
+            diagnostic.in_file(location.file)
         })
         .filter(|diagnostic| {
             // We filter out any warnings if they're going to be ignored later on to free up memory.

compiler/noirc_errors/src/reporter.rs

@@ -50,12 +50,12 @@ impl CustomDiagnostic {
     fn simple_with_kind(
         primary_message: String,
         secondary_message: String,
-        secondary_span: Span,
+        secondary_location: Location,
         kind: DiagnosticKind,
     ) -> CustomDiagnostic {
         CustomDiagnostic {
             message: primary_message,
-            secondaries: vec![CustomLabel::new(secondary_message, secondary_span, None)],
+            secondaries: vec![CustomLabel::new(secondary_message, secondary_location)],
             notes: Vec::new(),
             kind,
             deprecated: false,
@@ -67,50 +67,50 @@ impl CustomDiagnostic {
     pub fn simple_error(
         primary_message: String,
         secondary_message: String,
-        secondary_span: Span,
+        secondary_location: Location,
     ) -> CustomDiagnostic {
         Self::simple_with_kind(
             primary_message,
             secondary_message,
-            secondary_span,
+            secondary_location,
             DiagnosticKind::Error,
         )
     }
 
     pub fn simple_warning(
         primary_message: String,
         secondary_message: String,
-        secondary_span: Span,
+        secondary_location: Location,
     ) -> CustomDiagnostic {
         Self::simple_with_kind(
             primary_message,
             secondary_message,
-            secondary_span,
+            secondary_location,
             DiagnosticKind::Warning,
         )
     }
 
     pub fn simple_info(
         primary_message: String,
         secondary_message: String,
-        secondary_span: Span,
+        secondary_location: Location,
     ) -> CustomDiagnostic {
         Self::simple_with_kind(
             primary_message,
             secondary_message,
-            secondary_span,
+            secondary_location,
             DiagnosticKind::Info,
         )
     }
 
     pub fn simple_bug(
         primary_message: String,
         secondary_message: String,
-        secondary_span: Span,
+        secondary_location: Location,
     ) -> CustomDiagnostic {
         CustomDiagnostic {
             message: primary_message,
-            secondaries: vec![CustomLabel::new(secondary_message, secondary_span, None)],
+            secondaries: vec![CustomLabel::new(secondary_message, secondary_location)],
             notes: Vec::new(),
             kind: DiagnosticKind::Bug,
             deprecated: false,
@@ -132,12 +132,8 @@ impl CustomDiagnostic {
         self.notes.push(message);
     }
 
-    pub fn add_secondary(&mut self, message: String, span: Span) {
-        self.secondaries.push(CustomLabel::new(message, span, None));
-    }
-
-    pub fn add_secondary_with_file(&mut self, message: String, span: Span, file: fm::FileId) {
-        self.secondaries.push(CustomLabel::new(message, span, Some(file)));
+    pub fn add_secondary(&mut self, message: String, location: Location) {
+        self.secondaries.push(CustomLabel::new(message, location));
     }
 
     pub fn is_error(&self) -> bool {
@@ -176,13 +172,12 @@ impl std::fmt::Display for CustomDiagnostic {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct CustomLabel {
     pub message: String,
-    pub span: Span,
-    pub file: Option<fm::FileId>,
+    pub location: Location,
 }
 
 impl CustomLabel {
-    fn new(message: String, span: Span, file: Option<fm::FileId>) -> CustomLabel {
-        CustomLabel { message, span, file }
+    fn new(message: String, location: Location) -> CustomLabel {
+        CustomLabel { message, location }
     }
 }
 
@@ -217,15 +212,14 @@ impl FileDiagnostic {
         files: &'files impl Files<'files, FileId = fm::FileId>,
         deny_warnings: bool,
     ) -> bool {
-        report(files, &self.diagnostic, Some(self.file_id), deny_warnings)
+        report(files, &self.diagnostic, deny_warnings)
     }
 }
 
 /// Report the given diagnostic, and return true if it was an error
 pub fn report<'files>(
     files: &'files impl Files<'files, FileId = fm::FileId>,
     custom_diagnostic: &CustomDiagnostic,
-    file: Option<fm::FileId>,
     deny_warnings: bool,
 ) -> bool {
     let color_choice =
@@ -234,15 +228,14 @@ pub fn report<'files>(
     let config = term::Config::default();
 
     let stack_trace = stack_trace(files, &custom_diagnostic.call_stack);
-    let diagnostic = convert_diagnostic(custom_diagnostic, file, stack_trace, deny_warnings);
+    let diagnostic = convert_diagnostic(custom_diagnostic, stack_trace, deny_warnings);
     term::emit(&mut writer.lock(), &config, files, &diagnostic).unwrap();
 
     deny_warnings || custom_diagnostic.is_error()
 }
 
 fn convert_diagnostic(
     cd: &CustomDiagnostic,
-    file: Option<fm::FileId>,
     stack_trace: String,
     deny_warnings: bool,
 ) -> Diagnostic<fm::FileId> {
@@ -253,19 +246,18 @@ fn convert_diagnostic(
         _ => Diagnostic::error(),
     };
 
-    let secondary_labels = if let Some(file_id) = file {
-        cd.secondaries
-            .iter()
-            .map(|sl| {
-                let start_span = sl.span.start() as usize;
-                let end_span = sl.span.end() as usize;
-                let file = sl.file.unwrap_or(file_id);
-                Label::secondary(file, start_span..end_span).with_message(&sl.message)
-            })
-            .collect()
-    } else {
-        vec![]
-    };
+    let secondary_labels = cd
+        .secondaries
+        .iter()
+        .map(|custom_label| {
+            let location = custom_label.location;
+            let span = location.span;
+            let start_span = span.start() as usize;
+            let end_span = span.end() as usize;
+            let file = location.file;
+            Label::secondary(file, start_span..end_span).with_message(&custom_label.message)
+        })
+        .collect();
 
     let mut notes = cd.notes.clone();
     notes.push(stack_trace);

compiler/noirc_evaluator/src/errors.rs

@@ -9,7 +9,7 @@
 //! An Error of the latter is an error in the implementation of the compiler
 use acvm::FieldElement;
 use iter_extended::vecmap;
-use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic};
+use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic, Location};
 use thiserror::Error;
 
 use crate::ssa::ir::{call_stack::CallStack, types::NumericType};
@@ -85,8 +85,7 @@ 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.span);
+                let diagnostic = Diagnostic::simple_warning(message, secondary_message, *location);
                 diagnostic.with_call_stack(call_stack).in_file(file_id)
             }
             SsaReport::Bug(bug) => {
@@ -103,7 +102,7 @@ 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_bug(message, secondary_message, location.span);
+                let diagnostic = Diagnostic::simple_bug(message, secondary_message, *location);
                 diagnostic.with_call_stack(call_stack).in_file(file_id)
             }
         }
@@ -195,7 +194,7 @@ impl RuntimeError {
                     "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(),
-                    noirc_errors::Span::inclusive(0, 0)
+                    Location::dummy(),
                 )
             }
             RuntimeError::UnknownLoopBound { .. } => {
@@ -206,15 +205,15 @@ impl RuntimeError {
                 Diagnostic::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.span,
+                    *location,
                 )
             }
             _ => {
                 let message = self.to_string();
                 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.span)
+                Diagnostic::simple_error(message, String::new(), *location)
             }
         }
     }

compiler/noirc_frontend/src/ast/expression.rs

@@ -40,7 +40,7 @@ pub enum ExpressionKind {
     Quote(Tokens),
     Unquote(Box<Expression>),
     Comptime(BlockExpression, Location),
-    Unsafe(BlockExpression, Location),
+    Unsafe(BlockExpression, Location, Location /* unsafe keyword location */),
     AsTraitPath(AsTraitPath),
     TypePath(TypePath),
 
@@ -255,7 +255,7 @@ impl Expression {
         match &self.kind {
             ExpressionKind::Block(block_expression)
             | ExpressionKind::Comptime(block_expression, _)
-            | ExpressionKind::Unsafe(block_expression, _) => {
+            | ExpressionKind::Unsafe(block_expression, _, _) => {
                 if let Some(statement) = block_expression.statements.last() {
                     statement.type_location()
                 } else {
@@ -663,7 +663,7 @@ impl Display for ExpressionKind {
             Lambda(lambda) => lambda.fmt(f),
             Parenthesized(sub_expr) => write!(f, "({sub_expr})"),
             Comptime(block, _) => write!(f, "comptime {block}"),
-            Unsafe(block, _) => write!(f, "unsafe {block}"),
+            Unsafe(block, _, _) => write!(f, "unsafe {block}"),
             Error => write!(f, "Error"),
             Resolved(_) => write!(f, "?Resolved"),
             Interned(_) => write!(f, "?Interned"),

compiler/noirc_frontend/src/ast/statement.rs

@@ -243,14 +243,9 @@ impl From<LocatedToken> for Ident {
 
 impl From<Ident> for Expression {
     fn from(i: Ident) -> Expression {
-        Expression {
-            location: i.0.location(),
-            kind: ExpressionKind::Variable(Path {
-                location: i.location(),
-                segments: vec![PathSegment::from(i)],
-                kind: PathKind::Plain,
-            }),
-        }
+        let location = i.location();
+        let kind = ExpressionKind::Variable(Path::plain(vec![PathSegment::from(i)], location));
+        Expression { location, kind }
     }
 }
 
@@ -390,9 +385,15 @@ pub struct Path {
     pub segments: Vec<PathSegment>,
     pub kind: PathKind,
     pub location: Location,
+    // The location of `kind` (this is the same as `location` for plain kinds)
+    pub kind_location: Location,
 }
 
 impl Path {
+    pub fn plain(segments: Vec<PathSegment>, location: Location) -> Self {
+        Self { segments, location, kind: PathKind::Plain, kind_location: location }
+    }
+
     pub fn pop(&mut self) -> PathSegment {
         self.segments.pop().unwrap()
     }
@@ -409,11 +410,8 @@ impl Path {
     }
 
     pub fn from_ident(name: Ident) -> Path {
-        Path {
-            location: name.location(),
-            segments: vec![PathSegment::from(name)],
-            kind: PathKind::Plain,
-        }
+        let location = name.location();
+        Path::plain(vec![PathSegment::from(name)], location)
     }
 
     pub fn span(&self) -> Span {
@@ -813,11 +811,7 @@ impl ForRange {
 
                 // array.len()
                 let segments = vec![PathSegment::from(array_ident)];
-                let array_ident = ExpressionKind::Variable(Path {
-                    segments,
-                    kind: PathKind::Plain,
-                    location: array_location,
-                });
+                let array_ident = ExpressionKind::Variable(Path::plain(segments, array_location));
 
                 let end_range = ExpressionKind::MethodCall(Box::new(MethodCallExpression {
                     object: Expression::new(array_ident.clone(), array_location),
@@ -834,11 +828,7 @@ impl ForRange {
 
                 // array[i]
                 let segments = vec![PathSegment::from(Ident::new(index_name, array_location))];
-                let index_ident = ExpressionKind::Variable(Path {
-                    segments,
-                    kind: PathKind::Plain,
-                    location: array_location,
-                });
+                let index_ident = ExpressionKind::Variable(Path::plain(segments, array_location));
 
                 let loop_element = ExpressionKind::Index(Box::new(IndexExpression {
                     collection: Expression::new(array_ident, array_location),