feat: Impls with generics (#798)

* Remove unnecessary cloning

* No more Vecs of Vecs of errors

* Overhaul def collection to allow generics on impls

* Fix generics len assert

* Wrangle together all the type lookup functions and handle struct generics right

* Fix methods referring to incorrect type variables

* Rename finish report function

* Remove TODO

* PR Feedback

* Add Bar<Field> case to test

* PR Feedback

crates/nargo/tests/test_data/generics/src/main.nr

@@ -8,10 +8,45 @@ fn foo<T>(bar: Bar<T>) {
     constrain bar.one == bar.two;
 }
 
+struct BigInt<N> {
+    limbs: [u32; N],
+}
+
+impl<N> BigInt<N> {
+    // `N` is in scope of all methods in the impl
+    fn first(first: BigInt<N>, second: BigInt<N>) -> Self {
+        constrain first.limbs != second.limbs;
+        first
+    }
+
+    fn second(first: BigInt<N>, second: Self) -> Self {
+        constrain first.limbs != second.limbs;
+        second
+    }
+}
+
+impl Bar<Field> {
+    fn get_other(self) -> Field {
+        self.other
+    }
+}
+
 fn main(x: Field, y: Field) {
     let bar1: Bar<Field> = Bar { one: x, two: y, other: 0 };
     let bar2 = Bar { one: x, two: y, other: [0] };
 
     foo(bar1);
     foo(bar2);
+
+    // Test generic impls
+    let int1 = BigInt { limbs: [1] };
+    let int2 = BigInt { limbs: [2] };
+    let BigInt { limbs } = int1.second(int2).first(int1);
+    constrain limbs == int2.limbs;
+
+    // Test impl exclusively for Bar<Field>
+    constrain bar1.get_other() == bar1.other;
+
+    // Expected type error
+    // constrain bar2.get_other() == bar2.other;
 }

crates/noirc_driver/src/lib.rs

@@ -3,7 +3,7 @@ use acvm::acir::circuit::Circuit;
 use acvm::Language;
 use fm::FileType;
 use noirc_abi::Abi;
-use noirc_errors::{DiagnosableError, ReportedError, Reporter};
+use noirc_errors::{reporter, ReportedError};
 use noirc_evaluator::create_circuit;
 use noirc_frontend::graph::{CrateId, CrateName, CrateType, LOCAL_CRATE};
 use noirc_frontend::hir::def_map::CrateDefMap;
@@ -47,14 +47,7 @@ impl Driver {
     pub fn file_compiles(&mut self) -> bool {
         let mut errs = vec![];
         CrateDefMap::collect_defs(LOCAL_CRATE, &mut self.context, &mut errs);
-        for errors in &errs {
-            Reporter::with_diagnostics(
-                Some(errors.file_id),
-                &self.context.file_manager,
-                &errors.errors,
-                false,
-            );
-        }
+        reporter::report_all(&self.context.file_manager, &errs, false);
         errs.is_empty()
     }
 
@@ -135,17 +128,8 @@ impl Driver {
     pub fn check_crate(&mut self, allow_warnings: bool) -> Result<(), ReportedError> {
         let mut errs = vec![];
         CrateDefMap::collect_defs(LOCAL_CRATE, &mut self.context, &mut errs);
-        let mut error_count = 0;
-        for errors in &errs {
-            error_count += Reporter::with_diagnostics(
-                Some(errors.file_id),
-                &self.context.file_manager,
-                &errors.errors,
-                allow_warnings,
-            );
-        }
-
-        Reporter::finish(error_count)
+        let error_count = reporter::report_all(&self.context.file_manager, &errs, allow_warnings);
+        reporter::finish_report(error_count)
     }
 
     pub fn compute_abi(&self) -> Option<Abi> {
@@ -207,12 +191,10 @@ impl Driver {
             Err(err) => {
                 // The FileId here will be the file id of the file with the main file
                 // Errors will be shown at the call site without a stacktrace
-                let file_id = err.location.map(|loc| loc.file);
-                let error = &[err.to_diagnostic()];
+                let file = err.location.map(|loc| loc.file);
                 let files = &self.context.file_manager;
-
-                let error_count = Reporter::with_diagnostics(file_id, files, error, allow_warnings);
-                Reporter::finish(error_count)?;
+                let error = reporter::report(files, &err.into(), file, allow_warnings);
+                reporter::finish_report(error as u32)?;
                 Err(ReportedError)
             }
         }

crates/noirc_errors/src/lib.rs

@@ -1,19 +1,14 @@
 mod position;
-mod reporter;
-
+pub mod reporter;
 pub use position::{Location, Position, Span, Spanned};
-pub use reporter::*;
-
-pub trait DiagnosableError {
-    fn to_diagnostic(&self) -> CustomDiagnostic;
-}
+pub use reporter::{CustomDiagnostic, DiagnosticKind};
 
 /// Returned when the Reporter finishes after reporting errors
 #[derive(Copy, Clone)]
 pub struct ReportedError;
 
 #[derive(Debug, PartialEq, Eq)]
-pub struct CollectedErrors {
+pub struct FileDiagnostic {
     pub file_id: fm::FileId,
-    pub errors: Vec<CustomDiagnostic>,
+    pub diagnostic: CustomDiagnostic,
 }

crates/noirc_errors/src/reporter.rs

@@ -1,10 +1,9 @@
-use crate::{ReportedError, Span};
+use crate::{FileDiagnostic, ReportedError, Span};
 use codespan_reporting::diagnostic::{Diagnostic, Label};
 use codespan_reporting::term;
 use codespan_reporting::term::termcolor::{
     Color, ColorChoice, ColorSpec, StandardStream, WriteColor,
 };
-use fm::FileId;
 use std::io::Write;
 
 #[derive(Debug, PartialEq, Eq)]
@@ -57,13 +56,21 @@ impl CustomDiagnostic {
         }
     }
 
+    pub fn in_file(self, file_id: fm::FileId) -> FileDiagnostic {
+        FileDiagnostic { file_id, diagnostic: self }
+    }
+
     pub fn add_note(&mut self, message: String) {
         self.notes.push(message);
     }
 
     pub fn add_secondary(&mut self, message: String, span: Span) {
         self.secondaries.push(CustomLabel::new(message, span));
     }
+
+    fn is_error(&self) -> bool {
+        matches!(self.kind, DiagnosticKind::Error)
+    }
 }
 
 impl std::fmt::Display for CustomDiagnostic {
@@ -94,71 +101,72 @@ impl CustomLabel {
     }
 }
 
-pub struct Reporter;
-
-impl Reporter {
-    /// Writes the given diagnostics to stderr and returns the count
-    /// of diagnostics that were errors.
-    pub fn with_diagnostics(
-        file_id: Option<FileId>,
-        files: &fm::FileManager,
-        diagnostics: &[CustomDiagnostic],
-        allow_warnings: bool,
-    ) -> usize {
-        let mut error_count = 0;
-
-        // Convert each Custom Diagnostic into a diagnostic
-        let diagnostics = diagnostics.iter().map(|cd| {
-            let diagnostic = match (cd.kind, allow_warnings) {
-                (DiagnosticKind::Warning, true) => Diagnostic::warning(),
-                _ => {
-                    error_count += 1;
-                    Diagnostic::error()
-                }
-            };
-
-            let secondary_labels = if let Some(f_id) = file_id {
-                cd.secondaries
-                    .iter()
-                    .map(|sl| {
-                        let start_span = sl.span.start() as usize;
-                        let end_span = sl.span.end() as usize + 1;
-
-                        Label::secondary(f_id.as_usize(), start_span..end_span)
-                            .with_message(&sl.message)
-                    })
-                    .collect()
-            } else {
-                Vec::new()
-            };
-            diagnostic
-                .with_message(&cd.message)
-                .with_labels(secondary_labels)
-                .with_notes(cd.notes.clone())
-        });
+/// Writes the given diagnostics to stderr and returns the count
+/// of diagnostics that were errors.
+pub fn report_all(
+    files: &fm::FileManager,
+    diagnostics: &[FileDiagnostic],
+    allow_warnings: bool,
+) -> u32 {
+    diagnostics
+        .iter()
+        .map(|error| report(files, &error.diagnostic, Some(error.file_id), allow_warnings) as u32)
+        .sum()
+}
 
-        let writer = StandardStream::stderr(ColorChoice::Always);
-        let config = codespan_reporting::term::Config::default();
+/// Report the given diagnostic, and return true if it was an error
+pub fn report(
+    files: &fm::FileManager,
+    custom_diagnostic: &CustomDiagnostic,
+    file: Option<fm::FileId>,
+    allow_warnings: bool,
+) -> bool {
+    let writer = StandardStream::stderr(ColorChoice::Always);
+    let config = codespan_reporting::term::Config::default();
 
-        for diagnostic in diagnostics {
-            term::emit(&mut writer.lock(), &config, files.as_simple_files(), &diagnostic).unwrap();
-        }
+    let diagnostic = convert_diagnostic(custom_diagnostic, file, allow_warnings);
+    term::emit(&mut writer.lock(), &config, files.as_simple_files(), &diagnostic).unwrap();
 
-        error_count
-    }
+    !allow_warnings || custom_diagnostic.is_error()
+}
 
-    pub fn finish(error_count: usize) -> Result<(), ReportedError> {
-        if error_count != 0 {
-            let writer = StandardStream::stderr(ColorChoice::Always);
-            let mut writer = writer.lock();
+fn convert_diagnostic(
+    cd: &CustomDiagnostic,
+    file: Option<fm::FileId>,
+    allow_warnings: bool,
+) -> Diagnostic<usize> {
+    let diagnostic = match (cd.kind, allow_warnings) {
+        (DiagnosticKind::Warning, true) => Diagnostic::warning(),
+        _ => 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 + 1;
+                Label::secondary(file_id.as_usize(), start_span..end_span).with_message(&sl.message)
+            })
+            .collect()
+    } else {
+        vec![]
+    };
+
+    diagnostic.with_message(&cd.message).with_labels(secondary_labels).with_notes(cd.notes.clone())
+}
 
-            writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).unwrap();
-            writeln!(&mut writer, "error: aborting due to {error_count} previous errors").unwrap();
-            writer.reset().ok(); // Ignore any IO errors, we're exiting at this point anyway
+pub fn finish_report(error_count: u32) -> Result<(), ReportedError> {
+    if error_count != 0 {
+        let writer = StandardStream::stderr(ColorChoice::Always);
+        let mut writer = writer.lock();
 
-            Err(ReportedError)
-        } else {
-            Ok(())
-        }
+        writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).unwrap();
+        writeln!(&mut writer, "error: aborting due to {error_count} previous errors").unwrap();
+        writer.reset().ok();
+
+        Err(ReportedError)
+    } else {
+        Ok(())
     }
 }

crates/noirc_evaluator/src/errors.rs

@@ -1,4 +1,4 @@
-use noirc_errors::{CustomDiagnostic as Diagnostic, DiagnosableError, Location};
+use noirc_errors::{CustomDiagnostic as Diagnostic, Location};
 use thiserror::Error;
 
 #[derive(Debug)]
@@ -64,11 +64,11 @@ impl RuntimeErrorKind {
     }
 }
 
