diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index af00476c12..630e01f23f 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/compiler/mono/src/decision_tree.rs @@ -9,7 +9,7 @@ use roc_module::symbol::Symbol; use crate::expr::specialize_equality; use crate::layout::Builtin; use crate::layout::Layout; -use crate::pattern::{Ctor, Union}; +use crate::pattern::{Ctor, TagId, Union}; /// COMPILE CASES @@ -383,6 +383,7 @@ fn test_at_path<'a>(selected_path: &Path, branch: Branch<'a>, all_tests: &mut Ve RecordDestructure(destructs, _) => { let union = Union { alternatives: vec![Ctor { + tag_id: TagId(0), name: TagName::Global("#Record".into()), arity: destructs.len(), }], diff --git a/compiler/mono/src/expr.rs b/compiler/mono/src/expr.rs index 3b0e6afdf3..01202fd936 100644 --- a/compiler/mono/src/expr.rs +++ b/compiler/mono/src/expr.rs @@ -1,5 +1,5 @@ use crate::layout::{Builtin, Layout}; -use crate::pattern::{Ctor, Guard}; +use crate::pattern::{Ctor, Guard, TagId}; use bumpalo::collections::Vec; use bumpalo::Bump; use roc_can; @@ -1497,6 +1497,7 @@ fn from_can_pattern<'a>( tag_name: tag_name.clone(), union: Union { alternatives: vec![Ctor { + tag_id: TagId(0), name: tag_name.clone(), arity: 0, }], @@ -1508,11 +1509,13 @@ fn from_can_pattern<'a>( union: Union { alternatives: vec![ Ctor { - name: ttrue, + tag_id: TagId(0), + name: ffalse, arity: 0, }, Ctor { - name: ffalse, + tag_id: TagId(1), + name: ttrue, arity: 0, }, ], @@ -1525,8 +1528,9 @@ fn from_can_pattern<'a>( .expect("tag must be in its own type"); let mut ctors = std::vec::Vec::with_capacity(tag_names.len()); - for tag_name in &tag_names { + for (i, tag_name) in tag_names.iter().enumerate() { ctors.push(Ctor { + tag_id: TagId(i as u8), name: tag_name.clone(), arity: 0, }) @@ -1545,6 +1549,7 @@ fn from_can_pattern<'a>( Unwrapped(field_layouts) => { let union = crate::pattern::Union { alternatives: vec![Ctor { + tag_id: TagId(0), name: tag_name.clone(), arity: field_layouts.len(), }], @@ -1567,8 +1572,9 @@ fn from_can_pattern<'a>( } Wrapped(tags) => { let mut ctors = std::vec::Vec::with_capacity(tags.len()); - for (tag_name, args) in &tags { + for (i, (tag_name, args)) in tags.iter().enumerate() { ctors.push(Ctor { + tag_id: TagId(i as u8), name: tag_name.clone(), // don't include tag discriminant in arity arity: args.len() - 1, diff --git a/compiler/mono/src/pattern.rs b/compiler/mono/src/pattern.rs index 96a3e26b0a..211c001748 100644 --- a/compiler/mono/src/pattern.rs +++ b/compiler/mono/src/pattern.rs @@ -9,10 +9,13 @@ pub struct Union { pub alternatives: Vec, } +#[derive(Clone, Debug, PartialEq, Eq, Hash, Copy)] +pub struct TagId(pub u8); + #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct Ctor { pub name: TagName, - // pub tag_id: u8, + pub tag_id: TagId, pub arity: usize, } @@ -20,7 +23,7 @@ pub struct Ctor { pub enum Pattern { Anything, Literal(Literal), - Ctor(Union, TagName, std::vec::Vec), + Ctor(Union, TagId, std::vec::Vec), } #[derive(Clone, Debug, PartialEq)] @@ -42,19 +45,17 @@ fn simplify<'a>(pattern: &crate::expr::Pattern<'a>) -> Pattern { // To make sure these are exhaustive, we have to "fake" a union here // TODO: use the hash or some other integer to discriminate between constructors - BitLiteral { - tag_name, union, .. - } => Ctor(union.clone(), tag_name.clone(), vec![]), - EnumLiteral { - tag_name, union, .. - } => Ctor(union.clone(), tag_name.clone(), vec![]), + BitLiteral { value, union, .. } => Ctor(union.clone(), TagId(*value as u8), vec![]), + EnumLiteral { tag_id, union, .. } => Ctor(union.clone(), TagId(*tag_id), vec![]), Underscore => Anything, Identifier(_) => Anything, RecordDestructure(destructures, _) => { + let tag_id = TagId(0); let union = Union { alternatives: vec![Ctor { name: TagName::Global("#Record".into()), + tag_id, arity: destructures.len(), }], }; @@ -68,7 +69,7 @@ fn simplify<'a>(pattern: &crate::expr::Pattern<'a>) -> Pattern { } } - Ctor(union, TagName::Global("#Record".into()), patterns) + Ctor(union, tag_id, patterns) } Shadowed(_region, _ident) => { @@ -83,14 +84,14 @@ fn simplify<'a>(pattern: &crate::expr::Pattern<'a>) -> Pattern { } AppliedTag { - tag_name, + tag_id, arguments, union, .. } => { let simplified_args: std::vec::Vec<_> = arguments.iter().map(|v| simplify(&v.0)).collect(); - Ctor(union.clone(), tag_name.clone(), simplified_args) + Ctor(union.clone(), TagId(*tag_id), simplified_args) } } } @@ -217,16 +218,16 @@ fn is_exhaustive(matrix: &PatternMatrix, n: usize) -> PatternMatrix { result } else { - let is_alt_exhaustive = |Ctor { name, arity }| { + let is_alt_exhaustive = |Ctor { arity, tag_id, .. }| { let new_matrix = matrix .iter() - .filter_map(|r| specialize_row_by_ctor(&name, arity, r)) + .filter_map(|r| specialize_row_by_ctor(tag_id, arity, r)) .collect(); let rest: Vec> = is_exhaustive(&new_matrix, arity + n - 1); let mut result = Vec::with_capacity(rest.len()); for row in rest { - result.push(recover_ctor(alts.clone(), name.clone(), arity, row)); + result.push(recover_ctor(alts.clone(), tag_id, arity, row)); } result @@ -243,27 +244,27 @@ fn is_exhaustive(matrix: &PatternMatrix, n: usize) -> PatternMatrix { } } -fn is_missing(union: Union, ctors: MutMap, ctor: &Ctor) -> Option { - let Ctor { name, arity, .. } = ctor; +fn is_missing(union: Union, ctors: MutMap, ctor: &Ctor) -> Option { + let Ctor { arity, tag_id, .. } = ctor; - if ctors.contains_key(&name) { + if ctors.contains_key(tag_id) { None } else { let anythings = std::iter::repeat(Anything).take(*arity).collect(); - Some(Pattern::Ctor(union, name.clone(), anythings)) + Some(Pattern::Ctor(union, *tag_id, anythings)) } } fn recover_ctor( union: Union, - tag_name: TagName, + tag_id: TagId, arity: usize, mut patterns: Vec, ) -> Vec { let mut rest = patterns.split_off(arity); let args = patterns; - rest.push(Ctor(union, tag_name, args)); + rest.push(Ctor(union, tag_id, args)); rest } @@ -302,18 +303,19 @@ fn to_nonredundant_rows<'a>( Guard::NoGuard => Pattern::Anything, }; + let tag_id = TagId(0); + let union = Union { alternatives: vec![Ctor { + tag_id, name: TagName::Global("#Guard".into()), arity: 2, }], }; - let tag_name = TagName::Global("#Guard".into()); - vec![Pattern::Ctor( union, - tag_name, + tag_id, vec![simplify(&loc_pat.value), guard_pattern], )] } else { @@ -350,10 +352,10 @@ fn is_useful(matrix: &PatternMatrix, vector: &Row) -> bool { match first_pattern { // keep checking rows that start with this Ctor or Anything - Ctor(_, name, args) => { + Ctor(_, id, args) => { let new_matrix: Vec<_> = matrix .iter() - .filter_map(|r| specialize_row_by_ctor(&name, args.len(), r)) + .filter_map(|r| specialize_row_by_ctor(id, args.len(), r)) .collect(); let mut new_row = Vec::new(); @@ -381,10 +383,10 @@ fn is_useful(matrix: &PatternMatrix, vector: &Row) -> bool { // All Ctors are covered, so this Anything is not needed for any // of those. But what if some of those Ctors have subpatterns // that make them less general? If so, this actually is useful! - let is_useful_alt = |Ctor { name, arity, .. }| { + let is_useful_alt = |Ctor { arity, tag_id, .. }| { let new_matrix = matrix .iter() - .filter_map(|r| specialize_row_by_ctor(&name, arity, r)) + .filter_map(|r| specialize_row_by_ctor(tag_id, arity, r)) .collect(); let mut new_row: Vec = std::iter::repeat(Anything).take(arity).collect::>(); @@ -412,15 +414,15 @@ fn is_useful(matrix: &PatternMatrix, vector: &Row) -> bool { } /// INVARIANT: (length row == N) ==> (length result == arity + N - 1) -fn specialize_row_by_ctor(tag_name: &TagName, arity: usize, row: &Row) -> Option { +fn specialize_row_by_ctor(tag_id: TagId, arity: usize, row: &Row) -> Option { let mut row = row.clone(); let head = row.pop(); let patterns = row; match head { - Some(Ctor(_,name, args)) => - if &name == tag_name { + Some(Ctor(_, id, args)) => + if id == tag_id { // TODO order! let mut new_patterns = Vec::new(); new_patterns.extend(args); @@ -497,12 +499,12 @@ type RefPatternMatrix = [Vec]; type PatternMatrix = Vec>; type Row = Vec; -fn collect_ctors(matrix: &RefPatternMatrix) -> MutMap { +fn collect_ctors(matrix: &RefPatternMatrix) -> MutMap { let mut ctors = MutMap::default(); for row in matrix { - if let Some(Ctor(union, name, _)) = row.get(row.len() - 1) { - ctors.insert(name.clone(), union.clone()); + if let Some(Ctor(union, id, _)) = row.get(row.len() - 1) { + ctors.insert(*id, union.clone()); } } diff --git a/compiler/reporting/src/error/mono.rs b/compiler/reporting/src/error/mono.rs index 12f9f54801..5011801850 100644 --- a/compiler/reporting/src/error/mono.rs +++ b/compiler/reporting/src/error/mono.rs @@ -136,9 +136,15 @@ fn pattern_to_doc<'b>( Float(f) => alloc.text(f.to_string()), Str(s) => alloc.string(s.into()), }, - Ctor(_, tag_name, args) => { + Ctor(union, tag_id, args) => { let arg_docs = args.into_iter().map(|v| pattern_to_doc(alloc, v)); + let tag = &union.alternatives[tag_id.0 as usize]; + let tag_name = tag.name.clone(); + + // We assume the alternatives are sorted. If not, this assert will trigger + debug_assert!(tag_id == tag.tag_id); + let docs = std::iter::once(alloc.tag_name(tag_name)).chain(arg_docs); alloc.intersperse(docs, alloc.space()) diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 807c26cc9a..330a4b8363 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -2209,6 +2209,40 @@ mod test_reporting { ) } + #[test] + fn patterns_remote_data_not_exhaustive() { + report_problem_as( + indoc!( + r#" + RemoteData e a : [ NotAsked, Loading, Failure e, Success a ] + + x : RemoteData Int Str + + when x is + NotAsked -> 3 + "# + ), + indoc!( + r#" + -- UNSAFE PATTERN -------------------------------------------------------------- + + This `when` does not cover all the possibilities: + + 5 ┆> when x is + 6 ┆> NotAsked -> 3 + + Other possibilities include: + + Failure _ + Loading + Success _ + + I would have to crash if I saw one of those! Add branches for them! + "# + ), + ) + } + #[test] fn patterns_int_redundant() { report_problem_as(