chore: update error conversion traits to act on references (#4936)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR pulls out some of the changes made to errors in #4918 

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

compiler/noirc_driver/src/lib.rs

@@ -252,7 +252,7 @@ pub fn check_crate(
     let mut errors = vec![];
     let diagnostics = CrateDefMap::collect_defs(crate_id, context, macros);
     errors.extend(diagnostics.into_iter().map(|(error, file_id)| {
-        let diagnostic: CustomDiagnostic = error.into();
+        let diagnostic = CustomDiagnostic::from(&error);
         diagnostic.in_file(file_id)
     }));
 

compiler/noirc_frontend/src/hir/comptime/errors.rs

@@ -7,7 +7,7 @@ use super::value::Value;
 /// The possible errors that can halt the interpreter.
 #[derive(Debug, Clone)]
 pub enum InterpreterError {
-    ArgumentCountMismatch { expected: usize, actual: usize, call_location: Location },
+    ArgumentCountMismatch { expected: usize, actual: usize, location: Location },
     TypeMismatch { expected: Type, value: Value, location: Location },
     NonComptimeVarReferenced { name: String, location: Location },
     IntegerOutOfRangeForType { value: FieldElement, typ: Type, location: Location },
@@ -60,7 +60,7 @@ impl InterpreterError {
 
     pub fn get_location(&self) -> Location {
         match self {
-            InterpreterError::ArgumentCountMismatch { call_location: location, .. }
+            InterpreterError::ArgumentCountMismatch { location, .. }
             | InterpreterError::TypeMismatch { location, .. }
             | InterpreterError::NonComptimeVarReferenced { location, .. }
             | InterpreterError::IntegerOutOfRangeForType { location, .. }
@@ -98,20 +98,20 @@ impl InterpreterError {
     }
 }
 
-impl From<InterpreterError> for CustomDiagnostic {
-    fn from(error: InterpreterError) -> Self {
+impl<'a> From<&'a InterpreterError> for CustomDiagnostic {
+    fn from(error: &'a InterpreterError) -> Self {
         match error {
-            InterpreterError::ArgumentCountMismatch { expected, actual, call_location } => {
+            InterpreterError::ArgumentCountMismatch { expected, actual, location } => {
                 let only = if expected > actual { "only " } else { "" };
-                let plural = if expected == 1 { "" } else { "s" };
-                let was_were = if actual == 1 { "was" } else { "were" };
+                let plural = if *expected == 1 { "" } else { "s" };
+                let was_were = if *actual == 1 { "was" } else { "were" };
                 let msg = format!(
                     "Expected {expected} argument{plural}, but {only}{actual} {was_were} provided"
                 );
 
                 let few_many = if actual < expected { "few" } else { "many" };
                 let secondary = format!("Too {few_many} arguments");
-                CustomDiagnostic::simple_error(msg, secondary, call_location.span)
+                CustomDiagnostic::simple_error(msg, secondary, location.span)
             }
             InterpreterError::TypeMismatch { expected, value, location } => {
                 let typ = value.get_type();

compiler/noirc_frontend/src/hir/comptime/interpreter.rs

@@ -70,20 +70,21 @@ impl<'a> Interpreter<'a> {
         &mut self,
         function: FuncId,
         arguments: Vec<(Value, Location)>,
-        call_location: Location,
+        location: Location,
     ) -> IResult<Value> {
         let previous_state = self.enter_function();
 
         let meta = self.interner.function_meta(&function);
         if meta.kind != FunctionKind::Normal {
-            todo!("Evaluation for {:?} is unimplemented", meta.kind);
+            let item = "Evaluation for builtin functions";
+            return Err(InterpreterError::Unimplemented { item, location });
         }
 
         if meta.parameters.len() != arguments.len() {
             return Err(InterpreterError::ArgumentCountMismatch {
                 expected: meta.parameters.len(),
                 actual: arguments.len(),
-                call_location,
+                location,
             });
         }
 
@@ -113,7 +114,7 @@ impl<'a> Interpreter<'a> {
             return Err(InterpreterError::ArgumentCountMismatch {
                 expected: closure.parameters.len(),
                 actual: arguments.len(),
-                call_location,
+                location: call_location,
             });
         }
 

compiler/noirc_frontend/src/hir/comptime/value.rs

@@ -116,7 +116,7 @@ impl Value {
             }
             Value::Closure(_lambda, _env, _typ) => {
                 // TODO: How should a closure's environment be inlined?
-                let item = "returning closures from a comptime fn";
+                let item = "Returning closures from a comptime fn";
                 return Err(InterpreterError::Unimplemented { item, location });
             }
             Value::Tuple(fields) => {

compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -158,8 +158,8 @@ pub enum CompilationError {
     InterpreterError(InterpreterError),
 }
 
-impl From<CompilationError> for CustomDiagnostic {
-    fn from(value: CompilationError) -> Self {
+impl<'a> From<&'a CompilationError> for CustomDiagnostic {
+    fn from(value: &'a CompilationError) -> Self {
         match value {
             CompilationError::ParseError(error) => error.into(),
             CompilationError::DefinitionError(error) => error.into(),

compiler/noirc_frontend/src/hir/def_collector/errors.rs

@@ -77,7 +77,7 @@ pub struct MacroError {
 }
 
 impl DefCollectorErrorKind {
-    pub fn into_file_diagnostic(self, file: fm::FileId) -> FileDiagnostic {
+    pub fn into_file_diagnostic(&self, file: fm::FileId) -> FileDiagnostic {
         Diagnostic::from(self).in_file(file)
     }
 }
@@ -99,8 +99,8 @@ impl fmt::Display for DuplicateType {
     }
 }
 
-impl From<DefCollectorErrorKind> for Diagnostic {
-    fn from(error: DefCollectorErrorKind) -> Diagnostic {
+impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic {
+    fn from(error: &'a DefCollectorErrorKind) -> Diagnostic {
         match error {
             DefCollectorErrorKind::Duplicate { typ, first_def, second_def } => {
                 let primary_message = format!(
@@ -133,18 +133,18 @@ impl From<DefCollectorErrorKind> for Diagnostic {
             DefCollectorErrorKind::NonStructTypeInImpl { span } => Diagnostic::simple_error(
                 "Non-struct type used in impl".into(),
                 "Only struct types may have implementation methods".into(),
-                span,
+                *span,
             ),
             DefCollectorErrorKind::MutableReferenceInTraitImpl { span } => Diagnostic::simple_error(
                 "Trait impls are not allowed on mutable reference types".into(),
                 "Try using a struct type here instead".into(),
-                span,
+                *span,
             ),
             DefCollectorErrorKind::OverlappingImpl { span, typ } => {
                 Diagnostic::simple_error(
                     format!("Impl for type `{typ}` overlaps with existing impl"),
                     "Overlapping impl".into(),
-                    span,
+                    *span,
                 )
             }
             DefCollectorErrorKind::OverlappingImplNote { span } => {
@@ -154,13 +154,13 @@ impl From<DefCollectorErrorKind> for Diagnostic {
                 Diagnostic::simple_error(
                     "Previous impl defined here".into(),
                     "Previous impl defined here".into(),
-                    span,
+                    *span,
                 )
             }
             DefCollectorErrorKind::ForeignImpl { span, type_name } => Diagnostic::simple_error(
                 "Cannot `impl` a type that was defined outside the current crate".into(),
                 format!("{type_name} was defined outside the current crate"),
-                span,
+                *span,
             ),
             DefCollectorErrorKind::TraitNotFound { trait_path } => Diagnostic::simple_error(
                 format!("Trait {trait_path} not found"),
@@ -174,15 +174,15 @@ impl From<DefCollectorErrorKind> for Diagnostic {
                 origin,
                 span,
             } => {
-                let plural = if expected_generic_count == 1 { "" } else { "s" };
+                let plural = if *expected_generic_count == 1 { "" } else { "s" };
                 let primary_message = format!(
                     "`{origin}` expects {expected_generic_count} generic{plural}, but {location} has {actual_generic_count}");
-                Diagnostic::simple_error(primary_message, "".to_string(), span)
+                Diagnostic::simple_error(primary_message, "".to_string(), *span)
             }
             DefCollectorErrorKind::MethodNotInTrait { trait_name, impl_method } => {
-                let trait_name = trait_name.0.contents;
+                let trait_name = &trait_name.0.contents;
                 let impl_method_span = impl_method.span();
-                let impl_method_name = impl_method.0.contents;
+                let impl_method_name = &impl_method.0.contents;
                 let primary_message = format!("Method with name `{impl_method_name}` is not part of trait `{trait_name}`, therefore it can't be implemented");
                 Diagnostic::simple_error(primary_message, "".to_owned(), impl_method_span)
             }
@@ -191,15 +191,15 @@ impl From<DefCollectorErrorKind> for Diagnostic {
                 method_name,
                 trait_impl_span,
             } => {
-                let trait_name = trait_name.0.contents;
-                let impl_method_name = method_name.0.contents;
+                let trait_name = &trait_name.0.contents;
+                let impl_method_name = &method_name.0.contents;
                 let primary_message = format!(
                     "Method `{impl_method_name}` from trait `{trait_name}` is not implemented"
                 );
                 Diagnostic::simple_error(
                     primary_message,
                     format!("Please implement {impl_method_name} here"),
-                    trait_impl_span,
+                    *trait_impl_span,
                 )
             }
             DefCollectorErrorKind::NotATrait { not_a_trait_name } => {
@@ -213,20 +213,20 @@ impl From<DefCollectorErrorKind> for Diagnostic {
             DefCollectorErrorKind::ModuleAlreadyPartOfCrate { mod_name, span } => {
                 let message = format!("Module '{mod_name}' is already part of the crate");
                 let secondary = String::new();
-                Diagnostic::simple_error(message, secondary, span)
+                Diagnostic::simple_error(message, secondary, *span)
             }
             DefCollectorErrorKind::ModuleOriginallyDefined { mod_name, span } => {
                 let message = format!("Note: {mod_name} was originally declared here");
                 let secondary = String::new();
-                Diagnostic::simple_error(message, secondary, span)
+                Diagnostic::simple_error(message, secondary, *span)
             }
             DefCollectorErrorKind::TraitImplOrphaned { span } => Diagnostic::simple_error(
                 "Orphaned trait implementation".into(),
                 "Either the type or the trait must be from the same crate as the trait implementation".into(),
-                span,
+                *span,
             ),
             DefCollectorErrorKind::MacroError(macro_error) => {
-                Diagnostic::simple_error(macro_error.primary_message, macro_error.secondary_message.unwrap_or_default(), macro_error.span.unwrap_or_default())
+                Diagnostic::simple_error(macro_error.primary_message.clone(), macro_error.secondary_message.clone().unwrap_or_default(), macro_error.span.unwrap_or_default())
             },
         }
     }