feat(experimental): Issue errors for unreachable match branches

compiler/noirc_frontend/src/elaborator/expressions.rs

@@ -26,7 +26,8 @@ use crate::{
             HirArrayLiteral, HirBinaryOp, HirBlockExpression, HirCallExpression, HirCastExpression,
             HirConstrainExpression, HirConstructorExpression, HirExpression, HirIdent,
             HirIfExpression, HirIndexExpression, HirInfixExpression, HirLambda, HirLiteral,
-            HirMemberAccess, HirMethodCallExpression, HirPrefixExpression, ImplKind, TraitMethod,
+            HirMatch, HirMemberAccess, HirMethodCallExpression, HirPrefixExpression, ImplKind,
+            TraitMethod,
         },
         stmt::{HirLetStatement, HirPattern, HirStatement},
         traits::{ResolvedTraitBound, TraitConstraint},
@@ -1054,11 +1055,22 @@ impl Elaborator<'_> {
     ) -> (HirExpression, Type) {
         self.use_unstable_feature(super::UnstableFeature::Enums, location);
 
+        let expr_location = match_expr.expression.location;
         let (expression, typ) = self.elaborate_expression(match_expr.expression);
-        let (let_, variable) = self.wrap_in_let(expression, typ);
+        let (let_, variable) = self.wrap_in_let(expression, typ.clone());
+
+        let (errored, (rows, result_type)) =
+            self.errors_occurred_in(|this| this.elaborate_match_rules(variable, match_expr.rules));
+
+        // Avoid calling `elaborate_match_rows` if there were errors while constructing
+        // the match rows - it'll just lead to extra errors like `unreachable pattern`
+        // warnings on branches which previously had type errors.
+        let tree = HirExpression::Match(if !errored {
+            self.elaborate_match_rows(rows, &typ, expr_location)
+        } else {
+            HirMatch::Failure { missing_case: false }
+        });
 
-        let (rows, result_type) = self.elaborate_match_rules(variable, match_expr.rules);
-        let tree = HirExpression::Match(self.elaborate_match_rows(rows));
         let tree = self.interner.push_expr(tree);
         self.interner.push_expr_type(tree, result_type.clone());
         self.interner.push_expr_location(tree, location);

compiler/noirc_frontend/src/elaborator/lints.rs

@@ -338,7 +338,7 @@ fn can_return_without_recursing_match(
 
     match match_expr {
         HirMatch::Success(expr) => check(*expr),
-        HirMatch::Failure => true,
+        HirMatch::Failure { .. } => true,
         HirMatch::Guard { cond: _, body, otherwise } => check(*body) && check_match(otherwise),
         HirMatch::Switch(_, cases, otherwise) => {
             cases.iter().all(|case| check_match(&case.body))

compiler/noirc_frontend/src/elaborator/mod.rs

@@ -2163,4 +2163,13 @@ impl<'context> Elaborator<'context> {
             self.push_err(ParserError::with_reason(reason, location));
         }
     }
+
+    /// Run the given function using the resolver and return true if any errors (not warnings)
+    /// occurred while running it.
+    pub fn errors_occurred_in<T>(&mut self, f: impl FnOnce(&mut Self) -> T) -> (bool, T) {
+        let previous_errors = self.errors.len();
+        let ret = f(self);
+        let errored = self.errors.iter().skip(previous_errors).any(|error| error.is_error());
+        (errored, ret)
+    }
 }

compiler/noirc_frontend/src/elaborator/types.rs

@@ -766,8 +766,7 @@ impl Elaborator<'_> {
         make_error: impl FnOnce() -> TypeCheckError,
     ) {
         if let Err(UnificationError) = actual.unify(expected) {
-            let error: CompilationError = make_error().into();
-            self.push_err(error);
+            self.push_err(make_error());
         }
     }
 

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

@@ -233,7 +233,7 @@ impl HirMatch {
     fn to_display_ast(&self, interner: &NodeInterner, location: Location) -> ExpressionKind {
         match self {
             HirMatch::Success(expr) => expr.to_display_ast(interner).kind,
-            HirMatch::Failure => ExpressionKind::Error,
+            HirMatch::Failure { .. } => ExpressionKind::Error,
             HirMatch::Guard { cond, body, otherwise } => {
                 let condition = cond.to_display_ast(interner);
                 let consequence = body.to_display_ast(interner);

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

@@ -206,6 +206,13 @@ impl CompilationError {
             CompilationError::DebugComptimeScopeNotFound(_, location) => *location,
         }
     }
+
+    pub(crate) fn is_error(&self) -> bool {
+        // This is a bit expensive but not all error types have a `is_warning` method
+        // and it'd lead to code duplication to add them. `CompilationError::is_error`
+        // also isn't expected to be called too often.
+        CustomDiagnostic::from(self).is_error()
+    }
 }
 
 impl std::fmt::Display for CompilationError {

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

@@ -1,3 +1,4 @@
+use std::collections::BTreeSet;
 use std::rc::Rc;
 
 use acvm::FieldElement;
@@ -15,6 +16,9 @@ use crate::hir_def::types::{BinaryTypeOperator, Kind, Type};
 use crate::node_interner::NodeInterner;
 use crate::signed_field::SignedField;
 
+/// Rust also only shows 3 maximum, even for short patterns.
+pub const MAX_MISSING_CASES: usize = 3;
+
 #[derive(Error, Debug, Clone, PartialEq, Eq)]
 pub enum Source {
     #[error("Binary")]
@@ -235,6 +239,13 @@ pub enum TypeCheckError {
     UnnecessaryUnsafeBlock { location: Location },
     #[error("Unnecessary `unsafe` block")]
     NestedUnsafeBlock { location: Location },
+    #[error("Unreachable match case")]
+    UnreachableCase { location: Location },
+    #[error("Missing cases")]
+    MissingCases { cases: BTreeSet<String>, location: Location },
+    /// This error is used for types like integers which have too many variants to enumerate
+    #[error("Missing cases: `{typ}` is non-empty")]
+    MissingManyCases { typ: String, location: Location },
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -321,6 +332,9 @@ impl TypeCheckError {
             | TypeCheckError::CyclicType { location, .. }
             | TypeCheckError::TypeAnnotationsNeededForIndex { location }
             | TypeCheckError::UnnecessaryUnsafeBlock { location }
+            | TypeCheckError::UnreachableCase { location }
+            | TypeCheckError::MissingCases { location, .. }
+            | TypeCheckError::MissingManyCases { location, .. }
             | TypeCheckError::NestedUnsafeBlock { location } => *location,
             TypeCheckError::DuplicateNamedTypeArg { name: ident, .. }
             | TypeCheckError::NoSuchNamedTypeArg { name: ident, .. } => ident.location(),
@@ -641,6 +655,36 @@ impl<'a> From<&'a TypeCheckError> for Diagnostic {
                     *location,
                 )
             },
+            TypeCheckError::UnreachableCase { location } => {
+                Diagnostic::simple_warning(
+                    "Unreachable match case".into(), 
+                    "This pattern is redundant with one or more prior patterns".into(),
+                    *location,
+                )
+            },
+            TypeCheckError::MissingCases { cases, location } => {
+                let s = if cases.len() == 1 { "" } else { "s" };
+
+                let mut not_shown = String::new();
+                let mut shown_cases = cases.iter()
+                    .map(|s| format!("`{s}`"))
+                    .take(MAX_MISSING_CASES)
+                    .collect::<Vec<_>>();
+
+                if cases.len() > MAX_MISSING_CASES {
+                    shown_cases.truncate(MAX_MISSING_CASES);
+                    not_shown = format!(", and {} more not shown", cases.len() - MAX_MISSING_CASES);
+                }
+
+                let shown_cases = shown_cases.join(", ");
+                let msg = format!("Missing case{s}: {shown_cases}{not_shown}");
+                Diagnostic::simple_error(msg, String::new(), *location)
+            },
+            TypeCheckError::MissingManyCases { typ, location } => {
+                let msg = format!("Missing cases: `{typ}` is non-empty");
+                let secondary = "Try adding a match-all pattern: `_`".to_string();
+                Diagnostic::simple_error(msg, secondary, *location)
+            },
         }
     }
 }

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

@@ -2,4 +2,4 @@ mod errors;
 pub mod generics;
 
 pub use self::errors::Source;
-pub use errors::{NoMatchingImplFoundError, TypeCheckError};
+pub use errors::{MAX_MISSING_CASES, NoMatchingImplFoundError, TypeCheckError};

compiler/noirc_frontend/src/hir_def/expr.rs

@@ -1,4 +1,5 @@
 use fm::FileId;
+use iter_extended::vecmap;
 use noirc_errors::Location;
 
 use crate::Shared;
@@ -358,15 +359,13 @@ pub enum HirMatch {
     /// Jump directly to ExprId
     Success(ExprId),
 
-    Failure,
+    /// A Failure node in the match. `missing_case` is true if this node is the result of a missing
+    /// case of the match for which we should later reconstruct an example of.
+    Failure { missing_case: bool },
 
     /// Run `body` if the given expression is true.
     /// Otherwise continue with the given decision tree.
-    Guard {
-        cond: ExprId,
-        body: ExprId,
-        otherwise: Box<HirMatch>,
-    },
+    Guard { cond: ExprId, body: ExprId, otherwise: Box<HirMatch> },
 
     /// Switch on the given variable with the given cases to test.
     /// The final argument is an optional match-all case to take if
@@ -387,7 +386,7 @@ impl Case {
     }
 }
 
-#[derive(Debug, Clone, Hash, PartialEq, Eq)]
+#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
 pub enum Constructor {
     True,
     False,
@@ -437,6 +436,41 @@ impl Constructor {
             _ => false,
         }
     }
+
+    /// Return all the constructors of this type from one constructor. Intended to be used
+    /// for error reporting in cases where there are at least 2 constructors.
+    pub(crate) fn all_constructors(&self) -> Vec<(Constructor, /*arg count:*/ usize)> {
+        match self {
+            Constructor::True | Constructor::False => {
+                vec![(Constructor::True, 0), (Constructor::False, 0)]
+            }
+            Constructor::Unit => vec![(Constructor::Unit, 0)],
+            Constructor::Tuple(args) => vec![(self.clone(), args.len())],
+            Constructor::Variant(typ, _) => {
+                let typ = typ.follow_bindings();
+                let Type::DataType(def, generics) = &typ else {
+                    unreachable!(
+                        "Constructor::Variant should have a DataType type, but found {typ:?}"
+                    );
+                };
+
+                let def_ref = def.borrow();
+                if let Some(variants) = def_ref.get_variants(generics) {
+                    vecmap(variants.into_iter().enumerate(), |(i, (_, fields))| {
+                        (Constructor::Variant(typ.clone(), i), fields.len())
+                    })
+                } else
+                /* def is a struct */
+                {
+                    let field_count = def_ref.fields_raw().map(|fields| fields.len()).unwrap_or(0);
+                    vec![(Constructor::Variant(typ.clone(), 0), field_count)]
+                }
+            }
+
+            // Nothing great to return for these
+            Constructor::Int(_) | Constructor::Range(..) => Vec::new(),
+        }
+    }
 }
 
 impl std::fmt::Display for Constructor {

compiler/noirc_frontend/src/hir_def/types.rs

@@ -14,6 +14,7 @@
     ast::{IntegerBitSize, ItemVisibility},
     hir::type_check::{TypeCheckError, generics::TraitGenerics},
     node_interner::{ExprId, NodeInterner, TraitId, TypeAliasId},
+    signed_field::{AbsU128, SignedField},
 };
 use iter_extended::vecmap;
 use noirc_errors::Location;
@@ -1156,15 +1157,15 @@
     }
 
     pub fn is_field(&self) -> bool {
-        matches!(self.follow_bindings(), Type::FieldElement)
+        matches!(self.follow_bindings_shallow().as_ref(), Type::FieldElement)
     }
 
     pub fn is_bool(&self) -> bool {
-        matches!(self.follow_bindings(), Type::Bool)
+        matches!(self.follow_bindings_shallow().as_ref(), Type::Bool)
     }
 
     pub fn is_integer(&self) -> bool {
-        matches!(self.follow_bindings(), Type::Integer(_, _))
+        matches!(self.follow_bindings_shallow().as_ref(), Type::Integer(_, _))
     }
 
     /// If value_level, only check for Type::FieldElement,
@@ -2350,7 +2351,7 @@
             if replacement.type_variable_id() == Some(id) {
                 replacement.clone()
             } else {
                 replacement.substitute_helper(type_bindings, substitute_bound_typevars)
             }
         };
 
