From e73ebfba3e62f8a4d394904b3fd42053596ba496 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 20 Apr 2022 14:20:23 -0400 Subject: [PATCH 01/17] Add a way to view solved types of arbitrary expressions/patterns in a program --- compiler/can/src/traverse.rs | 11 +++++++++++ compiler/constrain/src/expr.rs | 8 ++++++-- compiler/solve/tests/solve_expr.rs | 5 +++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/compiler/can/src/traverse.rs b/compiler/can/src/traverse.rs index a1f338c4ca..046b329dd4 100644 --- a/compiler/can/src/traverse.rs +++ b/compiler/can/src/traverse.rs @@ -52,7 +52,11 @@ fn walk_def(visitor: &mut V, def: &Def) { ); visitor.visit_expr(&loc_expr.value, loc_expr.region, *expr_var); if let Some(annot) = &annotation { +<<<<<<< HEAD visitor.visit_annotation(annot); +======= + visitor.visit_annotation(&annot); +>>>>>>> 1c2489622 (Add a way to view solved types of arbitrary expressions/patterns in a program) } } @@ -117,8 +121,15 @@ fn walk_when_branch(visitor: &mut V, branch: &WhenBranch, expr_var: } } +<<<<<<< HEAD fn walk_pattern(_visitor: &mut V, _pat: &Pattern) { todo!() +======= +fn walk_pattern(_visitor: &mut V, pat: &Pattern) { + match pat { + _ => todo!(), + } +>>>>>>> 1c2489622 (Add a way to view solved types of arbitrary expressions/patterns in a program) } trait Visitor: Sized { diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index 98ce0e367d..12c2fd89b1 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -705,14 +705,18 @@ pub fn constrain_expr( // TODO: when we have exhaustiveness checking during the typechecking phase, perform // exhaustiveness checking when this expectation fails. That will produce better error // messages. - let cond_constraint = constrain_expr( + let expr_con = constrain_expr( constraints, env, loc_cond.region, &loc_cond.value, Expected::ForReason(Reason::WhenBranches, cond_type, branches_region), ); - pattern_cons.push(cond_constraint); + // branch_cons.extend(pattern_cons); + // branch_constraints.push(constraints.and_constraint(pattern_cons)); + let mut total_cons = Vec::with_capacity(1 + 2 * branches.len() + 1); + // total_cons.push(expr_con); + pattern_cons.push(expr_con); // Solve all the pattern constraints together, introducing variables in the pattern as // need be before solving the bodies. diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index ecf4ea4058..0b1a03af36 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -255,6 +255,7 @@ mod solve_expr { let can_problems = can_problems.remove(&home).unwrap_or_default(); let type_problems = type_problems.remove(&home).unwrap_or_default(); +<<<<<<< HEAD let (can_problems, type_problems) = format_problems(&src, home, &interns, can_problems, type_problems); @@ -264,6 +265,10 @@ mod solve_expr { can_problems ); assert!(type_problems.is_empty(), "Type problems: {}", type_problems); +======= + assert_eq!(can_problems, Vec::new(), "Canonicalization problems: "); + assert_eq!(type_problems, Vec::new(), "Type problems: "); +>>>>>>> 1c2489622 (Add a way to view solved types of arbitrary expressions/patterns in a program) let queries = parse_queries(&src); assert!(!queries.is_empty(), "No queries provided!"); From 43bff0b59d7260a6cacda96b67914da37afb8149 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Thu, 21 Apr 2022 10:16:02 -0400 Subject: [PATCH 02/17] Turn repl test back on --- compiler/can/src/traverse.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/compiler/can/src/traverse.rs b/compiler/can/src/traverse.rs index 046b329dd4..a1f338c4ca 100644 --- a/compiler/can/src/traverse.rs +++ b/compiler/can/src/traverse.rs @@ -52,11 +52,7 @@ fn walk_def(visitor: &mut V, def: &Def) { ); visitor.visit_expr(&loc_expr.value, loc_expr.region, *expr_var); if let Some(annot) = &annotation { -<<<<<<< HEAD visitor.visit_annotation(annot); -======= - visitor.visit_annotation(&annot); ->>>>>>> 1c2489622 (Add a way to view solved types of arbitrary expressions/patterns in a program) } } @@ -121,15 +117,8 @@ fn walk_when_branch(visitor: &mut V, branch: &WhenBranch, expr_var: } } -<<<<<<< HEAD fn walk_pattern(_visitor: &mut V, _pat: &Pattern) { todo!() -======= -fn walk_pattern(_visitor: &mut V, pat: &Pattern) { - match pat { - _ => todo!(), - } ->>>>>>> 1c2489622 (Add a way to view solved types of arbitrary expressions/patterns in a program) } trait Visitor: Sized { From 9602b3634c74c612574e01e5008e4f84beeb7200 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Fri, 22 Apr 2022 22:44:38 -0400 Subject: [PATCH 03/17] Fix repl test --- repl_test/src/tests.rs | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/repl_test/src/tests.rs b/repl_test/src/tests.rs index 29c43a10a3..7e3f2aa4c2 100644 --- a/repl_test/src/tests.rs +++ b/repl_test/src/tests.rs @@ -903,38 +903,31 @@ fn parse_problem() { #[test] fn exhaustiveness_problem() { expect_failure( - r#" + indoc!( + r#" t : [A, B, C] t = A when t is A -> "a" - "#, + "# + ), indoc!( r#" - ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── + ── UNSAFE PATTERN ────────────────────────────────────────────────────────────── - The branches of this when expression don't match the condition: - - 7│> when t is - 8│ A -> "a" - - This t value is a: - - [ A, B, C ] + This when does not cover all the possibilities: - But the branch patterns have type: + 7│> when t is + 8│> A -> "a" - [ A ] - - The branches must be cases of the when condition's type! + Other possibilities include: - Tip: Looks like the branches are missing coverage of the C and B tags. + B + C - Tip: Maybe you need to add a catch-all branch, like _? - - - Enter an expression, or :help, or :exit/:q."# + I would have to crash if I saw one of those! Add branches for them! + "# ), ); } From 356616d83442154407c56c356bbcb812c21b9629 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 10:22:49 -0400 Subject: [PATCH 04/17] Move exhaustiveness checking to type solving phase with solve tests --- Cargo.lock | 2 + compiler/can/Cargo.toml | 1 + compiler/can/src/constraint.rs | 17 +- compiler/can/src/exhaustive.rs | 337 +++++++++++++++++++++++++++++ compiler/can/src/lib.rs | 1 + compiler/constrain/src/expr.rs | 6 + compiler/exhaustive/src/lib.rs | 5 +- compiler/mono/src/exhaustive.rs | 2 +- compiler/solve/Cargo.toml | 1 + compiler/solve/src/solve.rs | 11 + compiler/solve/tests/solve_expr.rs | 96 +++++--- compiler/types/src/subs.rs | 5 +- reporting/src/error/type.rs | 236 ++++++++++++++++++++ 13 files changed, 678 insertions(+), 42 deletions(-) create mode 100644 compiler/can/src/exhaustive.rs diff --git a/Cargo.lock b/Cargo.lock index d12ff57f39..fcbcb4e2da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3453,6 +3453,7 @@ dependencies = [ "roc_builtins", "roc_collections", "roc_error_macros", + "roc_exhaustive", "roc_module", "roc_parse", "roc_problem", @@ -3962,6 +3963,7 @@ dependencies = [ "roc_can", "roc_collections", "roc_error_macros", + "roc_exhaustive", "roc_load", "roc_module", "roc_parse", diff --git a/compiler/can/Cargo.toml b/compiler/can/Cargo.toml index b1d092053e..b7b1851465 100644 --- a/compiler/can/Cargo.toml +++ b/compiler/can/Cargo.toml @@ -8,6 +8,7 @@ edition = "2018" [dependencies] roc_collections = { path = "../collections" } roc_error_macros = { path = "../../error_macros" } +roc_exhaustive = { path = "../exhaustive" } roc_region = { path = "../region" } roc_module = { path = "../module" } roc_parse = { path = "../parse" } diff --git a/compiler/can/src/constraint.rs b/compiler/can/src/constraint.rs index c858834263..e28ff8f229 100644 --- a/compiler/can/src/constraint.rs +++ b/compiler/can/src/constraint.rs @@ -1,3 +1,4 @@ +use crate::exhaustive::{ExhaustiveContext, SketchedRows}; use crate::expected::{Expected, PExpected}; use roc_collections::soa::{EitherIndex, Index, Slice}; use roc_module::ident::TagName; @@ -19,6 +20,7 @@ pub struct Constraints { pub pattern_expectations: Vec>, pub includes_tags: Vec, pub strings: Vec<&'static str>, + pub sketched_rows: Vec, } impl Default for Constraints { @@ -40,6 +42,7 @@ impl Constraints { let pattern_expectations = Vec::new(); let includes_tags = Vec::new(); let strings = Vec::new(); + let sketched_rows = Vec::new(); types.extend([ Type::EmptyRec, @@ -90,6 +93,7 @@ impl Constraints { pattern_expectations, includes_tags, strings, + sketched_rows, } } @@ -570,6 +574,7 @@ impl Constraints { Constraint::IsOpenType(_) => false, Constraint::IncludesTag(_) => false, Constraint::PatternPresence(_, _, _, _) => false, + Constraint::Exhaustive(_, _) => false, } } @@ -597,11 +602,17 @@ impl Constraints { Constraint::Store(type_index, variable, string_index, line_number) } + + pub fn exhaustive(&mut self, rows: SketchedRows, context: ExhaustiveContext) -> Constraint { + let rows_index = Index::push_new(&mut self.sketched_rows, rows); + + Constraint::Exhaustive(rows_index, context) + } } roc_error_macros::assert_sizeof_default!(Constraint, 3 * 8); -#[derive(Clone)] +#[derive(Clone, Copy)] pub enum Constraint { Eq( EitherIndex, @@ -641,6 +652,7 @@ pub enum Constraint { Index, Region, ), + Exhaustive(Index, ExhaustiveContext), } #[derive(Debug, Clone, Copy, Default)] @@ -695,6 +707,9 @@ impl std::fmt::Debug for Constraint { arg0, arg1, arg2, arg3 ) } + Self::Exhaustive(arg0, arg1) => { + write!(f, "Exhaustive({:?}, {:?})", arg0, arg1) + } } } } diff --git a/compiler/can/src/exhaustive.rs b/compiler/can/src/exhaustive.rs new file mode 100644 index 0000000000..060c688d06 --- /dev/null +++ b/compiler/can/src/exhaustive.rs @@ -0,0 +1,337 @@ +use crate::expr::{IntValue, WhenBranch}; +use crate::pattern::DestructType; +use roc_collections::all::HumanIndex; +use roc_error_macros::internal_error; +use roc_exhaustive::{is_useful, Ctor, Error, Guard, Literal, Pattern, RenderAs, TagId, Union}; +use roc_module::ident::{TagIdIntType, TagName}; +use roc_region::all::{Loc, Region}; +use roc_types::subs::{Content, FlatType, Subs, SubsFmtContent, Variable}; + +pub use roc_exhaustive::Context as ExhaustiveContext; + +pub fn check( + subs: &Subs, + sketched_rows: SketchedRows, + context: ExhaustiveContext, +) -> Result<(), Vec> { + let overall_region = sketched_rows.overall_region; + // TODO: can we keep going even if we had redundant rows? + let matrix = sketched_rows + .reify_to_non_redundant(subs) + .map_err(|e| vec![e])?; + roc_exhaustive::check(overall_region, context, matrix) +} + +#[derive(Clone, Debug, PartialEq)] +enum SketchedPattern { + Anything, + Literal(Literal), + Ctor(Variable, TagName, std::vec::Vec), + KnownCtor(Union, TagId, std::vec::Vec), +} + +impl SketchedPattern { + fn reify(self, subs: &Subs) -> Pattern { + match self { + Self::Anything => Pattern::Anything, + Self::Literal(lit) => Pattern::Literal(lit), + Self::KnownCtor(union, tag_id, patterns) => Pattern::Ctor( + union, + tag_id, + patterns.into_iter().map(|pat| pat.reify(subs)).collect(), + ), + Self::Ctor(var, tag_name, patterns) => { + let (union, tag_id) = convert_tag(subs, var, &tag_name); + Pattern::Ctor( + union, + tag_id, + patterns.into_iter().map(|pat| pat.reify(subs)).collect(), + ) + } + } + } +} + +#[derive(Clone, Debug, PartialEq)] +struct SketchedRow { + patterns: Vec, + region: Region, + guard: Guard, +} + +#[derive(Clone, Debug, PartialEq)] +pub struct SketchedRows { + rows: Vec, + overall_region: Region, +} + +impl SketchedRows { + pub fn reify_to_non_redundant(self, subs: &Subs) -> Result>, Error> { + to_nonredundant_rows(subs, self) + } +} + +fn sketch_pattern(var: Variable, pattern: &crate::pattern::Pattern) -> SketchedPattern { + use crate::pattern::Pattern::*; + use SketchedPattern as SP; + + match pattern { + &NumLiteral(_, _, IntValue::I128(n), _) | &IntLiteral(_, _, _, IntValue::I128(n), _) => { + SP::Literal(Literal::Int(n)) + } + &NumLiteral(_, _, IntValue::U128(n), _) | &IntLiteral(_, _, _, IntValue::U128(n), _) => { + SP::Literal(Literal::U128(n)) + } + &FloatLiteral(_, _, _, f, _) => SP::Literal(Literal::Float(f64::to_bits(f))), + StrLiteral(v) => SP::Literal(Literal::Str(v.clone())), + &SingleQuote(c) => SP::Literal(Literal::Byte(c as u8)), + RecordDestructure { destructs, .. } => { + let tag_id = TagId(0); + let mut patterns = std::vec::Vec::with_capacity(destructs.len()); + let mut field_names = std::vec::Vec::with_capacity(destructs.len()); + + for Loc { + value: destruct, + region: _, + } in destructs + { + field_names.push(destruct.label.clone()); + + match &destruct.typ { + DestructType::Required | DestructType::Optional(..) => { + patterns.push(SP::Anything) + } + DestructType::Guard(_, guard) => { + patterns.push(sketch_pattern(destruct.var, &guard.value)) + } + } + } + + let union = Union { + render_as: RenderAs::Record(field_names), + alternatives: vec![Ctor { + name: TagName::Global("#Record".into()), + tag_id, + arity: destructs.len(), + }], + }; + + SP::KnownCtor(union, tag_id, patterns) + } + + AppliedTag { + tag_name, + arguments, + .. + } => { + let simplified_args: std::vec::Vec<_> = arguments + .iter() + .map(|(var, arg)| sketch_pattern(*var, &arg.value)) + .collect(); + + SP::Ctor(var, tag_name.clone(), simplified_args) + } + + UnwrappedOpaque { + opaque, argument, .. + } => { + let (arg_var, argument) = &(**argument); + + let tag_id = TagId(0); + + let union = Union { + render_as: RenderAs::Opaque, + alternatives: vec![Ctor { + name: TagName::Private(*opaque), + tag_id, + arity: 1, + }], + }; + + SP::KnownCtor( + union, + tag_id, + vec![sketch_pattern(*arg_var, &argument.value)], + ) + } + + Underscore + | Identifier(_) + | AbilityMemberSpecialization { .. } + | Shadowed(..) + | OpaqueNotInScope(..) + | UnsupportedPattern(..) + | MalformedPattern(..) => SP::Anything, + } +} + +pub fn sketch_rows(target_var: Variable, region: Region, patterns: &[WhenBranch]) -> SketchedRows { + let mut rows: Vec = Vec::with_capacity(patterns.len()); + + // If any of the branches has a guard, e.g. + // + // when x is + // y if y < 10 -> "foo" + // _ -> "bar" + // + // then we treat it as a pattern match on the pattern and a boolean, wrapped in the #Guard + // constructor. We can use this special constructor name to generate better error messages. + // This transformation of the pattern match only works because we only report exhaustiveness + // errors: the Pattern created in this file is not used for code gen. + // + // when x is + // #Guard y True -> "foo" + // #Guard _ _ -> "bar" + let any_has_guard = patterns.iter().any(|branch| branch.guard.is_some()); + + use SketchedPattern as SP; + for WhenBranch { + patterns, + guard, + value: _, + } in patterns + { + let guard = if guard.is_some() { + Guard::HasGuard + } else { + Guard::NoGuard + }; + + for loc_pat in patterns { + // Decompose each pattern in the branch into its own row. + + let patterns = if any_has_guard { + let guard_pattern = match guard { + Guard::HasGuard => SP::Literal(Literal::Bit(true)), + Guard::NoGuard => SP::Anything, + }; + + let tag_id = TagId(0); + + let union = Union { + render_as: RenderAs::Guard, + alternatives: vec![Ctor { + tag_id, + name: TagName::Global("#Guard".into()), + arity: 2, + }], + }; + + vec![SP::KnownCtor( + union, + tag_id, + // NB: ordering the guard pattern first seems to be better at catching + // non-exhaustive constructors in the second argument; see the paper to see if + // there is a way to improve this in general. + vec![guard_pattern, sketch_pattern(target_var, &loc_pat.value)], + )] + } else { + // Simple case + vec![sketch_pattern(target_var, &loc_pat.value)] + }; + + let row = SketchedRow { + patterns, + region, + guard, + }; + rows.push(row); + } + } + + SketchedRows { + rows, + overall_region: region, + } +} + +/// REDUNDANT PATTERNS + +/// INVARIANT: Produces a list of rows where (forall row. length row == 1) +fn to_nonredundant_rows(subs: &Subs, rows: SketchedRows) -> Result>, Error> { + let SketchedRows { + rows, + overall_region, + } = rows; + let mut checked_rows = Vec::with_capacity(rows.len()); + + for SketchedRow { + patterns, + guard, + region, + } in rows.into_iter() + { + let next_row: Vec = patterns + .into_iter() + .map(|pattern| pattern.reify(subs)) + .collect(); + + if matches!(guard, Guard::HasGuard) || is_useful(checked_rows.clone(), next_row.clone()) { + checked_rows.push(next_row); + } else { + return Err(Error::Redundant { + overall_region, + branch_region: region, + index: HumanIndex::zero_based(checked_rows.len()), + }); + } + } + + Ok(checked_rows) +} + +fn convert_tag(subs: &Subs, whole_var: Variable, this_tag: &TagName) -> (Union, TagId) { + let content = subs.get_content_without_compacting(whole_var); + + use {Content::*, FlatType::*}; + + match content { + Structure(TagUnion(tags, ext) | RecursiveTagUnion(_, tags, ext)) => { + let (sorted_tags, ext) = tags.sorted_iterator_and_ext(subs, *ext); + + let mut num_tags = sorted_tags.len(); + + // DEVIATION: model openness by attaching a #Open constructor, that can never + // be matched unless there's an `Anything` pattern. + let opt_openness_tag = match subs.get_content_without_compacting(ext) { + FlexVar(_) | RigidVar(_) => { + let openness_tag = TagName::Global("#Open".into()); + num_tags += 1; + Some((openness_tag, &[] as _)) + } + Structure(EmptyTagUnion) => None, + // Anything else is erroneous and we ignore + _ => None, + }; + + // High tag ID if we're out-of-bounds. + let mut my_tag_id = TagId(num_tags as TagIdIntType); + + let mut alternatives = Vec::with_capacity(num_tags); + let alternatives_iter = sorted_tags.into_iter().chain(opt_openness_tag.into_iter()); + + for (index, (tag, args)) in alternatives_iter.enumerate() { + let tag_id = TagId(index as TagIdIntType); + if this_tag == &tag { + my_tag_id = tag_id; + } + alternatives.push(Ctor { + name: tag, + tag_id, + arity: args.len(), + }); + } + + let union = Union { + alternatives, + render_as: RenderAs::Tag, + }; + + (union, my_tag_id) + } + _ => internal_error!( + "Content is not a tag union: {:?}", + SubsFmtContent(content, subs) + ), + } +} diff --git a/compiler/can/src/lib.rs b/compiler/can/src/lib.rs index d7858bcde1..b2dc3f4aa1 100644 --- a/compiler/can/src/lib.rs +++ b/compiler/can/src/lib.rs @@ -8,6 +8,7 @@ pub mod constraint; pub mod def; pub mod effect_module; pub mod env; +pub mod exhaustive; pub mod expected; pub mod expr; pub mod module; diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index 12c2fd89b1..178c151a4c 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -5,6 +5,7 @@ use crate::pattern::{constrain_pattern, PatternState}; use roc_can::annotation::IntroducedVariables; use roc_can::constraint::{Constraint, Constraints}; use roc_can::def::{Declaration, Def}; +use roc_can::exhaustive::{sketch_rows, ExhaustiveContext}; use roc_can::expected::Expected::{self, *}; use roc_can::expected::PExpected; use roc_can::expr::Expr::{self, *}; @@ -718,6 +719,11 @@ pub fn constrain_expr( // total_cons.push(expr_con); pattern_cons.push(expr_con); + let sketched_rows = sketch_rows(cond_var, branches_region, &branches); + let exhaustiveness_constraint = + constraints.exhaustive(sketched_rows, ExhaustiveContext::BadCase); + pattern_cons.push(exhaustiveness_constraint); + // Solve all the pattern constraints together, introducing variables in the pattern as // need be before solving the bodies. let pattern_constraints = constraints.and_constraint(pattern_cons); diff --git a/compiler/exhaustive/src/lib.rs b/compiler/exhaustive/src/lib.rs index c94b98aaa0..19638404a4 100644 --- a/compiler/exhaustive/src/lib.rs +++ b/compiler/exhaustive/src/lib.rs @@ -78,6 +78,7 @@ pub enum Literal { U128(u128), Bit(bool), Byte(u8), + /// Stores the float bits Float(u64), Decimal(RocDec), Str(Box), @@ -95,14 +96,14 @@ pub enum Error { }, } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum Context { BadArg, BadDestruct, BadCase, } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum Guard { HasGuard, NoGuard, diff --git a/compiler/mono/src/exhaustive.rs b/compiler/mono/src/exhaustive.rs index 514cdf3f2d..461daba358 100644 --- a/compiler/mono/src/exhaustive.rs +++ b/compiler/mono/src/exhaustive.rs @@ -113,7 +113,7 @@ pub fn check( } } -pub fn check_patterns<'a>( +fn check_patterns<'a>( region: Region, context: Context, patterns: &[(Loc>, Guard)], diff --git a/compiler/solve/Cargo.toml b/compiler/solve/Cargo.toml index 1288c18273..6b32bc14a8 100644 --- a/compiler/solve/Cargo.toml +++ b/compiler/solve/Cargo.toml @@ -8,6 +8,7 @@ edition = "2018" [dependencies] roc_collections = { path = "../collections" } roc_error_macros = { path = "../../error_macros" } +roc_exhaustive = { path = "../exhaustive" } roc_region = { path = "../region" } roc_module = { path = "../module" } roc_types = { path = "../types" } diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index f977920b7a..e25c82dded 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -100,6 +100,7 @@ pub enum TypeError { ErrorType, Vec, ), + Exhaustive(roc_exhaustive::Error), } use roc_types::types::Alias; @@ -1196,6 +1197,16 @@ fn solve( } } } + Exhaustive(rows_index, context) => { + let sketched_rows = constraints.sketched_rows[rows_index.index()].clone(); + + let checked = roc_can::exhaustive::check(&subs, sketched_rows, *context); + if let Err(errors) = checked { + problems.extend(errors.into_iter().map(TypeError::Exhaustive)); + } + + state + } }; } diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index 0b1a03af36..6a5d90d3c7 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -255,7 +255,6 @@ mod solve_expr { let can_problems = can_problems.remove(&home).unwrap_or_default(); let type_problems = type_problems.remove(&home).unwrap_or_default(); -<<<<<<< HEAD let (can_problems, type_problems) = format_problems(&src, home, &interns, can_problems, type_problems); @@ -265,10 +264,6 @@ mod solve_expr { can_problems ); assert!(type_problems.is_empty(), "Type problems: {}", type_problems); -======= - assert_eq!(can_problems, Vec::new(), "Canonicalization problems: "); - assert_eq!(type_problems, Vec::new(), "Type problems: "); ->>>>>>> 1c2489622 (Add a way to view solved types of arbitrary expressions/patterns in a program) let queries = parse_queries(&src); assert!(!queries.is_empty(), "No queries provided!"); @@ -2565,11 +2560,10 @@ mod solve_expr { } // this test is related to a bug where ext_var would have an incorrect rank. - // This match has duplicate cases, but that's not important because exhaustiveness happens - // after inference. + // This match has duplicate cases, but we ignore that. #[test] fn to_bit_record() { - infer_eq_without_problem( + infer_eq( indoc!( r#" foo = \rec -> @@ -5266,95 +5260,123 @@ mod solve_expr { { u8: (\n -> when n is - 123u8 -> n), + 123u8 -> n + _ -> n), u16: (\n -> when n is - 123u16 -> n), + 123u16 -> n + _ -> n), u32: (\n -> when n is - 123u32 -> n), + 123u32 -> n + _ -> n), u64: (\n -> when n is - 123u64 -> n), + 123u64 -> n + _ -> n), u128: (\n -> when n is - 123u128 -> n), + 123u128 -> n + _ -> n), i8: (\n -> when n is - 123i8 -> n), + 123i8 -> n + _ -> n), i16: (\n -> when n is - 123i16 -> n), + 123i16 -> n + _ -> n), i32: (\n -> when n is - 123i32 -> n), + 123i32 -> n + _ -> n), i64: (\n -> when n is - 123i64 -> n), + 123i64 -> n + _ -> n), i128: (\n -> when n is - 123i128 -> n), + 123i128 -> n + _ -> n), nat: (\n -> when n is - 123nat -> n), + 123nat -> n + _ -> n), bu8: (\n -> when n is - 0b11u8 -> n), + 0b11u8 -> n + _ -> n), bu16: (\n -> when n is - 0b11u16 -> n), + 0b11u16 -> n + _ -> n), bu32: (\n -> when n is - 0b11u32 -> n), + 0b11u32 -> n + _ -> n), bu64: (\n -> when n is - 0b11u64 -> n), + 0b11u64 -> n + _ -> n), bu128: (\n -> when n is - 0b11u128 -> n), + 0b11u128 -> n + _ -> n), bi8: (\n -> when n is - 0b11i8 -> n), + 0b11i8 -> n + _ -> n), bi16: (\n -> when n is - 0b11i16 -> n), + 0b11i16 -> n + _ -> n), bi32: (\n -> when n is - 0b11i32 -> n), + 0b11i32 -> n + _ -> n), bi64: (\n -> when n is - 0b11i64 -> n), + 0b11i64 -> n + _ -> n), bi128: (\n -> when n is - 0b11i128 -> n), + 0b11i128 -> n + _ -> n), bnat: (\n -> when n is - 0b11nat -> n), + 0b11nat -> n + _ -> n), dec: (\n -> when n is - 123.0dec -> n), + 123.0dec -> n + _ -> n), f32: (\n -> when n is - 123.0f32 -> n), + 123.0f32 -> n + _ -> n), f64: (\n -> when n is - 123.0f64 -> n), + 123.0f64 -> n + _ -> n), fdec: (\n -> when n is - 123dec -> n), + 123dec -> n + _ -> n), ff32: (\n -> when n is - 123f32 -> n), + 123f32 -> n + _ -> n), ff64: (\n -> when n is - 123f64 -> n), + 123f64 -> n + _ -> n), } "# ), @@ -5686,6 +5708,7 @@ mod solve_expr { @Id (Id _ A) -> "" @Id (Id _ B) -> "" @Id (Id _ (C { a: "" })) -> "" + @Id (Id _ (C { a: _ })) -> "" # any other string, for exhautiveness "# ), r#"Id [ A, B, C { a : Str }* ] -> Str"#, @@ -5705,6 +5728,7 @@ mod solve_expr { @Id (Id _ A) -> "" @Id (Id _ B) -> "" @Id (Id _ (C { a: "" })) -> "" + @Id (Id _ (C { a: _ })) -> "" # any other string, for exhautiveness f "# diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index 8905a9aa58..1d9a181c67 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -2333,7 +2333,8 @@ impl UnionTags { pub fn iter_all( &self, - ) -> impl Iterator, SubsIndex)> { + ) -> impl Iterator, SubsIndex)> + ExactSizeIterator + { self.tag_names() .into_iter() .zip(self.variables().into_iter()) @@ -2436,7 +2437,7 @@ impl<'a> UnsortedUnionTags<'a> { } } -pub type SortedTagsIterator<'a> = Box + 'a>; +pub type SortedTagsIterator<'a> = Box + 'a>; pub type SortedTagsSlicesIterator<'a> = Box + 'a>; pub fn is_empty_tag_union(subs: &Subs, mut var: Variable) -> bool { diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index 90e7ad15b9..97a0d27a24 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -198,6 +198,7 @@ pub fn type_problem<'b>( }; Some(report) } + Exhaustive(problem) => Some(exhaustive_problem(alloc, lines, filename, problem)), } } @@ -3484,3 +3485,238 @@ fn report_record_field_typo<'b>( severity: Severity::RuntimeError, } } + +fn exhaustive_problem<'a>( + alloc: &'a RocDocAllocator<'a>, + lines: &LineInfo, + filename: PathBuf, + problem: roc_exhaustive::Error, +) -> Report<'a> { + use roc_exhaustive::Context::*; + use roc_exhaustive::Error::*; + + match problem { + Incomplete(region, context, missing) => match context { + BadArg => { + let doc = alloc.stack([ + alloc.reflow("This pattern does not cover all the possibilities:"), + alloc.region(lines.convert_region(region)), + alloc.reflow("Other possibilities include:"), + unhandled_patterns_to_doc_block(alloc, missing), + alloc.concat([ + alloc.reflow( + "I would have to crash if I saw one of those! \ + So rather than pattern matching in function arguments, put a ", + ), + alloc.keyword("when"), + alloc.reflow(" in the function body to account for all possibilities."), + ]), + ]); + + Report { + filename, + title: "UNSAFE PATTERN".to_string(), + doc, + severity: Severity::RuntimeError, + } + } + BadDestruct => { + let doc = alloc.stack([ + alloc.reflow("This pattern does not cover all the possibilities:"), + alloc.region(lines.convert_region(region)), + alloc.reflow("Other possibilities include:"), + unhandled_patterns_to_doc_block(alloc, missing), + alloc.concat([ + alloc.reflow( + "I would have to crash if I saw one of those! \ + You can use a binding to deconstruct a value if there is only ONE possibility. \ + Use a " + ), + alloc.keyword("when"), + alloc.reflow(" to account for all possibilities."), + ]), + ]); + + Report { + filename, + title: "UNSAFE PATTERN".to_string(), + doc, + severity: Severity::RuntimeError, + } + } + BadCase => { + let doc = alloc.stack([ + alloc.concat([ + alloc.reflow("This "), + alloc.keyword("when"), + alloc.reflow(" does not cover all the possibilities:"), + ]), + alloc.region(lines.convert_region(region)), + alloc.reflow("Other possibilities include:"), + unhandled_patterns_to_doc_block(alloc, missing), + alloc.reflow( + "I would have to crash if I saw one of those! \ + Add branches for them!", + ), + // alloc.hint().append(alloc.reflow("or use a hole.")), + ]); + + Report { + filename, + title: "UNSAFE PATTERN".to_string(), + doc, + severity: Severity::RuntimeError, + } + } + }, + Redundant { + overall_region, + branch_region, + index, + } => { + let doc = alloc.stack([ + alloc.concat([ + alloc.reflow("The "), + alloc.string(index.ordinal()), + alloc.reflow(" pattern is redundant:"), + ]), + alloc.region_with_subregion( + lines.convert_region(overall_region), + lines.convert_region(branch_region), + ), + alloc.reflow( + "Any value of this shape will be handled by \ + a previous pattern, so this one should be removed.", + ), + ]); + + Report { + filename, + title: "REDUNDANT PATTERN".to_string(), + doc, + severity: Severity::Warning, + } + } + } +} + +pub fn unhandled_patterns_to_doc_block<'b>( + alloc: &'b RocDocAllocator<'b>, + patterns: Vec, +) -> RocDocBuilder<'b> { + alloc + .vcat( + patterns + .into_iter() + .map(|v| exhaustive_pattern_to_doc(alloc, v)), + ) + .indent(4) + .annotate(Annotation::TypeBlock) +} + +fn exhaustive_pattern_to_doc<'b>( + alloc: &'b RocDocAllocator<'b>, + pattern: roc_exhaustive::Pattern, +) -> RocDocBuilder<'b> { + pattern_to_doc_help(alloc, pattern, false) +} + +const AFTER_TAG_INDENT: &str = " "; + +fn pattern_to_doc_help<'b>( + alloc: &'b RocDocAllocator<'b>, + pattern: roc_exhaustive::Pattern, + in_type_param: bool, +) -> RocDocBuilder<'b> { + use roc_exhaustive::Literal::*; + use roc_exhaustive::Pattern::*; + use roc_exhaustive::RenderAs; + + match pattern { + Anything => alloc.text("_"), + Literal(l) => match l { + Int(i) => alloc.text(i.to_string()), + U128(i) => alloc.text(i.to_string()), + Bit(true) => alloc.text("True"), + Bit(false) => alloc.text("False"), + Byte(b) => alloc.text(b.to_string()), + Float(f) => alloc.text(f.to_string()), + Decimal(d) => alloc.text(d.to_string()), + Str(s) => alloc.string(s.into()), + }, + Ctor(union, tag_id, args) => { + match union.render_as { + RenderAs::Guard => { + // #Guard + debug_assert_eq!( + union.alternatives[tag_id.0 as usize].name, + TagName::Global("#Guard".into()) + ); + debug_assert!(args.len() == 2); + let tag = pattern_to_doc_help(alloc, args[1].clone(), in_type_param); + alloc.concat([ + tag, + alloc.text(AFTER_TAG_INDENT), + alloc.text("(note the lack of an "), + alloc.keyword("if"), + alloc.text(" clause)"), + ]) + } + RenderAs::Record(field_names) => { + let mut arg_docs = Vec::with_capacity(args.len()); + + for (label, v) in field_names.into_iter().zip(args.into_iter()) { + match &v { + Anything => { + arg_docs.push(alloc.text(label.to_string())); + } + Literal(_) | Ctor(_, _, _) => { + arg_docs.push( + alloc + .text(label.to_string()) + .append(alloc.reflow(": ")) + .append(pattern_to_doc_help(alloc, v, false)), + ); + } + } + } + + alloc + .text("{ ") + .append(alloc.intersperse(arg_docs, alloc.reflow(", "))) + .append(" }") + } + RenderAs::Tag | RenderAs::Opaque => { + let has_args = !args.is_empty(); + let arg_docs = args + .into_iter() + .map(|v| pattern_to_doc_help(alloc, v, true)); + + let tag = &union.alternatives[tag_id.0 as usize]; + let tag_name = match union.render_as { + RenderAs::Tag => alloc.tag_name(tag.name.clone()), + RenderAs::Opaque => match tag.name { + TagName::Private(opaque) => alloc.wrapped_opaque_name(opaque), + _ => unreachable!(), + }, + _ => unreachable!(), + }; + + // We assume the alternatives are sorted. If not, this assert will trigger + debug_assert!(tag_id == tag.tag_id); + + let docs = std::iter::once(tag_name).chain(arg_docs); + + if in_type_param && has_args { + alloc + .text("(") + .append(alloc.intersperse(docs, alloc.space())) + .append(")") + } else { + alloc.intersperse(docs, alloc.space()) + } + } + } + } + } +} From f7e04490c09f1610352f3e17cd0bd73e25c95eae Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 10:43:08 -0400 Subject: [PATCH 05/17] Remove stale comment --- compiler/constrain/src/expr.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index 178c151a4c..2d39f196a6 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -742,8 +742,6 @@ pub fn constrain_expr( let total_cons = [when_body_con, result_con]; let branch_constraints = constraints.and_constraint(total_cons); - // exhautiveness checking happens when converting to mono::Expr - // ...for now constraints.exists([cond_var, *expr_var], branch_constraints) } Access { From 85e3373d8b34593eb06da19ebead5eb1f59e99de Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 15:23:56 -0400 Subject: [PATCH 06/17] Move exhaustiveness checking to type checking --- compiler/can/src/builtins.rs | 6 ++ compiler/can/src/constraint.rs | 49 ++++++++-- compiler/can/src/effect_module.rs | 1 + compiler/can/src/exhaustive.rs | 32 +++++-- compiler/can/src/expr.rs | 47 +++++++++- compiler/can/src/traverse.rs | 1 + compiler/constrain/src/expr.rs | 49 ++++++---- compiler/exhaustive/src/lib.rs | 6 +- compiler/mono/src/ir.rs | 3 + compiler/solve/src/solve.rs | 143 ++++++++++++++++++++++++++++- compiler/solve/tests/solve_expr.rs | 2 +- compiler/types/src/subs.rs | 12 +++ compiler/types/src/types.rs | 3 + reporting/src/error/type.rs | 6 +- reporting/tests/test_reporting.rs | 80 ++++++---------- 15 files changed, 346 insertions(+), 94 deletions(-) diff --git a/compiler/can/src/builtins.rs b/compiler/can/src/builtins.rs index 446a32a194..d7de1a8748 100644 --- a/compiler/can/src/builtins.rs +++ b/compiler/can/src/builtins.rs @@ -4727,6 +4727,7 @@ fn result_map(symbol: Symbol, var_store: &mut VarStore) -> Def { region: Region::zero(), loc_cond: Box::new(no_region(Var(Symbol::ARG_1))), branches, + branches_cond_var: var_store.fresh(), }; defn( @@ -4824,6 +4825,7 @@ fn result_map_err(symbol: Symbol, var_store: &mut VarStore) -> Def { region: Region::zero(), loc_cond: Box::new(no_region(Var(Symbol::ARG_1))), branches, + branches_cond_var: var_store.fresh(), }; defn( @@ -4887,6 +4889,7 @@ fn result_with_default(symbol: Symbol, var_store: &mut VarStore) -> Def { region: Region::zero(), loc_cond: Box::new(no_region(Var(Symbol::ARG_1))), branches, + branches_cond_var: var_store.fresh(), }; defn( @@ -4964,6 +4967,7 @@ fn result_is_err(symbol: Symbol, var_store: &mut VarStore) -> Def { region: Region::zero(), loc_cond: Box::new(no_region(Var(Symbol::ARG_1))), branches, + branches_cond_var: var_store.fresh(), }; defn( @@ -5041,6 +5045,7 @@ fn result_is_ok(symbol: Symbol, var_store: &mut VarStore) -> Def { region: Region::zero(), loc_cond: Box::new(no_region(Var(Symbol::ARG_1))), branches, + branches_cond_var: var_store.fresh(), }; defn( @@ -5133,6 +5138,7 @@ fn result_after(symbol: Symbol, var_store: &mut VarStore) -> Def { region: Region::zero(), loc_cond: Box::new(no_region(Var(Symbol::ARG_1))), branches, + branches_cond_var: var_store.fresh(), }; defn( diff --git a/compiler/can/src/constraint.rs b/compiler/can/src/constraint.rs index e28ff8f229..6927268108 100644 --- a/compiler/can/src/constraint.rs +++ b/compiler/can/src/constraint.rs @@ -574,7 +574,7 @@ impl Constraints { Constraint::IsOpenType(_) => false, Constraint::IncludesTag(_) => false, Constraint::PatternPresence(_, _, _, _) => false, - Constraint::Exhaustive(_, _) => false, + Constraint::Exhaustive { .. } => false, } } @@ -603,10 +603,27 @@ impl Constraints { Constraint::Store(type_index, variable, string_index, line_number) } - pub fn exhaustive(&mut self, rows: SketchedRows, context: ExhaustiveContext) -> Constraint { - let rows_index = Index::push_new(&mut self.sketched_rows, rows); + pub fn exhaustive( + &mut self, + real_var: Variable, + real_region: Region, + real_category: Category, + expected_branches: Expected, + sketched_rows: SketchedRows, + context: ExhaustiveContext, + ) -> Constraint { + let real_category = Index::push_new(&mut self.categories, real_category); + let expected_branches = Index::push_new(&mut self.expectations, expected_branches); + let sketched_rows = Index::push_new(&mut self.sketched_rows, sketched_rows); - Constraint::Exhaustive(rows_index, context) + Constraint::Exhaustive { + real_var, + real_region, + real_category, + expected_branches, + sketched_rows, + context, + } } } @@ -652,7 +669,14 @@ pub enum Constraint { Index, Region, ), - Exhaustive(Index, ExhaustiveContext), + Exhaustive { + real_var: Variable, + real_region: Region, + real_category: Index, + expected_branches: Index>, + sketched_rows: Index, + context: ExhaustiveContext, + }, } #[derive(Debug, Clone, Copy, Default)] @@ -707,8 +731,19 @@ impl std::fmt::Debug for Constraint { arg0, arg1, arg2, arg3 ) } - Self::Exhaustive(arg0, arg1) => { - write!(f, "Exhaustive({:?}, {:?})", arg0, arg1) + Self::Exhaustive { + real_var: arg0, + real_region: arg1, + real_category: arg2, + expected_branches: arg3, + sketched_rows: arg4, + context: arg5, + } => { + write!( + f, + "Exhaustive({:?}, {:?}, {:?}, {:?}, {:?}, {:?})", + arg0, arg1, arg2, arg3, arg4, arg5 + ) } } } diff --git a/compiler/can/src/effect_module.rs b/compiler/can/src/effect_module.rs index d2f6e61c5d..e7ff812d5d 100644 --- a/compiler/can/src/effect_module.rs +++ b/compiler/can/src/effect_module.rs @@ -1366,6 +1366,7 @@ fn build_effect_loop_inner_body( region: Region::zero(), loc_cond: Box::new(force_thunk_call), branches, + branches_cond_var: var_store.fresh(), }; Expr::LetNonRec( diff --git a/compiler/can/src/exhaustive.rs b/compiler/can/src/exhaustive.rs index 060c688d06..e69f445cfd 100644 --- a/compiler/can/src/exhaustive.rs +++ b/compiler/can/src/exhaustive.rs @@ -6,6 +6,7 @@ use roc_exhaustive::{is_useful, Ctor, Error, Guard, Literal, Pattern, RenderAs, use roc_module::ident::{TagIdIntType, TagName}; use roc_region::all::{Loc, Region}; use roc_types::subs::{Content, FlatType, Subs, SubsFmtContent, Variable}; +use roc_types::types::AliasKind; pub use roc_exhaustive::Context as ExhaustiveContext; @@ -22,7 +23,7 @@ pub fn check( roc_exhaustive::check(overall_region, context, matrix) } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] enum SketchedPattern { Anything, Literal(Literal), @@ -52,14 +53,14 @@ impl SketchedPattern { } } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] struct SketchedRow { patterns: Vec, region: Region, guard: Guard, } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct SketchedRows { rows: Vec, overall_region: Region, @@ -155,13 +156,15 @@ fn sketch_pattern(var: Variable, pattern: &crate::pattern::Pattern) -> SketchedP ) } + // Treat this like a literal so we mark it as non-exhaustive + MalformedPattern(..) => SP::Literal(Literal::Byte(1)), + Underscore | Identifier(_) | AbilityMemberSpecialization { .. } | Shadowed(..) | OpaqueNotInScope(..) - | UnsupportedPattern(..) - | MalformedPattern(..) => SP::Anything, + | UnsupportedPattern(..) => SP::Anything, } } @@ -232,7 +235,7 @@ pub fn sketch_rows(target_var: Variable, region: Region, patterns: &[WhenBranch] let row = SketchedRow { patterns, - region, + region: loc_pat.region, guard, }; rows.push(row); @@ -285,7 +288,7 @@ fn convert_tag(subs: &Subs, whole_var: Variable, this_tag: &TagName) -> (Union, use {Content::*, FlatType::*}; - match content { + match dealias_tag(subs, content) { Structure(TagUnion(tags, ext) | RecursiveTagUnion(_, tags, ext)) => { let (sorted_tags, ext) = tags.sorted_iterator_and_ext(subs, *ext); @@ -335,3 +338,18 @@ fn convert_tag(subs: &Subs, whole_var: Variable, this_tag: &TagName) -> (Union, ), } } + +pub fn dealias_tag<'a>(subs: &'a Subs, content: &'a Content) -> &'a Content { + use Content::*; + let mut result = content; + loop { + match result { + Alias(_, _, real_var, AliasKind::Structural) + | RecursionVar { + structure: real_var, + .. + } => result = subs.get_content_without_compacting(*real_var), + _ => return result, + } + } +} diff --git a/compiler/can/src/expr.rs b/compiler/can/src/expr.rs index 15301c832a..19d1b946d3 100644 --- a/compiler/can/src/expr.rs +++ b/compiler/can/src/expr.rs @@ -19,7 +19,7 @@ use roc_parse::pattern::PatternType::*; use roc_problem::can::{PrecedenceProblem, Problem, RuntimeError}; use roc_region::all::{Loc, Region}; use roc_types::subs::{VarStore, Variable}; -use roc_types::types::{Alias, LambdaSet, Type}; +use roc_types::types::{Alias, Category, LambdaSet, Type}; use std::fmt::{Debug, Display}; use std::{char, u32}; @@ -89,6 +89,7 @@ pub enum Expr { region: Region, loc_cond: Box>, branches: Vec, + branches_cond_var: Variable, }, If { cond_var: Variable, @@ -193,6 +194,47 @@ pub enum Expr { // Compiles, but will crash if reached RuntimeError(RuntimeError), } + +impl Expr { + pub fn category(&self) -> Category { + match self { + Self::Num(..) => Category::Num, + Self::Int(..) => Category::Int, + Self::Float(..) => Category::Float, + Self::Str(..) => Category::Str, + Self::SingleQuote(..) => Category::Character, + Self::List { .. } => Category::List, + &Self::Var(sym) => Category::Lookup(sym), + Self::When { .. } => Category::When, + Self::If { .. } => Category::If, + Self::LetRec(_, expr, _) => expr.value.category(), + Self::LetNonRec(_, expr, _) => expr.value.category(), + &Self::Call(_, _, called_via) => Category::CallResult(None, called_via), + &Self::RunLowLevel { op, .. } => Category::LowLevelOpResult(op), + Self::ForeignCall { .. } => Category::ForeignCall, + Self::Closure(..) => Category::Lambda, + Self::Record { .. } => Category::Record, + Self::EmptyRecord => Category::Record, + Self::Access { field, .. } => Category::Access(field.clone()), + Self::Accessor(data) => Category::Accessor(data.field.clone()), + Self::Update { .. } => Category::Record, + Self::Tag { + name, arguments, .. + } => Category::TagApply { + tag_name: name.clone(), + args_count: arguments.len(), + }, + Self::ZeroArgumentTag { name, .. } => Category::TagApply { + tag_name: name.clone(), + args_count: 0, + }, + &Self::OpaqueRef { name, .. } => Category::OpaqueWrap(name), + Self::Expect(..) => Category::Expect, + Self::RuntimeError(..) => Category::Unknown, + } + } +} + #[derive(Clone, Debug)] pub struct ClosureData { pub function_type: Variable, @@ -782,6 +824,7 @@ pub fn canonicalize_expr<'a>( region, loc_cond: Box::new(can_cond), branches: can_branches, + branches_cond_var: var_store.fresh(), }; (expr, output) @@ -1298,6 +1341,7 @@ pub fn inline_calls(var_store: &mut VarStore, scope: &mut Scope, expr: Expr) -> region, loc_cond, branches, + branches_cond_var, } => { let loc_cond = Box::new(Loc { region: loc_cond.region, @@ -1333,6 +1377,7 @@ pub fn inline_calls(var_store: &mut VarStore, scope: &mut Scope, expr: Expr) -> region, loc_cond, branches: new_branches, + branches_cond_var, } } If { diff --git a/compiler/can/src/traverse.rs b/compiler/can/src/traverse.rs index a1f338c4ca..d45f7f3a87 100644 --- a/compiler/can/src/traverse.rs +++ b/compiler/can/src/traverse.rs @@ -65,6 +65,7 @@ fn walk_expr(visitor: &mut V, expr: &Expr) { loc_cond, branches, region: _, + branches_cond_var: _, } => { walk_when(visitor, *cond_var, *expr_var, loc_cond, branches); } diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index 2d39f196a6..ddb094cf38 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -580,17 +580,18 @@ pub fn constrain_expr( } } When { - cond_var, + cond_var: real_cond_var, expr_var, loc_cond, branches, + branches_cond_var, .. } => { - let cond_var = *cond_var; - let cond_type = Variable(cond_var); + let branches_cond_var = *branches_cond_var; + let branches_cond_type = Variable(branches_cond_var); - let branch_var = *expr_var; - let branch_type = Variable(branch_var); + let body_var = *expr_var; + let body_type = Variable(body_var); let branches_region = { debug_assert!(!branches.is_empty()); @@ -615,13 +616,13 @@ pub fn constrain_expr( index, region: ann_source.region(), }, - branch_type.clone(), + body_type.clone(), ) } _ => ForReason( Reason::WhenBranch { index }, - branch_type.clone(), + body_type.clone(), branch_region, ), }; @@ -659,7 +660,7 @@ pub fn constrain_expr( index: HumanIndex::zero_based(index), sub_pattern, }, - cond_type.clone(), + branches_cond_type.clone(), sub_region, ) }; @@ -703,15 +704,16 @@ pub fn constrain_expr( // After solving the condition variable with what's expected from the branch patterns, // check it against the condition expression. - // TODO: when we have exhaustiveness checking during the typechecking phase, perform - // exhaustiveness checking when this expectation fails. That will produce better error - // messages. - let expr_con = constrain_expr( + // + // First, solve the condition type. + let real_cond_var = *real_cond_var; + let real_cond_type = Type::Variable(real_cond_var); + let cond_constraint = constrain_expr( constraints, env, loc_cond.region, &loc_cond.value, - Expected::ForReason(Reason::WhenBranches, cond_type, branches_region), + Expected::NoExpectation(real_cond_type), ); // branch_cons.extend(pattern_cons); // branch_constraints.push(constraints.and_constraint(pattern_cons)); @@ -719,10 +721,18 @@ pub fn constrain_expr( // total_cons.push(expr_con); pattern_cons.push(expr_con); - let sketched_rows = sketch_rows(cond_var, branches_region, &branches); - let exhaustiveness_constraint = - constraints.exhaustive(sketched_rows, ExhaustiveContext::BadCase); - pattern_cons.push(exhaustiveness_constraint); + // Now check the condition against the type expected by the branches. + let sketched_rows = sketch_rows(real_cond_var, branches_region, &branches); + // let exhaustive_reason = Reason::Exhaustive(sketched_rows, ExhaustiveContext::BadCase); + let cond_matches_branches_constraint = constraints.exhaustive( + real_cond_var, + loc_cond.region, + loc_cond.value.category(), + Expected::ForReason(Reason::WhenBranches, branches_cond_type, branches_region), + sketched_rows, + ExhaustiveContext::BadCase, + ); + pattern_cons.push(cond_matches_branches_constraint); // Solve all the pattern constraints together, introducing variables in the pattern as // need be before solving the bodies. @@ -742,7 +752,10 @@ pub fn constrain_expr( let total_cons = [when_body_con, result_con]; let branch_constraints = constraints.and_constraint(total_cons); - constraints.exists([cond_var, *expr_var], branch_constraints) + constraints.exists( + [branches_cond_var, real_cond_var, *expr_var], + branch_constraints, + ) } Access { record_var, diff --git a/compiler/exhaustive/src/lib.rs b/compiler/exhaustive/src/lib.rs index 19638404a4..a01e0a7033 100644 --- a/compiler/exhaustive/src/lib.rs +++ b/compiler/exhaustive/src/lib.rs @@ -72,7 +72,7 @@ pub enum Pattern { Ctor(Union, TagId, std::vec::Vec), } -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum Literal { Int(i128), U128(u128), @@ -96,14 +96,14 @@ pub enum Error { }, } -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Context { BadArg, BadDestruct, BadCase, } -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Guard { HasGuard, NoGuard, diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index f98b26370f..4f4b2accfe 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -2091,6 +2091,7 @@ fn pattern_to_when<'a>( value: body, guard: None, }], + branches_cond_var: pattern_var, }; (symbol, Loc::at_zero(wrapped_body)) @@ -3753,6 +3754,7 @@ pub fn with_hole<'a>( region, loc_cond, branches, + branches_cond_var: _, } => { let cond_symbol = possible_reuse_symbol(env, procs, &loc_cond.value); @@ -5443,6 +5445,7 @@ pub fn from_can<'a>( region, loc_cond, branches, + branches_cond_var: _, } => { let cond_symbol = possible_reuse_symbol(env, procs, &loc_cond.value); diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index e25c82dded..3ce187c1e7 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1197,12 +1197,116 @@ fn solve( } } } - Exhaustive(rows_index, context) => { - let sketched_rows = constraints.sketched_rows[rows_index.index()].clone(); + &Exhaustive { + real_var, + real_region, + real_category, + expected_branches, + sketched_rows, + context, + } => { + // A few cases: + // 1. Either condition or branch types already have a type error. In this case just + // propogate it. + // 2. Types are correct, but there are redundancies. In this case we want + // exhaustiveness checking to pull those out. + // 3. Condition and branch types are "almost equal", that is one or the other is + // only missing a few more tags. In this case we want to run + // exhaustiveness checking both ways, to see which one is missing tags. + // 4. Condition and branch types aren't "almost equal", this is just a normal type + // error. - let checked = roc_can::exhaustive::check(&subs, sketched_rows, *context); - if let Err(errors) = checked { - problems.extend(errors.into_iter().map(TypeError::Exhaustive)); + let expected_branches = &constraints.expectations[expected_branches.index()]; + let branches_var = + type_to_var(subs, rank, pools, aliases, expected_branches.get_type_ref()); + + let real_content = subs.get_content_without_compacting(real_var); + let branches_content = subs.get_content_without_compacting(branches_var); + let already_have_error = match (real_content, branches_content) { + (Content::Error | Content::Structure(FlatType::Erroneous(_)), _) => true, + (_, Content::Error | Content::Structure(FlatType::Erroneous(_))) => true, + _ => false, + }; + + let snapshot = subs.snapshot(); + let outcome = unify(subs, real_var, branches_var, Mode::EQ); + + let should_check_exhaustiveness; + match outcome { + Success { + vars, + must_implement_ability, + } => { + subs.commit_snapshot(snapshot); + + introduce(subs, rank, pools, &vars); + if !must_implement_ability.is_empty() { + internal_error!("Didn't expect ability vars to land here"); + } + + // Case 1: unify error types, but don't check exhaustiveness. + // Case 2: run exhaustiveness to check for redundant branches. + should_check_exhaustiveness = !already_have_error; + } + Failure(..) => { + // Rollback and check for almost-equality. + subs.rollback_to(snapshot); + + let almost_eq_snapshot = subs.snapshot(); + // TODO: turn this on for bidirectional exhaustiveness checking + // open_tag_union(subs, real_var); + open_tag_union(subs, branches_var); + let almost_eq = matches!( + unify(subs, real_var, branches_var, Mode::EQ), + Success { .. } + ); + + subs.rollback_to(almost_eq_snapshot); + + if almost_eq { + // Case 3: almost equal, check exhaustiveness. + should_check_exhaustiveness = true; + } else { + // Case 4: incompatible types, report type error. + // Re-run first failed unification to get the type diff. + match unify(subs, real_var, branches_var, Mode::EQ) { + Failure(vars, actual_type, expected_type, _bad_impls) => { + introduce(subs, rank, pools, &vars); + + let real_category = + constraints.categories[real_category.index()].clone(); + let problem = TypeError::BadExpr( + real_region, + real_category, + actual_type, + expected_branches.replace_ref(expected_type), + ); + + problems.push(problem); + should_check_exhaustiveness = false; + } + _ => internal_error!("Must be failure"), + } + } + } + BadType(vars, problem) => { + subs.commit_snapshot(snapshot); + + introduce(subs, rank, pools, &vars); + + problems.push(TypeError::BadType(problem)); + + should_check_exhaustiveness = false; + } + } + + let sketched_rows = constraints.sketched_rows[sketched_rows.index()].clone(); + + if should_check_exhaustiveness { + let checked = roc_can::exhaustive::check(&subs, sketched_rows, context); + if let Err(errors) = checked { + problems.extend(errors.into_iter().map(TypeError::Exhaustive)); + } } state @@ -1213,6 +1317,35 @@ fn solve( state } +fn open_tag_union(subs: &mut Subs, var: Variable) { + let mut stack = vec![var]; + while let Some(var) = stack.pop() { + use {Content::*, FlatType::*}; + + let mut desc = subs.get(var); + if let Structure(TagUnion(tags, ext)) = desc.content { + if let Structure(EmptyTagUnion) = subs.get_content_without_compacting(ext) { + let new_ext = subs.fresh_unnamed_flex_var(); + subs.set_rank(new_ext, desc.rank); + let new_union = Structure(TagUnion(tags, new_ext)); + desc.content = new_union; + subs.set(var, desc); + } + + // Also open up all nested tag unions. + let all_vars = tags.variables().into_iter(); + stack.extend(all_vars.flat_map(|slice| subs[slice]).map(|var| subs[var])); + } + + // Today, an "open" constraint doesn't affect any types + // other than tag unions. Recursive tag unions are constructed + // at a later time (during occurs checks after tag unions are + // resolved), so that's not handled here either. + // NB: Handle record types here if we add presence constraints + // to their type inference as well. + } +} + /// If a symbol claims to specialize an ability member, check that its solved type in fact /// does specialize the ability, and record the specialization. #[allow(clippy::too_many_arguments)] diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index 6a5d90d3c7..76013dee4c 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -6163,7 +6163,7 @@ mod solve_expr { ), &[ "ob : Bool", - "ob : [ False, True ]", + "ob : Bool", "True : [ False, True ]", "False : [ False, True ]", ], diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index 1d9a181c67..f9c129693a 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -2118,6 +2118,18 @@ impl Content { self } + + pub fn unroll_structural_alias<'a>(&'a self, subs: &'a Subs) -> &'a Self { + let mut result = self; + loop { + match result { + Self::Alias(_, _, real_var, AliasKind::Structural) => { + result = subs.get_content_without_compacting(*real_var) + } + _ => return result, + } + } + } } #[derive(Clone, Copy, Debug)] diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index b9c3a9a261..96994d3d20 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -1815,6 +1815,9 @@ pub enum Category { DefaultValue(Lowercase), // for setting optional fields AbilityMemberSpecialization(Symbol), + + Expect, + Unknown, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index 97a0d27a24..71230ab115 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -1528,7 +1528,7 @@ fn format_category<'b>( alloc.concat([this_is, alloc.text(" an uniqueness attribute")]), alloc.text(" of type:"), ), - Storage(_file, _line) => ( + Storage(..) | Unknown => ( alloc.concat([this_is, alloc.text(" a value")]), alloc.text(" of type:"), ), @@ -1540,6 +1540,10 @@ fn format_category<'b>( alloc.concat([this_is, alloc.text(" a declared specialization")]), alloc.text(" of type:"), ), + Expect => ( + alloc.concat([this_is, alloc.text(" an expectation")]), + alloc.text(" of type:"), + ), } } diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index a918bdeda0..c8f1741887 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -1197,6 +1197,7 @@ mod test_reporting { when 1 is 2 -> "foo" 3 -> {} + _ -> "" "# ), indoc!( @@ -1205,10 +1206,10 @@ mod test_reporting { The 2nd branch of this `when` does not match all the previous branches: - 1│ when 1 is - 2│ 2 -> "foo" - 3│ 3 -> {} - ^^ + 1│ when 1 is + 2│ 2 -> "foo" + 3│> 3 -> {} + 4│ _ -> "" The 2nd branch is a record of type: @@ -1788,7 +1789,7 @@ mod test_reporting { indoc!( r#" when { foo: 1 } is - { foo: 2 } -> foo + { foo: _ } -> foo "# ), indoc!( @@ -1797,7 +1798,7 @@ mod test_reporting { I cannot find a `foo` value - 2│ { foo: 2 } -> foo + 2│ { foo: _ } -> foo ^^^ Did you mean one of these? @@ -2791,26 +2792,18 @@ mod test_reporting { ), indoc!( r#" - ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─ + ── UNSAFE PATTERN ──────────────────────────────────────── /code/proj/Main.roc ─ - The branches of this `when` expression don't match the condition: + This `when` does not cover all the possibilities: 4│> when x is - 5│ Red -> 3 + 5│> Red -> 3 - This `x` value is a: + Other possibilities include: - [ Green, Red ] + Green - But the branch patterns have type: - - [ Red ] - - The branches must be cases of the `when` condition's type! - - Tip: Looks like the branches are missing coverage of the `Green` tag. - - Tip: Maybe you need to add a catch-all branch, like `_`? + I would have to crash if I saw one of those! Add branches for them! "# ), ) @@ -2831,27 +2824,19 @@ mod test_reporting { ), indoc!( r#" - ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─ + ── UNSAFE PATTERN ──────────────────────────────────────── /code/proj/Main.roc ─ - The branches of this `when` expression don't match the condition: + This `when` does not cover all the possibilities: 4│> when x is - 5│ Red -> 0 - 6│ Green -> 1 + 5│> Red -> 0 + 6│> Green -> 1 - This `x` value is a: + Other possibilities include: - [ Blue, Green, Red ] + Blue - But the branch patterns have type: - - [ Green, Red ] - - The branches must be cases of the `when` condition's type! - - Tip: Looks like the branches are missing coverage of the `Blue` tag. - - Tip: Maybe you need to add a catch-all branch, like `_`? + I would have to crash if I saw one of those! Add branches for them! "# ), ) @@ -2872,27 +2857,20 @@ mod test_reporting { ), indoc!( r#" - ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─ + ── UNSAFE PATTERN ──────────────────────────────────────── /code/proj/Main.roc ─ - The branches of this `when` expression don't match the condition: + This `when` does not cover all the possibilities: 5│> when x is - 6│ NotAsked -> 3 + 6│> NotAsked -> 3 - This `x` value is a: + Other possibilities include: - [ Failure I64, Loading, NotAsked, Success Str ] + Failure _ + Loading + Success _ - But the branch patterns have type: - - [ NotAsked ] - - The branches must be cases of the `when` condition's type! - - Tip: Looks like the branches are missing coverage of the - `Success`, `Failure` and `Loading` tags. - - Tip: Maybe you need to add a catch-all branch, like `_`? + I would have to crash if I saw one of those! Add branches for them! "# ), ) @@ -2955,7 +2933,7 @@ mod test_reporting { Other possibilities include: - { a: Just _, b } + { a: Just _ } I would have to crash if I saw one of those! Add branches for them! "# From 372b12bec96a4f1d2ebd565972b3894f16ec56d4 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 15:27:08 -0400 Subject: [PATCH 07/17] Typo --- compiler/solve/src/solve.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 3ce187c1e7..6784a10082 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1207,7 +1207,7 @@ fn solve( } => { // A few cases: // 1. Either condition or branch types already have a type error. In this case just - // propogate it. + // propagate it. // 2. Types are correct, but there are redundancies. In this case we want // exhaustiveness checking to pull those out. // 3. Condition and branch types are "almost equal", that is one or the other is From 7d908dc99c763e377062895ac6789bd5877fc123 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 15:28:06 -0400 Subject: [PATCH 08/17] Remove unused line --- compiler/constrain/src/expr.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index ddb094cf38..4cc62269f5 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -723,7 +723,6 @@ pub fn constrain_expr( // Now check the condition against the type expected by the branches. let sketched_rows = sketch_rows(real_cond_var, branches_region, &branches); - // let exhaustive_reason = Reason::Exhaustive(sketched_rows, ExhaustiveContext::BadCase); let cond_matches_branches_constraint = constraints.exhaustive( real_cond_var, loc_cond.region, From 17e7b10267197022dec6767319921031face3e90 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 15:30:55 -0400 Subject: [PATCH 09/17] Fought against clippy and lost --- compiler/constrain/src/expr.rs | 2 +- compiler/mono/src/ir.rs | 4 ++-- compiler/solve/src/solve.rs | 17 +++++++++++------ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index 4cc62269f5..86c01a56c2 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -722,7 +722,7 @@ pub fn constrain_expr( pattern_cons.push(expr_con); // Now check the condition against the type expected by the branches. - let sketched_rows = sketch_rows(real_cond_var, branches_region, &branches); + let sketched_rows = sketch_rows(real_cond_var, branches_region, branches); let cond_matches_branches_constraint = constraints.exhaustive( real_cond_var, loc_cond.region, diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 4f4b2accfe..1a78f24f3c 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -5847,7 +5847,7 @@ fn to_opt_branches<'a>( Ok((mono_pattern, assignments)) => { loc_branches.push(( Loc::at(loc_pattern.region, mono_pattern.clone()), - exhaustive_guard.clone(), + exhaustive_guard, )); let mut loc_expr = when_branch.value.clone(); @@ -5877,7 +5877,7 @@ fn to_opt_branches<'a>( Err(runtime_error) => { loc_branches.push(( Loc::at(loc_pattern.region, Pattern::Underscore), - exhaustive_guard.clone(), + exhaustive_guard, )); // TODO remove clone? diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 6784a10082..7b9d8d65ef 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1222,11 +1222,16 @@ fn solve( let real_content = subs.get_content_without_compacting(real_var); let branches_content = subs.get_content_without_compacting(branches_var); - let already_have_error = match (real_content, branches_content) { - (Content::Error | Content::Structure(FlatType::Erroneous(_)), _) => true, - (_, Content::Error | Content::Structure(FlatType::Erroneous(_))) => true, - _ => false, - }; + let already_have_error = matches!( + (real_content, branches_content), + ( + Content::Error | Content::Structure(FlatType::Erroneous(_)), + _ + ) | ( + _, + Content::Error | Content::Structure(FlatType::Erroneous(_)) + ) + ); let snapshot = subs.snapshot(); let outcome = unify(subs, real_var, branches_var, Mode::EQ); @@ -1303,7 +1308,7 @@ fn solve( let sketched_rows = constraints.sketched_rows[sketched_rows.index()].clone(); if should_check_exhaustiveness { - let checked = roc_can::exhaustive::check(&subs, sketched_rows, context); + let checked = roc_can::exhaustive::check(subs, sketched_rows, context); if let Err(errors) = checked { problems.extend(errors.into_iter().map(TypeError::Exhaustive)); } From 1de67fe19a3b93dcb7c88cef127b082affc0f6ad Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 16:00:42 -0400 Subject: [PATCH 10/17] Nested Eq constraint in Exhaustive behind SoA --- compiler/can/src/constraint.rs | 67 +++++++++++++--------------------- compiler/solve/src/solve.rs | 21 ++++++----- 2 files changed, 38 insertions(+), 50 deletions(-) diff --git a/compiler/can/src/constraint.rs b/compiler/can/src/constraint.rs index 6927268108..7a8857a413 100644 --- a/compiler/can/src/constraint.rs +++ b/compiler/can/src/constraint.rs @@ -21,6 +21,7 @@ pub struct Constraints { pub includes_tags: Vec, pub strings: Vec<&'static str>, pub sketched_rows: Vec, + pub eq: Vec, } impl Default for Constraints { @@ -43,6 +44,7 @@ impl Constraints { let includes_tags = Vec::new(); let strings = Vec::new(); let sketched_rows = Vec::new(); + let eq = Vec::new(); types.extend([ Type::EmptyRec, @@ -94,6 +96,7 @@ impl Constraints { includes_tags, strings, sketched_rows, + eq, } } @@ -229,7 +232,7 @@ impl Constraints { let expected_index = Index::push_new(&mut self.expectations, expected); let category_index = Self::push_category(self, category); - Constraint::Eq(type_index, expected_index, category_index, region) + Constraint::Eq(Eq(type_index, expected_index, category_index, region)) } #[inline(always)] @@ -244,7 +247,7 @@ impl Constraints { let expected_index = Index::push_new(&mut self.expectations, expected); let category_index = Self::push_category(self, category); - Constraint::Eq(type_index, expected_index, category_index, region) + Constraint::Eq(Eq(type_index, expected_index, category_index, region)) } #[inline(always)] @@ -260,17 +263,17 @@ impl Constraints { let expected_index = Index::push_new(&mut self.expectations, expected); let category_index = Self::push_category(self, category); - let equal = Constraint::Eq(type_index, expected_index, category_index, region); + let equal = Constraint::Eq(Eq(type_index, expected_index, category_index, region)); let storage_type_index = Self::push_type_variable(storage_var); let storage_category = Category::Storage(std::file!(), std::line!()); let storage_category_index = Self::push_category(self, storage_category); - let storage = Constraint::Eq( + let storage = Constraint::Eq(Eq( storage_type_index, expected_index, storage_category_index, region, - ); + )); self.and_constraint([equal, storage]) } @@ -612,31 +615,31 @@ impl Constraints { sketched_rows: SketchedRows, context: ExhaustiveContext, ) -> Constraint { + let real_var = Self::push_type_variable(real_var); let real_category = Index::push_new(&mut self.categories, real_category); let expected_branches = Index::push_new(&mut self.expectations, expected_branches); + let equality = Eq(real_var, expected_branches, real_category, real_region); + let equality = Index::push_new(&mut self.eq, equality); let sketched_rows = Index::push_new(&mut self.sketched_rows, sketched_rows); - Constraint::Exhaustive { - real_var, - real_region, - real_category, - expected_branches, - sketched_rows, - context, - } + Constraint::Exhaustive(equality, sketched_rows, context) } } roc_error_macros::assert_sizeof_default!(Constraint, 3 * 8); +roc_error_macros::assert_sizeof_aarch64!(Constraint, 3 * 8); + +#[derive(Clone, Copy, Debug)] +pub struct Eq( + pub EitherIndex, + pub Index>, + pub Index, + pub Region, +); #[derive(Clone, Copy)] pub enum Constraint { - Eq( - EitherIndex, - Index>, - Index, - Region, - ), + Eq(Eq), Store( EitherIndex, Variable, @@ -669,14 +672,7 @@ pub enum Constraint { Index, Region, ), - Exhaustive { - real_var: Variable, - real_region: Region, - real_category: Index, - expected_branches: Index>, - sketched_rows: Index, - context: ExhaustiveContext, - }, + Exhaustive(Index, Index, ExhaustiveContext), } #[derive(Debug, Clone, Copy, Default)] @@ -706,7 +702,7 @@ pub struct IncludesTag { impl std::fmt::Debug for Constraint { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Eq(arg0, arg1, arg2, arg3) => { + Self::Eq(Eq(arg0, arg1, arg2, arg3)) => { write!(f, "Eq({:?}, {:?}, {:?}, {:?})", arg0, arg1, arg2, arg3) } Self::Store(arg0, arg1, arg2, arg3) => { @@ -731,19 +727,8 @@ impl std::fmt::Debug for Constraint { arg0, arg1, arg2, arg3 ) } - Self::Exhaustive { - real_var: arg0, - real_region: arg1, - real_category: arg2, - expected_branches: arg3, - sketched_rows: arg4, - context: arg5, - } => { - write!( - f, - "Exhaustive({:?}, {:?}, {:?}, {:?}, {:?}, {:?})", - arg0, arg1, arg2, arg3, arg4, arg5 - ) + Self::Exhaustive(arg0, arg1, arg2) => { + write!(f, "Exhaustive({:?}, {:?}, {:?})", arg0, arg1, arg2) } } } diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 7b9d8d65ef..72930cd335 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -776,7 +776,7 @@ fn solve( copy } - Eq(type_index, expectation_index, category_index, region) => { + Eq(roc_can::constraint::Eq(type_index, expectation_index, category_index, region)) => { let category = &constraints.categories[category_index.index()]; let actual = @@ -1197,14 +1197,7 @@ fn solve( } } } - &Exhaustive { - real_var, - real_region, - real_category, - expected_branches, - sketched_rows, - context, - } => { + &Exhaustive(eq, sketched_rows, context) => { // A few cases: // 1. Either condition or branch types already have a type error. In this case just // propagate it. @@ -1216,6 +1209,16 @@ fn solve( // 4. Condition and branch types aren't "almost equal", this is just a normal type // error. + let roc_can::constraint::Eq( + real_var, + expected_branches, + real_category, + real_region, + ) = constraints.eq[eq.index()]; + + let real_var = + either_type_index_to_var(constraints, subs, rank, pools, aliases, real_var); + let expected_branches = &constraints.expectations[expected_branches.index()]; let branches_var = type_to_var(subs, rank, pools, aliases, expected_branches.get_type_ref()); From fe8eb38d8960c0e8c901d6eecdcbbe934fc72263 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 16:12:04 -0400 Subject: [PATCH 11/17] Catch non-exhaustive open unions --- reporting/src/error/type.rs | 21 +++++++++++++++----- reporting/tests/test_reporting.rs | 33 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index 71230ab115..d52f69bfdc 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -3691,12 +3691,18 @@ fn pattern_to_doc_help<'b>( .append(" }") } RenderAs::Tag | RenderAs::Opaque => { - let has_args = !args.is_empty(); - let arg_docs = args - .into_iter() - .map(|v| pattern_to_doc_help(alloc, v, true)); - let tag = &union.alternatives[tag_id.0 as usize]; + match &tag.name { + TagName::Global(name) if name.as_str() == "#Open" => { + return pattern_to_doc_help( + alloc, + roc_exhaustive::Pattern::Anything, + in_type_param, + ) + } + _ => {} + } + let tag_name = match union.render_as { RenderAs::Tag => alloc.tag_name(tag.name.clone()), RenderAs::Opaque => match tag.name { @@ -3706,6 +3712,11 @@ fn pattern_to_doc_help<'b>( _ => unreachable!(), }; + let has_args = !args.is_empty(); + let arg_docs = args + .into_iter() + .map(|v| pattern_to_doc_help(alloc, v, true)); + // We assume the alternatives are sorted. If not, this assert will trigger debug_assert!(tag_id == tag.tag_id); diff --git a/reporting/tests/test_reporting.rs b/reporting/tests/test_reporting.rs index c8f1741887..76ec6596f8 100644 --- a/reporting/tests/test_reporting.rs +++ b/reporting/tests/test_reporting.rs @@ -9881,4 +9881,37 @@ I need all branches in an `if` to have the same type! "", ) } + + #[test] + fn not_enough_cases_for_open_union() { + new_report_problem_as( + "branches_have_more_cases_than_condition", + indoc!( + r#" + foo : [A, B]a -> Str + foo = \it -> + when it is + A -> "" + foo + "# + ), + indoc!( + r#" + ── UNSAFE PATTERN ──────────────────────────────────────── /code/proj/Main.roc ─ + + This `when` does not cover all the possibilities: + + 6│> when it is + 7│> A -> "" + + Other possibilities include: + + B + _ + + I would have to crash if I saw one of those! Add branches for them! + "# + ), + ) + } } From 03deec23c363e81b882dfc613282ed47000ed96f Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 16:16:21 -0400 Subject: [PATCH 12/17] Constants --- compiler/can/src/exhaustive.rs | 7 +++++-- compiler/types/src/subs.rs | 12 ------------ reporting/src/error/type.rs | 5 +++-- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/compiler/can/src/exhaustive.rs b/compiler/can/src/exhaustive.rs index e69f445cfd..235e347772 100644 --- a/compiler/can/src/exhaustive.rs +++ b/compiler/can/src/exhaustive.rs @@ -10,6 +10,9 @@ use roc_types::types::AliasKind; pub use roc_exhaustive::Context as ExhaustiveContext; +pub const GUARD_CTOR: &'static str = "#Guard"; +pub const NONEXHAUSIVE_CTOR: &'static str = "#Open"; + pub fn check( subs: &Subs, sketched_rows: SketchedRows, @@ -215,7 +218,7 @@ pub fn sketch_rows(target_var: Variable, region: Region, patterns: &[WhenBranch] render_as: RenderAs::Guard, alternatives: vec![Ctor { tag_id, - name: TagName::Global("#Guard".into()), + name: TagName::Global(GUARD_CTOR.into()), arity: 2, }], }; @@ -298,7 +301,7 @@ fn convert_tag(subs: &Subs, whole_var: Variable, this_tag: &TagName) -> (Union, // be matched unless there's an `Anything` pattern. let opt_openness_tag = match subs.get_content_without_compacting(ext) { FlexVar(_) | RigidVar(_) => { - let openness_tag = TagName::Global("#Open".into()); + let openness_tag = TagName::Global(NONEXHAUSIVE_CTOR.into()); num_tags += 1; Some((openness_tag, &[] as _)) } diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index f9c129693a..1d9a181c67 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -2118,18 +2118,6 @@ impl Content { self } - - pub fn unroll_structural_alias<'a>(&'a self, subs: &'a Subs) -> &'a Self { - let mut result = self; - loop { - match result { - Self::Alias(_, _, real_var, AliasKind::Structural) => { - result = subs.get_content_without_compacting(*real_var) - } - _ => return result, - } - } - } } #[derive(Clone, Copy, Debug)] diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index d52f69bfdc..3d1d6f58d6 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -3632,6 +3632,7 @@ fn pattern_to_doc_help<'b>( pattern: roc_exhaustive::Pattern, in_type_param: bool, ) -> RocDocBuilder<'b> { + use roc_can::exhaustive::{GUARD_CTOR, NONEXHAUSIVE_CTOR}; use roc_exhaustive::Literal::*; use roc_exhaustive::Pattern::*; use roc_exhaustive::RenderAs; @@ -3654,7 +3655,7 @@ fn pattern_to_doc_help<'b>( // #Guard debug_assert_eq!( union.alternatives[tag_id.0 as usize].name, - TagName::Global("#Guard".into()) + TagName::Global(GUARD_CTOR.into()) ); debug_assert!(args.len() == 2); let tag = pattern_to_doc_help(alloc, args[1].clone(), in_type_param); @@ -3693,7 +3694,7 @@ fn pattern_to_doc_help<'b>( RenderAs::Tag | RenderAs::Opaque => { let tag = &union.alternatives[tag_id.0 as usize]; match &tag.name { - TagName::Global(name) if name.as_str() == "#Open" => { + TagName::Global(name) if name.as_str() == NONEXHAUSIVE_CTOR => { return pattern_to_doc_help( alloc, roc_exhaustive::Pattern::Anything, From 1ff18af8f630beadcff8833b0a91985723aa3b3a Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 22 Apr 2022 17:53:10 -0400 Subject: [PATCH 13/17] Nothing is static in this world --- compiler/can/src/exhaustive.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/can/src/exhaustive.rs b/compiler/can/src/exhaustive.rs index 235e347772..0bd99a0060 100644 --- a/compiler/can/src/exhaustive.rs +++ b/compiler/can/src/exhaustive.rs @@ -10,8 +10,8 @@ use roc_types::types::AliasKind; pub use roc_exhaustive::Context as ExhaustiveContext; -pub const GUARD_CTOR: &'static str = "#Guard"; -pub const NONEXHAUSIVE_CTOR: &'static str = "#Open"; +pub const GUARD_CTOR: &str = "#Guard"; +pub const NONEXHAUSIVE_CTOR: &str = "#Open"; pub fn check( subs: &Subs, From 752b3ee042c733d35406bad5320edb02e7d45aec Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Mon, 25 Apr 2022 08:37:52 -0400 Subject: [PATCH 14/17] Bugfix merge conflicts --- compiler/constrain/src/expr.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index 86c01a56c2..bf08238fc5 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -715,11 +715,7 @@ pub fn constrain_expr( &loc_cond.value, Expected::NoExpectation(real_cond_type), ); - // branch_cons.extend(pattern_cons); - // branch_constraints.push(constraints.and_constraint(pattern_cons)); - let mut total_cons = Vec::with_capacity(1 + 2 * branches.len() + 1); - // total_cons.push(expr_con); - pattern_cons.push(expr_con); + pattern_cons.push(cond_constraint); // Now check the condition against the type expected by the branches. let sketched_rows = sketch_rows(real_cond_var, branches_region, branches); @@ -746,7 +742,7 @@ pub fn constrain_expr( ); let result_con = - constraints.equal_types_var(branch_var, expected, Category::When, region); + constraints.equal_types_var(body_var, expected, Category::When, region); let total_cons = [when_body_con, result_con]; let branch_constraints = constraints.and_constraint(total_cons); From 07e69c786039004dad518c179d6cff53ef6201ed Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Mon, 25 Apr 2022 14:09:29 -0400 Subject: [PATCH 15/17] Style fixes --- compiler/can/src/constraint.rs | 18 +++++++++--------- compiler/can/src/exhaustive.rs | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/compiler/can/src/constraint.rs b/compiler/can/src/constraint.rs index 7a8857a413..7ef3cceaaf 100644 --- a/compiler/can/src/constraint.rs +++ b/compiler/can/src/constraint.rs @@ -551,11 +551,6 @@ impl Constraints { pub fn contains_save_the_environment(&self, constraint: &Constraint) -> bool { match constraint { - Constraint::Eq(..) => false, - Constraint::Store(..) => false, - Constraint::Lookup(..) => false, - Constraint::Pattern(..) => false, - Constraint::True => false, Constraint::SaveTheEnvironment => true, Constraint::Let(index, _) => { let let_constraint = &self.let_constraints[index.index()]; @@ -574,10 +569,15 @@ impl Constraints { .iter() .any(|c| self.contains_save_the_environment(c)) } - Constraint::IsOpenType(_) => false, - Constraint::IncludesTag(_) => false, - Constraint::PatternPresence(_, _, _, _) => false, - Constraint::Exhaustive { .. } => false, + Constraint::Eq(..) + | Constraint::Store(..) + | Constraint::Lookup(..) + | Constraint::Pattern(..) + | Constraint::True + | Constraint::IsOpenType(_) + | Constraint::IncludesTag(_) + | Constraint::PatternPresence(_, _, _, _) + | Constraint::Exhaustive { .. } => false, } } diff --git a/compiler/can/src/exhaustive.rs b/compiler/can/src/exhaustive.rs index 0bd99a0060..d11df7aaee 100644 --- a/compiler/can/src/exhaustive.rs +++ b/compiler/can/src/exhaustive.rs @@ -30,8 +30,8 @@ pub fn check( enum SketchedPattern { Anything, Literal(Literal), - Ctor(Variable, TagName, std::vec::Vec), - KnownCtor(Union, TagId, std::vec::Vec), + Ctor(Variable, TagName, Vec), + KnownCtor(Union, TagId, Vec), } impl SketchedPattern { From 19233e08c2341856c30d69ec20f2b4568c36cdb5 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Mon, 25 Apr 2022 14:10:32 -0400 Subject: [PATCH 16/17] Consolidate IsOpen solve --- compiler/solve/src/solve.rs | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index 72930cd335..7bbf7d5751 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -1105,32 +1105,7 @@ fn solve( let actual = either_type_index_to_var(constraints, subs, rank, pools, aliases, *type_index); - let mut stack = vec![actual]; - while let Some(var) = stack.pop() { - use {Content::*, FlatType::*}; - - let mut desc = subs.get(var); - if let Structure(TagUnion(tags, ext)) = desc.content { - if let Structure(EmptyTagUnion) = subs.get_content_without_compacting(ext) { - let new_ext = subs.fresh_unnamed_flex_var(); - subs.set_rank(new_ext, desc.rank); - let new_union = Structure(TagUnion(tags, new_ext)); - desc.content = new_union; - subs.set(var, desc); - } - - // Also open up all nested tag unions. - let all_vars = tags.variables().into_iter(); - stack.extend(all_vars.flat_map(|slice| subs[slice]).map(|var| subs[var])); - } - - // Today, an "open" constraint doesn't affect any types - // other than tag unions. Recursive tag unions are constructed - // at a later time (during occurs checks after tag unions are - // resolved), so that's not handled here either. - // NB: Handle record types here if we add presence constraints - // to their type inference as well. - } + open_tag_union(subs, actual); state } From 909331c1b1b0574b0d3de0814d8f03566c401312 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Mon, 25 Apr 2022 19:07:57 -0400 Subject: [PATCH 17/17] Bugfix compile error --- compiler/can/src/exhaustive.rs | 12 +++++++----- reporting/src/error/type.rs | 29 +++++++++++++++-------------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/compiler/can/src/exhaustive.rs b/compiler/can/src/exhaustive.rs index d11df7aaee..65ced4608a 100644 --- a/compiler/can/src/exhaustive.rs +++ b/compiler/can/src/exhaustive.rs @@ -2,7 +2,9 @@ use crate::expr::{IntValue, WhenBranch}; use crate::pattern::DestructType; use roc_collections::all::HumanIndex; use roc_error_macros::internal_error; -use roc_exhaustive::{is_useful, Ctor, Error, Guard, Literal, Pattern, RenderAs, TagId, Union}; +use roc_exhaustive::{ + is_useful, Ctor, CtorName, Error, Guard, Literal, Pattern, RenderAs, TagId, Union, +}; use roc_module::ident::{TagIdIntType, TagName}; use roc_region::all::{Loc, Region}; use roc_types::subs::{Content, FlatType, Subs, SubsFmtContent, Variable}; @@ -114,7 +116,7 @@ fn sketch_pattern(var: Variable, pattern: &crate::pattern::Pattern) -> SketchedP let union = Union { render_as: RenderAs::Record(field_names), alternatives: vec![Ctor { - name: TagName::Global("#Record".into()), + name: CtorName::Tag(TagName::Global("#Record".into())), tag_id, arity: destructs.len(), }], @@ -146,7 +148,7 @@ fn sketch_pattern(var: Variable, pattern: &crate::pattern::Pattern) -> SketchedP let union = Union { render_as: RenderAs::Opaque, alternatives: vec![Ctor { - name: TagName::Private(*opaque), + name: CtorName::Opaque(*opaque), tag_id, arity: 1, }], @@ -218,7 +220,7 @@ pub fn sketch_rows(target_var: Variable, region: Region, patterns: &[WhenBranch] render_as: RenderAs::Guard, alternatives: vec![Ctor { tag_id, - name: TagName::Global(GUARD_CTOR.into()), + name: CtorName::Tag(TagName::Global(GUARD_CTOR.into())), arity: 2, }], }; @@ -322,7 +324,7 @@ fn convert_tag(subs: &Subs, whole_var: Variable, this_tag: &TagName) -> (Union, my_tag_id = tag_id; } alternatives.push(Ctor { - name: tag, + name: CtorName::Tag(tag), tag_id, arity: args.len(), }); diff --git a/reporting/src/error/type.rs b/reporting/src/error/type.rs index 3d1d6f58d6..a71bce5847 100644 --- a/reporting/src/error/type.rs +++ b/reporting/src/error/type.rs @@ -1,6 +1,7 @@ use crate::report::{Annotation, Report, RocDocAllocator, RocDocBuilder, Severity}; use roc_can::expected::{Expected, PExpected}; use roc_collections::all::{HumanIndex, MutSet, SendMap}; +use roc_exhaustive::CtorName; use roc_module::called_via::{BinOp, CalledVia}; use roc_module::ident::{Ident, IdentStr, Lowercase, TagName}; use roc_module::symbol::Symbol; @@ -3653,10 +3654,9 @@ fn pattern_to_doc_help<'b>( match union.render_as { RenderAs::Guard => { // #Guard - debug_assert_eq!( - union.alternatives[tag_id.0 as usize].name, - TagName::Global(GUARD_CTOR.into()) - ); + debug_assert!(union.alternatives[tag_id.0 as usize] + .name + .is_tag(&TagName::Global(GUARD_CTOR.into()))); debug_assert!(args.len() == 2); let tag = pattern_to_doc_help(alloc, args[1].clone(), in_type_param); alloc.concat([ @@ -3692,9 +3692,11 @@ fn pattern_to_doc_help<'b>( .append(" }") } RenderAs::Tag | RenderAs::Opaque => { - let tag = &union.alternatives[tag_id.0 as usize]; - match &tag.name { - TagName::Global(name) if name.as_str() == NONEXHAUSIVE_CTOR => { + let ctor = &union.alternatives[tag_id.0 as usize]; + match &ctor.name { + CtorName::Tag(TagName::Global(name)) + if name.as_str() == NONEXHAUSIVE_CTOR => + { return pattern_to_doc_help( alloc, roc_exhaustive::Pattern::Anything, @@ -3704,12 +3706,11 @@ fn pattern_to_doc_help<'b>( _ => {} } - let tag_name = match union.render_as { - RenderAs::Tag => alloc.tag_name(tag.name.clone()), - RenderAs::Opaque => match tag.name { - TagName::Private(opaque) => alloc.wrapped_opaque_name(opaque), - _ => unreachable!(), - }, + let tag_name = match (union.render_as, &ctor.name) { + (RenderAs::Tag, CtorName::Tag(tag)) => alloc.tag_name(tag.clone()), + (RenderAs::Opaque, CtorName::Opaque(opaque)) => { + alloc.wrapped_opaque_name(*opaque) + } _ => unreachable!(), }; @@ -3719,7 +3720,7 @@ fn pattern_to_doc_help<'b>( .map(|v| pattern_to_doc_help(alloc, v, true)); // We assume the alternatives are sorted. If not, this assert will trigger - debug_assert!(tag_id == tag.tag_id); + debug_assert!(tag_id == ctor.tag_id); let docs = std::iter::once(tag_name).chain(arg_docs);