use tag id instead of name in exhaustiveness checking

This commit is contained in:
Folkert 2020-04-21 16:24:43 +02:00
parent 0b670baad4
commit f0aa598ff9
5 changed files with 89 additions and 40 deletions

View file

@ -9,7 +9,7 @@ use roc_module::symbol::Symbol;
use crate::expr::specialize_equality; use crate::expr::specialize_equality;
use crate::layout::Builtin; use crate::layout::Builtin;
use crate::layout::Layout; use crate::layout::Layout;
use crate::pattern::{Ctor, Union}; use crate::pattern::{Ctor, TagId, Union};
/// COMPILE CASES /// COMPILE CASES
@ -383,6 +383,7 @@ fn test_at_path<'a>(selected_path: &Path, branch: Branch<'a>, all_tests: &mut Ve
RecordDestructure(destructs, _) => { RecordDestructure(destructs, _) => {
let union = Union { let union = Union {
alternatives: vec![Ctor { alternatives: vec![Ctor {
tag_id: TagId(0),
name: TagName::Global("#Record".into()), name: TagName::Global("#Record".into()),
arity: destructs.len(), arity: destructs.len(),
}], }],

View file

@ -1,5 +1,5 @@
use crate::layout::{Builtin, Layout}; use crate::layout::{Builtin, Layout};
use crate::pattern::{Ctor, Guard}; use crate::pattern::{Ctor, Guard, TagId};
use bumpalo::collections::Vec; use bumpalo::collections::Vec;
use bumpalo::Bump; use bumpalo::Bump;
use roc_can; use roc_can;
@ -1497,6 +1497,7 @@ fn from_can_pattern<'a>(
tag_name: tag_name.clone(), tag_name: tag_name.clone(),
union: Union { union: Union {
alternatives: vec![Ctor { alternatives: vec![Ctor {
tag_id: TagId(0),
name: tag_name.clone(), name: tag_name.clone(),
arity: 0, arity: 0,
}], }],
@ -1508,11 +1509,13 @@ fn from_can_pattern<'a>(
union: Union { union: Union {
alternatives: vec![ alternatives: vec![
Ctor { Ctor {
name: ttrue, tag_id: TagId(0),
name: ffalse,
arity: 0, arity: 0,
}, },
Ctor { Ctor {
name: ffalse, tag_id: TagId(1),
name: ttrue,
arity: 0, arity: 0,
}, },
], ],
@ -1525,8 +1528,9 @@ fn from_can_pattern<'a>(
.expect("tag must be in its own type"); .expect("tag must be in its own type");
let mut ctors = std::vec::Vec::with_capacity(tag_names.len()); 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 { ctors.push(Ctor {
tag_id: TagId(i as u8),
name: tag_name.clone(), name: tag_name.clone(),
arity: 0, arity: 0,
}) })
@ -1545,6 +1549,7 @@ fn from_can_pattern<'a>(
Unwrapped(field_layouts) => { Unwrapped(field_layouts) => {
let union = crate::pattern::Union { let union = crate::pattern::Union {
alternatives: vec![Ctor { alternatives: vec![Ctor {
tag_id: TagId(0),
name: tag_name.clone(), name: tag_name.clone(),
arity: field_layouts.len(), arity: field_layouts.len(),
}], }],
@ -1567,8 +1572,9 @@ fn from_can_pattern<'a>(
} }
Wrapped(tags) => { Wrapped(tags) => {
let mut ctors = std::vec::Vec::with_capacity(tags.len()); 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 { ctors.push(Ctor {
tag_id: TagId(i as u8),
name: tag_name.clone(), name: tag_name.clone(),
// don't include tag discriminant in arity // don't include tag discriminant in arity
arity: args.len() - 1, arity: args.len() - 1,

View file

@ -9,10 +9,13 @@ pub struct Union {
pub alternatives: Vec<Ctor>, pub alternatives: Vec<Ctor>,
} }
#[derive(Clone, Debug, PartialEq, Eq, Hash, Copy)]
pub struct TagId(pub u8);
#[derive(Clone, Debug, PartialEq, Eq, Hash)] #[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct Ctor { pub struct Ctor {
pub name: TagName, pub name: TagName,
// pub tag_id: u8, pub tag_id: TagId,
pub arity: usize, pub arity: usize,
} }
@ -20,7 +23,7 @@ pub struct Ctor {
pub enum Pattern { pub enum Pattern {
Anything, Anything,
Literal(Literal), Literal(Literal),
Ctor(Union, TagName, std::vec::Vec<Pattern>), Ctor(Union, TagId, std::vec::Vec<Pattern>),
} }
#[derive(Clone, Debug, PartialEq)] #[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 // 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 // TODO: use the hash or some other integer to discriminate between constructors
BitLiteral { BitLiteral { value, union, .. } => Ctor(union.clone(), TagId(*value as u8), vec![]),
tag_name, union, .. EnumLiteral { tag_id, union, .. } => Ctor(union.clone(), TagId(*tag_id), vec![]),
} => Ctor(union.clone(), tag_name.clone(), vec![]),
EnumLiteral {
tag_name, union, ..
} => Ctor(union.clone(), tag_name.clone(), vec![]),
Underscore => Anything, Underscore => Anything,
Identifier(_) => Anything, Identifier(_) => Anything,
RecordDestructure(destructures, _) => { RecordDestructure(destructures, _) => {
let tag_id = TagId(0);
let union = Union { let union = Union {
alternatives: vec![Ctor { alternatives: vec![Ctor {
name: TagName::Global("#Record".into()), name: TagName::Global("#Record".into()),
tag_id,
arity: destructures.len(), 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) => { Shadowed(_region, _ident) => {
@ -83,14 +84,14 @@ fn simplify<'a>(pattern: &crate::expr::Pattern<'a>) -> Pattern {
} }
AppliedTag { AppliedTag {
tag_name, tag_id,
arguments, arguments,
union, union,
.. ..
} => { } => {
let simplified_args: std::vec::Vec<_> = let simplified_args: std::vec::Vec<_> =
arguments.iter().map(|v| simplify(&v.0)).collect(); 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 result
} else { } else {
let is_alt_exhaustive = |Ctor { name, arity }| { let is_alt_exhaustive = |Ctor { arity, tag_id, .. }| {
let new_matrix = matrix let new_matrix = matrix
.iter() .iter()
.filter_map(|r| specialize_row_by_ctor(&name, arity, r)) .filter_map(|r| specialize_row_by_ctor(tag_id, arity, r))
.collect(); .collect();
let rest: Vec<Vec<Pattern>> = is_exhaustive(&new_matrix, arity + n - 1); let rest: Vec<Vec<Pattern>> = is_exhaustive(&new_matrix, arity + n - 1);
let mut result = Vec::with_capacity(rest.len()); let mut result = Vec::with_capacity(rest.len());
for row in rest { 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 result
@ -243,27 +244,27 @@ fn is_exhaustive(matrix: &PatternMatrix, n: usize) -> PatternMatrix {
} }
} }
fn is_missing<T>(union: Union, ctors: MutMap<TagName, T>, ctor: &Ctor) -> Option<Pattern> { fn is_missing<T>(union: Union, ctors: MutMap<TagId, T>, ctor: &Ctor) -> Option<Pattern> {
let Ctor { name, arity, .. } = ctor; let Ctor { arity, tag_id, .. } = ctor;
if ctors.contains_key(&name) { if ctors.contains_key(tag_id) {
None None
} else { } else {
let anythings = std::iter::repeat(Anything).take(*arity).collect(); 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( fn recover_ctor(
union: Union, union: Union,
tag_name: TagName, tag_id: TagId,
arity: usize, arity: usize,
mut patterns: Vec<Pattern>, mut patterns: Vec<Pattern>,
) -> Vec<Pattern> { ) -> Vec<Pattern> {
let mut rest = patterns.split_off(arity); let mut rest = patterns.split_off(arity);
let args = patterns; let args = patterns;
rest.push(Ctor(union, tag_name, args)); rest.push(Ctor(union, tag_id, args));
rest rest
} }
@ -302,18 +303,19 @@ fn to_nonredundant_rows<'a>(
Guard::NoGuard => Pattern::Anything, Guard::NoGuard => Pattern::Anything,
}; };
let tag_id = TagId(0);
let union = Union { let union = Union {
alternatives: vec![Ctor { alternatives: vec![Ctor {
tag_id,
name: TagName::Global("#Guard".into()), name: TagName::Global("#Guard".into()),
arity: 2, arity: 2,
}], }],
}; };
let tag_name = TagName::Global("#Guard".into());
vec![Pattern::Ctor( vec![Pattern::Ctor(
union, union,
tag_name, tag_id,
vec![simplify(&loc_pat.value), guard_pattern], vec![simplify(&loc_pat.value), guard_pattern],
)] )]
} else { } else {
@ -350,10 +352,10 @@ fn is_useful(matrix: &PatternMatrix, vector: &Row) -> bool {
match first_pattern { match first_pattern {
// keep checking rows that start with this Ctor or Anything // keep checking rows that start with this Ctor or Anything
Ctor(_, name, args) => { Ctor(_, id, args) => {
let new_matrix: Vec<_> = matrix let new_matrix: Vec<_> = matrix
.iter() .iter()
.filter_map(|r| specialize_row_by_ctor(&name, args.len(), r)) .filter_map(|r| specialize_row_by_ctor(id, args.len(), r))
.collect(); .collect();
let mut new_row = Vec::new(); 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 // All Ctors are covered, so this Anything is not needed for any
// of those. But what if some of those Ctors have subpatterns // of those. But what if some of those Ctors have subpatterns
// that make them less general? If so, this actually is useful! // 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 let new_matrix = matrix
.iter() .iter()
.filter_map(|r| specialize_row_by_ctor(&name, arity, r)) .filter_map(|r| specialize_row_by_ctor(tag_id, arity, r))
.collect(); .collect();
let mut new_row: Vec<Pattern> = let mut new_row: Vec<Pattern> =
std::iter::repeat(Anything).take(arity).collect::<Vec<_>>(); std::iter::repeat(Anything).take(arity).collect::<Vec<_>>();
@ -412,15 +414,15 @@ fn is_useful(matrix: &PatternMatrix, vector: &Row) -> bool {
} }
/// INVARIANT: (length row == N) ==> (length result == arity + N - 1) /// INVARIANT: (length row == N) ==> (length result == arity + N - 1)
fn specialize_row_by_ctor(tag_name: &TagName, arity: usize, row: &Row) -> Option<Row> { fn specialize_row_by_ctor(tag_id: TagId, arity: usize, row: &Row) -> Option<Row> {
let mut row = row.clone(); let mut row = row.clone();
let head = row.pop(); let head = row.pop();
let patterns = row; let patterns = row;
match head { match head {
Some(Ctor(_,name, args)) => Some(Ctor(_, id, args)) =>
if &name == tag_name { if id == tag_id {
// TODO order! // TODO order!
let mut new_patterns = Vec::new(); let mut new_patterns = Vec::new();
new_patterns.extend(args); new_patterns.extend(args);
@ -497,12 +499,12 @@ type RefPatternMatrix = [Vec<Pattern>];
type PatternMatrix = Vec<Vec<Pattern>>; type PatternMatrix = Vec<Vec<Pattern>>;
type Row = Vec<Pattern>; type Row = Vec<Pattern>;
fn collect_ctors(matrix: &RefPatternMatrix) -> MutMap<TagName, Union> { fn collect_ctors(matrix: &RefPatternMatrix) -> MutMap<TagId, Union> {
let mut ctors = MutMap::default(); let mut ctors = MutMap::default();
for row in matrix { for row in matrix {
if let Some(Ctor(union, name, _)) = row.get(row.len() - 1) { if let Some(Ctor(union, id, _)) = row.get(row.len() - 1) {
ctors.insert(name.clone(), union.clone()); ctors.insert(*id, union.clone());
} }
} }

View file

@ -136,9 +136,15 @@ fn pattern_to_doc<'b>(
Float(f) => alloc.text(f.to_string()), Float(f) => alloc.text(f.to_string()),
Str(s) => alloc.string(s.into()), 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 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); let docs = std::iter::once(alloc.tag_name(tag_name)).chain(arg_docs);
alloc.intersperse(docs, alloc.space()) alloc.intersperse(docs, alloc.space())

View file

@ -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] #[test]
fn patterns_int_redundant() { fn patterns_int_redundant() {
report_problem_as( report_problem_as(