@@ -2436,7 +2437,7 @@
                 // is usually impossible and indicative of an error in the type checker somewhere.
                 for var in typevars {
                     assert!(!type_bindings.contains_key(&var.id()));
                 }
                 let typ = Box::new(typ.substitute_helper(type_bindings, substitute_bound_typevars));
                 Type::Forall(typevars.clone(), typ)
             }
@@ -2602,7 +2603,7 @@
     /// type. Unlike `follow_bindings`, this won't recursively follow any bindings on any
     /// fields or arguments of this type.
     pub fn follow_bindings_shallow(&self) -> Cow<Type> {
         match self {
             Type::TypeVariable(var) | Type::NamedGeneric(var, _) => {
                 if let TypeBinding::Bound(typ) = &*var.borrow() {
                     return Cow::Owned(typ.follow_bindings_shallow().into_owned());
@@ -2627,7 +2628,7 @@
     pub fn replace_named_generics_with_type_variables(&mut self) {
         match self {
             Type::FieldElement
             | Type::Constant(_, _)
             | Type::Integer(_, _)
             | Type::Bool
             | Type::Unit
@@ -2761,6 +2762,36 @@
             | Type::Error => None,
         }
     }
+
+    pub(crate) fn integral_minimum_size(&self) -> Option<SignedField> {
+        match self.follow_bindings_shallow().as_ref() {
+            Type::FieldElement => None,
+            Type::Integer(sign, num_bits) => {
+                if *sign == Signedness::Unsigned {
+                    return Some(SignedField::zero());
+                }
+
+                let max_bit_size = num_bits.bit_size() - 1;
+                Some(if max_bit_size == 128 {
+                    SignedField::negative(i128::MIN.abs_u128())
+                } else {
+                    SignedField::negative(1u128 << max_bit_size)
+                })
+            }
+            Type::Bool => Some(SignedField::zero()),
+            Type::TypeVariable(var) => {
+                let binding = &var.1;
+                match &*binding.borrow() {
+                    TypeBinding::Unbound(_, type_var_kind) => match type_var_kind {
+                        Kind::Any | Kind::Normal | Kind::Integer | Kind::IntegerOrField => None,
+                        Kind::Numeric(typ) => typ.integral_minimum_size(),
+                    },
+                    TypeBinding::Bound(typ) => typ.integral_minimum_size(),
+                }
+            }
+            _ => None,
+        }
+    }
 }
 
 /// Wraps a given `expression` in `expression.as_slice()`
@@ -3100,7 +3131,7 @@
             Type::DataType(def, args) => {
                 def.hash(state);
                 args.hash(state);
             }
             Type::Alias(alias, args) => {
                 alias.hash(state);
                 args.hash(state);

compiler/noirc_frontend/src/monomorphization/mod.rs

@@ -1995,7 +1995,7 @@ impl<'interner> Monomorphizer<'interner> {
     ) -> Result<ast::Expression, MonomorphizationError> {
         match match_expr {
             HirMatch::Success(id) => self.expr(id),
-            HirMatch::Failure => {
+            HirMatch::Failure { .. } => {
                 let false_ = Box::new(ast::Expression::Literal(ast::Literal::Bool(false)));
                 let msg = "match failure";
                 let msg_expr = ast::Expression::Literal(ast::Literal::Str(msg.to_string()));

compiler/noirc_frontend/src/signed_field.rs

@@ -19,6 +19,14 @@ impl SignedField {
         Self { field: field.into(), is_negative: true }
     }
 
+    pub fn zero() -> SignedField {
+        Self { field: FieldElement::zero(), is_negative: false }
+    }
+
+    pub fn one() -> SignedField {
+        Self { field: FieldElement::one(), is_negative: false }
+    }
+
     /// Convert a signed integer to a SignedField, carefully handling
     /// INT_MIN in the process. Note that to convert an unsigned integer
     /// you can call `SignedField::positive`.
@@ -125,6 +133,30 @@ impl std::fmt::Display for SignedField {
     }
 }
 
+impl rangemap::StepLite for SignedField {
+    fn add_one(&self) -> Self {
+        if self.is_negative {
+            if self.field.is_one() {
+                Self::new(FieldElement::zero(), false)
+            } else {
+                Self::new(self.field - FieldElement::one(), self.is_negative)
+            }
+        } else {
+            Self::new(self.field + FieldElement::one(), self.is_negative)
+        }
+    }
+
+    fn sub_one(&self) -> Self {
+        if self.is_negative {
+            Self::new(self.field + FieldElement::one(), self.is_negative)
+        } else if self.field.is_zero() {
+            Self::new(FieldElement::one(), true)
+        } else {
+            Self::new(self.field - FieldElement::one(), self.is_negative)
+        }
+    }
+}
+
 pub trait AbsU128 {
     /// Necessary to handle casting to unsigned generically without overflowing on INT_MIN.
     fn abs_u128(self) -> u128;