-impl DiagnosableError for RuntimeError {
-    fn to_diagnostic(&self) -> Diagnostic {
+impl From<RuntimeError> for Diagnostic {
+    fn from(error: RuntimeError) -> Diagnostic {
         let span =
-            if let Some(loc) = self.location { loc.span } else { noirc_errors::Span::new(0..0) };
-        match &self.kind {
+            if let Some(loc) = error.location { loc.span } else { noirc_errors::Span::new(0..0) };
+        match &error.kind {
             RuntimeErrorKind::ArrayOutOfBounds { index, bound } => Diagnostic::simple_error(
                 "index out of bounds".to_string(),
                 format!("out of bounds error, index is {index} but length is {bound}"),

crates/noirc_frontend/src/ast/expression.rs

@@ -26,6 +26,10 @@ pub enum ExpressionKind {
     Error,
 }
 
+/// A Vec of unresolved names for type variables.
+/// For `fn foo<A, B>(...)` this corresponds to vec!["A", "B"].
+pub type UnresolvedGenerics = Vec<Ident>;
+
 impl ExpressionKind {
     pub fn into_path(self) -> Option<Path> {
         match self {
@@ -306,7 +310,7 @@ pub struct Lambda {
 pub struct FunctionDefinition {
     pub name: Ident,
     pub attribute: Option<Attribute>, // XXX: Currently we only have one attribute defined. If more attributes are needed per function, we can make this a vector and make attribute definition more expressive
-    pub generics: Vec<Ident>,
+    pub generics: UnresolvedGenerics,
     pub parameters: Vec<(Pattern, UnresolvedType, noirc_abi::AbiVisibility)>,
     pub body: BlockExpression,
     pub span: Span,

crates/noirc_frontend/src/ast/mod.rs

@@ -18,7 +18,7 @@ use iter_extended::vecmap;
 /// The parser parses types as 'UnresolvedType's which
 /// require name resolution to resolve any type names used
 /// for structs within, but are otherwise identical to Types.
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, PartialEq, Eq, Clone, Hash)]
 pub enum UnresolvedType {
     FieldElement(CompTime),
     Array(Option<UnresolvedTypeExpression>, Box<UnresolvedType>), // [4]Witness = Array(4, Witness)
@@ -43,7 +43,7 @@ pub enum UnresolvedType {
 /// The precursor to TypeExpression, this is the type that the parser allows
 /// to be used in the length position of an array type. Only constants, variables,
 /// and numeric binary operators are allowed here.
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, PartialEq, Eq, Clone, Hash)]
 pub enum UnresolvedTypeExpression {
     Variable(Path),
     Constant(u64, Span),