PR Feedback

crates/noirc_errors/src/lib.rs

@@ -12,14 +12,3 @@ pub struct FileDiagnostic {
     pub file_id: fm::FileId,
     pub diagnostic: CustomDiagnostic,
 }
-
-/// Extension trait just to enable the syntax err.in_file(..) for CustomDiagnostic
-pub trait IntoFileDiagnostic {
-    fn in_file(self, id: fm::FileId) -> FileDiagnostic;
-}
-
-impl IntoFileDiagnostic for CustomDiagnostic {
-    fn in_file(self, file_id: fm::FileId) -> FileDiagnostic {
-        FileDiagnostic { file_id, diagnostic: self }
-    }
-}

crates/noirc_errors/src/reporter.rs

@@ -56,6 +56,10 @@ 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);
     }

crates/noirc_frontend/src/ast/expression.rs

@@ -26,6 +26,8 @@ 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 {

crates/noirc_frontend/src/ast/structure.rs

@@ -4,6 +4,7 @@ use crate::{Ident, NoirFunction, UnresolvedGenerics, UnresolvedType};
 use iter_extended::vecmap;
 use noirc_errors::Span;
 
+/// Ast node for a struct
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct NoirStruct {
     pub name: Ident,
@@ -23,6 +24,7 @@ impl NoirStruct {
     }
 }
 
+/// Ast node for an impl
 #[derive(Clone, Debug)]
 pub struct NoirImpl {
     pub object_type: UnresolvedType,

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

@@ -18,8 +18,8 @@ use crate::{
 };
 use fm::FileId;
 use iter_extended::vecmap;
+use noirc_errors::Span;
 use noirc_errors::{CustomDiagnostic, FileDiagnostic};
-use noirc_errors::{IntoFileDiagnostic, Span};
 use std::collections::{BTreeMap, HashMap};
 use std::rc::Rc;
 

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

@@ -2,7 +2,6 @@ use crate::{hir::resolution::import::ImportDirective, Ident};
 
 use noirc_errors::CustomDiagnostic as Diagnostic;
 use noirc_errors::FileDiagnostic;
-use noirc_errors::IntoFileDiagnostic;
 use noirc_errors::Span;
 use thiserror::Error;
 

crates/noirc_frontend/src/hir/def_map/mod.rs

@@ -5,7 +5,7 @@ use crate::node_interner::FuncId;
 use crate::parser::{parse_program, ParsedModule};
 use arena::{Arena, Index};
 use fm::{FileId, FileManager};
-use noirc_errors::{FileDiagnostic, IntoFileDiagnostic};
+use noirc_errors::FileDiagnostic;
 use std::collections::HashMap;
 
 mod module_def;

crates/noirc_frontend/src/hir/resolution/errors.rs

@@ -1,5 +1,5 @@
 pub use noirc_errors::Span;
-use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic, IntoFileDiagnostic};
+use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic};
 use thiserror::Error;
 
 use crate::{Ident, Type};

crates/noirc_frontend/src/hir/resolution/resolver.rs

@@ -57,6 +57,8 @@ use crate::hir_def::{
 
 use super::errors::ResolverError;
 
+const SELF_TYPE_NAME: &str = "Self";
+
 type Scope = GenericScope<String, ResolverMeta>;
 type ScopeTree = GenericScopeTree<String, ResolverMeta>;
 type ScopeForest = GenericScopeForest<String, ResolverMeta>;
@@ -354,7 +356,7 @@ impl<'a> Resolver<'a> {
                 }
             }
 
-            if name == "Self" {
+            if name == SELF_TYPE_NAME {
                 if let Some(self_type) = self.self_type.clone() {
                     if !args.is_empty() {
                         self.push_err(ResolverError::GenericsOnSelfType { span: path.span() });
@@ -486,6 +488,8 @@ impl<'a> Resolver<'a> {
         }
     }
 
+    /// Add the given generics to scope.
+    /// Each generic will have a fresh Shared<TypeBinding> associated with it.
     pub fn add_generics(&mut self, generics: &UnresolvedGenerics) -> Generics {
         vecmap(generics, |generic| {
             // Map the generic to a fresh type variable
@@ -856,19 +860,21 @@ impl<'a> Resolver<'a> {
                 HirPattern::Tuple(fields, span)
             }
             Pattern::Struct(name, fields, span) => {
-                let error_ident = |this: &mut Self| {
-                    // Must create a name here to return a HirPattern::Identifer. $error is chosen
-                    // since it is an invalid identifier that will not crash with any user variables.
-                    let ident = this.add_variable_decl("$error".into(), false, true, definition);
-                    HirPattern::Identifier(ident)
+                let error_identifier = |this: &mut Self| {
+                    // Must create a name here to return a HirPattern::Identifer. Allowing
+                    // shadowing here lets us avoid further errors if we define ERROR_IDENT
+                    // multiple times.
+                    let name = ERROR_IDENT.into();
+                    let identifier = this.add_variable_decl(name, false, true, definition);
+                    HirPattern::Identifier(identifier)
                 };
 
                 let (struct_type, generics) = match self.lookup_type_or_error(name) {
                     Some(Type::Struct(struct_type, generics)) => (struct_type, generics),
-                    None => return error_ident(self),
+                    None => return error_identifier(self),
                     Some(typ) => {
                         self.push_err(ResolverError::NonStructUsedInConstructor { typ, span });
-                        return error_ident(self);
+                        return error_identifier(self);
                     }
                 };
 
@@ -1000,7 +1006,7 @@ impl<'a> Resolver<'a> {
     /// This will also instantiate any struct types found.
     fn lookup_type_or_error(&mut self, path: Path) -> Option<Type> {
         let ident = path.as_ident();
-        if ident.map_or(false, |i| i == "Self") {
+        if ident.map_or(false, |i| i == SELF_TYPE_NAME) {
             if let Some(typ) = &self.self_type {
                 return Some(typ.clone());
             }

crates/noirc_frontend/src/node_interner.rs

@@ -583,6 +583,9 @@ impl NodeInterner {
         })
     }
 
+    /// Add a method to a type.
+    /// This will panic for non-struct types currently as we do not support methods
+    /// for primitives. We could allow this in the future however.
     pub fn add_method(
         &mut self,
         self_type: &Type,
@@ -598,6 +601,7 @@ impl NodeInterner {
         }
     }
 
+    /// Search by name for a method on the given struct
     pub fn lookup_method(&self, id: StructId, method_name: &str) -> Option<FuncId> {
         self.struct_methods.get(&(id, method_name.to_owned())).map(|(_, id)| *id)
     }