chore: Document type checking (#927)

* Document name resolution

* Document type checking

* Fix typos

* More docs

crates/noirc_frontend/src/hir/mod.rs

@@ -11,7 +11,9 @@ use def_map::CrateDefMap;
 use fm::FileManager;
 use std::collections::HashMap;
 
-/// Global context that is accessible during each stage
+/// Helper object which groups together several useful context objects used
+/// during name resolution. Once name resolution is finished, only the
+/// def_interner is required for type inference and monomorphization.
 #[derive(Default)]
 pub struct Context {
     pub def_interner: NodeInterner,

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

@@ -1,5 +1,5 @@
 //! This set of modules implements the second half of the name resolution pass.
-//! After all definitions are collected by def_collector, resovler::Resolvers are
+//! After all definitions are collected by def_collector, resolver::Resolvers are
 //! created to resolve all names within a definition. In this context 'resolving'
 //! a name means validating that it has a valid definition, and that it was not
 //! redefined multiple times in the same scope. Once this is validated, it is linked

crates/noirc_frontend/src/hir/type_check/expr.rs

@@ -12,6 +12,14 @@ use crate::{
 
 use super::{bind_pattern, errors::TypeCheckError};
 
+/// Infers a type for a given expression, and return this type.
+/// As a side-effect, this function will also remember this type in the NodeInterner
+/// for the given expr_id key.
+///
+/// This function also converts any HirExpression::MethodCalls `a.foo(b, c)` into
+/// an equivalent HirExpression::Call in the form `foo(a, b, c)`. This cannot
+/// be done earlier since we need to know the type of the object `a` to resolve which
+/// function `foo` to refer to.
 pub(crate) fn type_check_expression(
     interner: &mut NodeInterner,
     expr_id: &ExprId,

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

@@ -1,11 +1,19 @@
+//! This file contains type_check_func, the entry point to the type checking pass (for each function).
+//!
+//! The pass structure of type checking is relatively straightforward. It is a single pass through
+//! the HIR of each function and outputs the inferred type of each HIR node into the NodeInterner,
+//! keyed by the ID of the node.
+//!
+//! The algorithm for checking and inferring types itself is somewhat ad-hoc. It includes both
+//! unification and subtyping, with the only difference between the two being how CompTime
+//! is handled (See note on CompTime and make_subtype_of for details). Additionally, although
+//! this algorithm features inference via TypeVariables, there is no generalization step as
+//! all functions are required to give their full signatures. Closures are inferred but are
+//! never generalized and thus cannot be used polymorphically.
 mod errors;
 mod expr;
 mod stmt;
 
-// Type checking at the moment is very simple due to what is supported in the grammar.
-// If polymorphism is never need, then Wands algorithm should be powerful enough to accommodate
-// all foreseeable types, if it is needed then we would need to switch to Hindley-Milner type or maybe bidirectional
-
 pub use errors::TypeCheckError;
 use expr::type_check_expression;
 
@@ -16,23 +24,23 @@ pub(crate) use self::stmt::{bind_pattern, type_check};
 /// Type checks a function and assigns the
 /// appropriate types to expressions in a side table
 pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec<TypeCheckError> {
-    // First fetch the metadata and add the types for parameters
-    // Note that we do not look for the defining Identifier for a parameter,
-    // since we know that it is the parameter itself
     let meta = interner.function_meta(&func_id);
     let declared_return_type = meta.return_type().clone();
     let can_ignore_ret = meta.can_ignore_return_type();
 
+    // Bind each parameter to its annotated type.
+    // This is locally obvious, but it must be bound here so that the
+    // Definition object of the parameter in the NodeInterner is given the correct type.
     let mut errors = vec![];
     for param in meta.parameters.into_iter() {
         bind_pattern(interner, &param.0, param.1, &mut errors);
     }
 
     // Fetch the HirFunction and iterate all of it's statements
-    let hir_func = interner.function(&func_id);
-    let func_as_expr = hir_func.as_expr();
+    let function_body = interner.function(&func_id);
+    let function_body_id = function_body.as_expr();
 
-    let function_last_type = type_check_expression(interner, func_as_expr, &mut errors);
+    let function_last_type = type_check_expression(interner, function_body_id, &mut errors);
 
     // Go through any delayed type checking errors to see if they are resolved, or error otherwise.
     for type_check_fn in interner.take_delayed_type_check_functions() {
@@ -43,7 +51,7 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec<Type
 
     // Check declared return type and actual return type
     if !can_ignore_ret {
-        let func_span = interner.expr_span(func_as_expr); // XXX: We could be more specific and return the span of the last stmt, however stmts do not have spans yet
+        let func_span = interner.expr_span(function_body_id); // XXX: We could be more specific and return the span of the last stmt, however stmts do not have spans yet
         function_last_type.make_subtype_of(&declared_return_type, func_span, &mut errors, || {
             TypeCheckError::TypeMismatch {
                 expected_typ: declared_return_type.to_string(),

crates/noirc_frontend/src/hir/type_check/stmt.rs

@@ -9,6 +9,11 @@ use crate::CompTime;
 
 use super::{errors::TypeCheckError, expr::type_check_expression};
 
+/// Type checks a statement and all expressions/statements contained within.
+///
+/// All statements have a unit type `()` as their type so the type of the statement
+/// is not interesting. Type checking must still be done on statements to ensure any
+/// expressions used within them are typed correctly.
 pub(crate) fn type_check(
     interner: &mut NodeInterner,
     stmt_id: &StmtId,
@@ -53,6 +58,8 @@ pub(crate) fn type_check(
     Type::Unit
 }
 
+/// Associate a given HirPattern with the given Type, and remember
+/// this association in the NodeInterner.
 pub fn bind_pattern(
     interner: &mut NodeInterner,
     pattern: &HirPattern,
@@ -125,6 +132,7 @@ fn type_check_assign_stmt(
     });
 }
 
+/// Type check an lvalue - the left hand side of an assignment statement.
 fn type_check_lvalue(
     interner: &mut NodeInterner,
     lvalue: HirLValue,

crates/noirc_frontend/src/hir_def/types.rs

@@ -13,17 +13,56 @@ use crate::{node_interner::StructId, Ident, Signedness};
 
 #[derive(Debug, PartialEq, Eq, Clone, Hash)]
 pub enum Type {
+    /// A primitive Field type, and whether or not it is known at compile-time.
     FieldElement(CompTime),
-    Array(Box<Type>, Box<Type>),        // Array(4, Field) = [Field; 4]
-    Integer(CompTime, Signedness, u32), // u32 = Integer(unsigned, 32)
-    PolymorphicInteger(CompTime, TypeVariable),
+
+    /// Array(N, E) is an array of N elements of type E. It is expected that N
+    /// is either a type variable of some kind or a Type::Constant.
+    Array(Box<Type>, Box<Type>),
+
+    /// A primitive integer type with the given sign, bit count, and whether it is known at compile-time.
+    /// E.g. `u32` would be `Integer(CompTime::No(None), Unsigned, 32)`
+    Integer(CompTime, Signedness, u32),
+
+    /// The primitive `bool` type. Like other primitive types, whether bools are known at CompTime
+    /// is also tracked. Unlike the other primitives however, it isn't as useful since it is
+    /// primarily only used when converting between a bool and an integer type for indexing arrays.
     Bool(CompTime),
+
+    /// String(N) is an array of characters of length N. It is expected that N
+    /// is either a type variable of some kind or a Type::Constant.
     String(Box<Type>),
+
+    /// The unit type `()`.
     Unit,
+
+    /// A user-defined struct type. The `Shared<StructType>` field here refers to
+    /// the shared definition for each instance of this struct type. The `Vec<Type>`
+    /// represents the generic arguments (if any) to this struct type.
     Struct(Shared<StructType>, Vec<Type>),
+
+    /// A tuple type with the given list of fields in the order they appear in source code.
     Tuple(Vec<Type>),
+
+    /// TypeVariables are stand-in variables for some type which is not yet known.
+    /// They are not to be confused with NamedGenerics. While the later mostly works
+    /// as with normal types (ie. for two NamedGenerics T and U, T != U), TypeVariables
+    /// will be automatically rebound as necessary to satisfy any calls to unify
+    /// and make_subtype_of.
+    ///
+    /// TypeVariables are often created when a generic function is instantiated. This
+    /// is a process that replaces each NamedGeneric in a generic function with a TypeVariable.
+    /// Doing this at each call site of a generic function is how they can be called with
+    /// different argument types each time.
     TypeVariable(TypeVariable),
 
+    /// A generic integer or field type. This is a more specific kind of TypeVariable
+    /// that can only be bound to Type::Field, Type::Integer, or other PolymorphicIntegers.
+    /// This is the type of undecorated integer literals like `46`. Typing them in this way
+    /// allows them to be polymorphic over the actual integer/field type used without requiring
+    /// type annotations on each integer literal.
+    PolymorphicInteger(CompTime, TypeVariable),
+
     /// NamedGenerics are the 'T' or 'U' in a user-defined generic function
     /// like `fn foo<T, U>(...) {}`. Unlike TypeVariables, they cannot be bound over.
     NamedGeneric(TypeVariable, Rc<String>),
@@ -42,6 +81,10 @@ pub enum Type {
     /// bind to an integer without special checks to bind it to a non-type.
     Constant(u64),
 
+    /// The result of some type error. Remembering type errors as their own type variant lets
+    /// us avoid issuing repeat type errors for the same item. For example, a lambda with
+    /// an invalid type would otherwise issue a new error each time it is called
+    /// if not for this variant.
     Error,
 }