From b45b39af5e6a678e0d31a57644403f9a19a30df3 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 21 Jul 2020 15:25:21 +0200 Subject: [PATCH 1/2] optional fields improvements --- compiler/constrain/src/expr.rs | 6 +- compiler/constrain/src/module.rs | 1 + compiler/constrain/src/pattern.rs | 26 ++-- compiler/constrain/src/uniq.rs | 17 ++- compiler/parse/src/ast.rs | 21 +-- compiler/reporting/src/error/type.rs | 41 ++++- compiler/reporting/tests/test_reporting.rs | 169 ++++++++++++++++++++- compiler/solve/src/solve.rs | 4 + compiler/solve/tests/solve_expr.rs | 16 ++ compiler/types/src/pretty_print.rs | 4 + compiler/types/src/solved_types.rs | 2 + compiler/types/src/subs.rs | 5 + compiler/types/src/types.rs | 40 ++++- compiler/unify/src/unify.rs | 24 ++- 14 files changed, 329 insertions(+), 47 deletions(-) diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index 70259ea39b..d7441520c5 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -619,7 +619,7 @@ pub fn constrain_expr( let mut rec_field_types = SendMap::default(); let label = field.clone(); - rec_field_types.insert(label, RecordField::Required(field_type.clone())); + rec_field_types.insert(label, RecordField::Demanded(field_type.clone())); let record_type = Type::Record(rec_field_types, Box::new(ext_type)); let record_expected = Expected::NoExpectation(record_type); @@ -665,7 +665,7 @@ pub fn constrain_expr( let mut field_types = SendMap::default(); let label = field.clone(); - field_types.insert(label, RecordField::Required(field_type.clone())); + field_types.insert(label, RecordField::Demanded(field_type.clone())); let record_type = Type::Record(field_types, Box::new(ext_type)); let category = Category::Accessor(field.clone()); @@ -1031,7 +1031,7 @@ fn constrain_def(env: &Env, def: &Def, body_con: Constraint) -> Constraint { ( Closure(fn_var, _symbol, _recursive, args, boxed), Type::Function(arg_types, _), - ) if true => { + ) => { let expected = annotation_expected; let region = def.loc_expr.region; diff --git a/compiler/constrain/src/module.rs b/compiler/constrain/src/module.rs index 63914b6774..9a54c44551 100644 --- a/compiler/constrain/src/module.rs +++ b/compiler/constrain/src/module.rs @@ -222,6 +222,7 @@ fn to_type(solved_type: &SolvedType, free_vars: &mut FreeVars, var_store: &mut V let field_val = match field { Required(typ) => Required(to_type(&typ, free_vars, var_store)), Optional(typ) => Optional(to_type(&typ, free_vars, var_store)), + Demanded(typ) => Demanded(to_type(&typ, free_vars, var_store)), }; new_fields.insert(label.clone(), field_val); diff --git a/compiler/constrain/src/pattern.rs b/compiler/constrain/src/pattern.rs index 56d678ac29..a3e1d0f4a6 100644 --- a/compiler/constrain/src/pattern.rs +++ b/compiler/constrain/src/pattern.rs @@ -227,24 +227,28 @@ pub fn constrain_pattern( constrain_pattern(env, &loc_guard.value, loc_guard.region, expected, state); - RecordField::Required(pat_type) + RecordField::Demanded(pat_type) } DestructType::Optional(expr_var, loc_expr) => { + state.constraints.push(Constraint::Pattern( + region, + PatternCategory::PatternDefault, + Type::Variable(*expr_var), + PExpected::ForReason( + PReason::OptionalField, + pat_type.clone(), + loc_expr.region, + ), + )); + + state.vars.push(*expr_var); + let expr_expected = Expected::ForReason( Reason::RecordDefaultField(label.clone()), pat_type.clone(), loc_expr.region, ); - state.constraints.push(Constraint::Eq( - Type::Variable(*expr_var), - expr_expected.clone(), - Category::DefaultValue(label.clone()), - region, - )); - - state.vars.push(*expr_var); - let expr_con = constrain_expr(env, loc_expr.region, &loc_expr.value, expr_expected); state.constraints.push(expr_con); @@ -253,7 +257,7 @@ pub fn constrain_pattern( } DestructType::Required => { // No extra constraints necessary. - RecordField::Required(pat_type) + RecordField::Demanded(pat_type) } }; diff --git a/compiler/constrain/src/uniq.rs b/compiler/constrain/src/uniq.rs index 27e6e8615c..d786e585b3 100644 --- a/compiler/constrain/src/uniq.rs +++ b/compiler/constrain/src/uniq.rs @@ -259,7 +259,7 @@ fn constrain_pattern( expected, ); - RecordField::Required(pat_type) + RecordField::Demanded(pat_type) } DestructType::Optional(expr_var, loc_expr) => { let expr_expected = Expected::ForReason( @@ -292,7 +292,7 @@ fn constrain_pattern( } DestructType::Required => { // No extra constraints necessary. - RecordField::Required(pat_type) + RecordField::Demanded(pat_type) } }; @@ -1364,7 +1364,7 @@ pub fn constrain_expr( let field_uniq_type = Bool::variable(field_uniq_var); let field_type = attr_type(field_uniq_type, Type::Variable(*field_var)); - field_types.insert(field.clone(), RecordField::Required(field_type.clone())); + field_types.insert(field.clone(), RecordField::Demanded(field_type.clone())); let record_uniq_var = var_store.fresh(); let record_uniq_type = Bool::container(record_uniq_var, vec![field_uniq_var]); @@ -1421,7 +1421,7 @@ pub fn constrain_expr( let field_uniq_type = Bool::variable(field_uniq_var); let field_type = attr_type(field_uniq_type, Type::Variable(*field_var)); - field_types.insert(field.clone(), RecordField::Required(field_type.clone())); + field_types.insert(field.clone(), RecordField::Demanded(field_type.clone())); let record_uniq_var = var_store.fresh(); let record_uniq_type = Bool::container(record_uniq_var, vec![field_uniq_var]); @@ -1916,6 +1916,14 @@ fn annotation_to_attr_type( use RecordField::*; let lifted_field = match field { + Demanded(tipe) => { + let (new_vars, lifted_field) = + annotation_to_attr_type(var_store, &tipe, rigids, change_var_kind); + + vars.extend(new_vars); + + Demanded(lifted_field) + } Required(tipe) => { let (new_vars, lifted_field) = annotation_to_attr_type(var_store, &tipe, rigids, change_var_kind); @@ -2542,6 +2550,7 @@ fn fix_mutual_recursive_alias_help_help(rec_var: Variable, attribute: &Type, int let arg = match field { RecordField::Required(arg) => arg, RecordField::Optional(arg) => arg, + RecordField::Demanded(arg) => arg, }; fix_mutual_recursive_alias_help(rec_var, attribute, arg); diff --git a/compiler/parse/src/ast.rs b/compiler/parse/src/ast.rs index 0e7ba96f4b..681a214e5e 100644 --- a/compiler/parse/src/ast.rs +++ b/compiler/parse/src/ast.rs @@ -409,6 +409,7 @@ impl<'a> Pattern<'a> { /// different locations/whitespace pub fn equivalent(&self, other: &Self) -> bool { use Pattern::*; + match (self, other) { (Identifier(x), Identifier(y)) => x == y, (GlobalTag(x), GlobalTag(y)) => x == y, @@ -428,21 +429,13 @@ impl<'a> Pattern<'a> { (RequiredField(x, inner_x), RequiredField(y, inner_y)) => { x == y && inner_x.value.equivalent(&inner_y.value) } - (OptionalField(x, _inner_x), OptionalField(y, _inner_y)) => { + (OptionalField(x, _), OptionalField(y, _)) + | (OptionalField(x, _), Identifier(y)) + | (Identifier(x), OptionalField(y, _)) => { + // optional record fields can be annotated as: + // { x, y } : { x : Int, y ? Bool } + // { x, y ? False } = rec x == y - // TODO - // - // We can give an annotation like so - // - // { x, y } : { x : Int, y : Bool } - // { x, y } = rec - // - // But what about: - // - // { x, y ? False } : { x : Int, y ? Bool } - // { x, y ? False } = rec - // - // inner_x.value.equivalent(&inner_y.value) } (Nested(x), Nested(y)) => x.equivalent(y), diff --git a/compiler/reporting/src/error/type.rs b/compiler/reporting/src/error/type.rs index 71f38302c5..5c8d059768 100644 --- a/compiler/reporting/src/error/type.rs +++ b/compiler/reporting/src/error/type.rs @@ -953,6 +953,7 @@ fn to_pattern_report<'b>( } PExpected::ForReason(reason, expected_type, region) => match reason { + PReason::OptionalField => unreachable!("this will never be reached I think"), PReason::TypedArg { opt_name, index } => { let name = match opt_name { Some(n) => alloc.symbol_unqualified(n), @@ -1097,6 +1098,7 @@ fn add_pattern_category<'b>( Record => alloc.reflow(" record values of type:"), EmptyRecord => alloc.reflow(" an empty record:"), PatternGuard => alloc.reflow(" a pattern guard of type:"), + PatternDefault => alloc.reflow(" an optional field of type:"), Set => alloc.reflow(" sets of type:"), Map => alloc.reflow(" maps of type:"), Ctor(tag_name) => alloc.concat(vec![ @@ -1151,6 +1153,7 @@ pub enum Problem { TagTypo(TagName, Vec), TagsMissing(Vec), BadRigidVar(Lowercase, ErrorType), + OptionalRequiredMismatch(Lowercase), } fn problems_to_hint<'b>( @@ -1245,6 +1248,7 @@ fn to_comparison<'b>( } } +#[derive(Debug)] pub enum Status { Similar, // the structure is the same or e.g. record fields are different Different(Vec), // e.g. found Bool, expected Int @@ -1344,6 +1348,9 @@ pub fn to_doc<'b>( RecordField::Required(v) => { RecordField::Required(to_doc(alloc, Parens::Unnecessary, v)) } + RecordField::Demanded(v) => { + RecordField::Demanded(to_doc(alloc, Parens::Unnecessary, v)) + } }, ) }) @@ -1643,6 +1650,7 @@ fn diff_record<'b>( match t1 { RecordField::Optional(_) => RecordField::Optional(diff.left), RecordField::Required(_) => RecordField::Required(diff.left), + RecordField::Demanded(_) => RecordField::Demanded(diff.left), }, ), right: ( @@ -1651,11 +1659,31 @@ fn diff_record<'b>( match t2 { RecordField::Optional(_) => RecordField::Optional(diff.right), RecordField::Required(_) => RecordField::Required(diff.right), + RecordField::Demanded(_) => RecordField::Demanded(diff.right), }, ), - status: diff.status, + status: { + match (&t1, &t2) { + (RecordField::Demanded(_), RecordField::Optional(_)) + | (RecordField::Optional(_), RecordField::Demanded(_)) => match diff.status + { + Status::Similar => { + Status::Different(vec![Problem::OptionalRequiredMismatch( + field.clone(), + )]) + } + Status::Different(mut problems) => { + problems.push(Problem::OptionalRequiredMismatch(field.clone())); + + Status::Different(problems) + } + }, + _ => diff.status, + } + }, } }; + let to_unknown_docs = |(field, tipe): &(Lowercase, RecordField)| { ( field.clone(), @@ -2006,6 +2034,9 @@ mod report_text { let entry_to_doc = |(field_name, field_type): (RocDocBuilder<'b>, RecordField>)| { match field_type { + RecordField::Demanded(field) => { + field_name.append(alloc.text(" : ")).append(field) + } RecordField::Required(field) => { field_name.append(alloc.text(" : ")).append(field) } @@ -2414,5 +2445,13 @@ fn type_problem_to_pretty<'b>( alloc.stack(vec![hint1, hint2]) } }, + OptionalRequiredMismatch(field) => alloc.hint().append(alloc.concat(vec![ + alloc.reflow("To extract the "), + alloc.record_field(field), + alloc.reflow( + " field it must be non-optional, but the type says this field is optional. ", + ), + alloc.reflow("Learn more about optional fields at TODO."), + ])), } } diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 27c12ef16c..dea96d1c69 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -3524,10 +3524,172 @@ mod test_reporting { ) } + #[test] + fn optional_record_invalid_let_binding() { + report_problem_as( + indoc!( + r#" + \rec -> + { x, y } : { x : Int, y ? Bool } + { x, y } = rec + + { x, y } + "# + ), + indoc!( + r#" + -- TYPE MISMATCH --------------------------------------------------------------- + + Something is off with the body of this definition: + + 2 ┆ { x, y } : { x : Int, y ? Bool } + ┆ ^^^^^^^^^^^^^^^^^^^^^ + + The body is a value of type: + + { x : Int, y : Bool } + + But the type annotation says it should be: + + { x : Int, y ? Bool } + + Hint: To extract the `.y` field it must be non-optional, but the type + says this field is optional. Learn more about optional fields at TODO. + "# + ), + ) + } + + #[test] + fn optional_record_invalid_function() { + report_problem_as( + indoc!( + r#" + f : { x : Int, y ? Int } -> Int + f = \{ x, y } -> x + y + + f + "# + ), + indoc!( + r#" + -- TYPE MISMATCH --------------------------------------------------------------- + + The 1st argument to `f` is weird: + + 2 ┆ f = \{ x, y } -> x + y + ┆ ^^^^^^^^ + + The argument is a pattern that matches record values of type: + + { x : Int, y : Int } + + But the annotation on `f` says the 1st argument should be: + + { x : Int, y ? Int } + + Hint: To extract the `.y` field it must be non-optional, but the type + says this field is optional. Learn more about optional fields at TODO. + "# + ), + ) + } + + #[test] + fn optional_record_invalid_when() { + report_problem_as( + indoc!( + r#" + f : { x : Int, y ? Int } -> Int + f = \r -> + when r is + { x, y } -> x + y + + f + "# + ), + indoc!( + r#" + -- TYPE MISMATCH --------------------------------------------------------------- + + The 1st pattern in this `when` is causing a mismatch: + + 4 ┆ { x, y } -> x + y + ┆ ^^^^^^^^ + + The first pattern is trying to match record values of type: + + { x : Int, y : Int } + + But the expression between `when` and `is` has the type: + + { x : Int, y ? Int } + + Hint: To extract the `.y` field it must be non-optional, but the type + says this field is optional. Learn more about optional fields at TODO. + "# + ), + ) + } + + #[test] + fn optional_record_invalid_access() { + report_problem_as( + indoc!( + r#" + f : { x : Int, y ? Int } -> Int + f = \r -> r.y + + f + "# + ), + indoc!( + r#" + -- TYPE MISMATCH --------------------------------------------------------------- + + This expression is used in an unexpected way: + + 2 ┆ f = \r -> r.y + ┆ ^^^ + + This `r` value is a: + + { x : Int, y ? Int } + + But you are trying to use it as: + + { x : Int, y : Int } + + Hint: To extract the `.y` field it must be non-optional, but the type + says this field is optional. Learn more about optional fields at TODO. + "# + ), + ) + } + + // TODO field accessors give a parse error at the moment + // #[test] + // fn optional_record_invalid_accessor() { + // report_problem_as( + // indoc!( + // r#" + // f : { x : Int, y ? Int } -> Int + // f = \r -> .y r + // + // f + // "# + // ), + // indoc!( + // r#" + // -- TYPE MISMATCH --------------------------------------------------------------- + // + // "# + // ), + // ) + // } + #[test] fn guard_mismatch_with_annotation() { - // TODO improve this message - // The problem is the guard, and the message should point to that report_problem_as( indoc!( r#" @@ -3563,9 +3725,6 @@ mod test_reporting { #[test] fn optional_field_mismatch_with_annotation() { - // TODO improve this message - // The problem is the pattern match default - // the message should point to that. report_problem_as( indoc!( r#" diff --git a/compiler/solve/src/solve.rs b/compiler/solve/src/solve.rs index c3976275c1..fbe5ab6f28 100644 --- a/compiler/solve/src/solve.rs +++ b/compiler/solve/src/solve.rs @@ -620,6 +620,7 @@ fn type_to_variable( let field_var = match field_type { Required(typ) => Required(type_to_variable(subs, rank, pools, cached, typ)), Optional(typ) => Optional(type_to_variable(subs, rank, pools, cached, typ)), + Demanded(typ) => Demanded(type_to_variable(subs, rank, pools, cached, typ)), }; field_vars.insert(field.clone(), field_var); @@ -1257,6 +1258,9 @@ fn deep_copy_var_help( use RecordField::*; let new_field = match field { + Demanded(var) => { + Demanded(deep_copy_var_help(subs, max_rank, pools, var)) + } Required(var) => { Required(deep_copy_var_help(subs, max_rank, pools, var)) } diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index db4d27b26f..bd8d09f3ce 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -2669,4 +2669,20 @@ mod solve_expr { "{ x : Num a, y ? Num a }* -> Num a", ); } + + #[test] + fn optional_field_let_with_signature() { + infer_eq_without_problem( + indoc!( + r#" + \rec -> + { x, y } : { x : Int, y ? Bool }* + { x, y ? False } = rec + + { x, y } + "# + ), + "{ x : Int, y ? Bool }* -> { x : Int, y : Bool }", + ); + } } diff --git a/compiler/types/src/pretty_print.rs b/compiler/types/src/pretty_print.rs index 6a6f1b321a..b4aad222ae 100644 --- a/compiler/types/src/pretty_print.rs +++ b/compiler/types/src/pretty_print.rs @@ -416,6 +416,10 @@ fn write_flat_type(env: &Env, flat_type: FlatType, subs: &Subs, buf: &mut String buf.push_str(" : "); var } + Demanded(var) => { + buf.push_str(" : "); + var + } }; write_content( diff --git a/compiler/types/src/solved_types.rs b/compiler/types/src/solved_types.rs index 2b621805d5..f48f6e482a 100644 --- a/compiler/types/src/solved_types.rs +++ b/compiler/types/src/solved_types.rs @@ -129,6 +129,7 @@ impl SolvedType { let solved_type = match field { Optional(typ) => RecordField::Optional(Self::from_type(solved_subs, typ)), Required(typ) => RecordField::Required(Self::from_type(solved_subs, typ)), + Demanded(typ) => RecordField::Demanded(Self::from_type(solved_subs, typ)), }; solved_fields.push((label.clone(), solved_type)); @@ -246,6 +247,7 @@ impl SolvedType { let solved_type = match field { Optional(var) => Optional(Self::from_var(subs, var)), Required(var) => Required(Self::from_var(subs, var)), + Demanded(var) => Demanded(Self::from_var(subs, var)), }; new_fields.push((label, solved_type)); diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index 96931dfa5b..19bab67ca4 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -616,6 +616,7 @@ fn occurs( once(&ext_var).chain(vars_by_field.values().map(|field| match field { RecordField::Optional(var) => var, RecordField::Required(var) => var, + RecordField::Demanded(var) => var, })); short_circuit(subs, root_var, &new_seen, it) } @@ -743,6 +744,9 @@ fn explicit_substitute( Required(var) => { Required(explicit_substitute(subs, from, to, *var, seen)) } + Demanded(var) => { + Demanded(explicit_substitute(subs, from, to, *var, seen)) + } }; } subs.set_content(in_var, Structure(Record(vars_by_field, new_ext_var))); @@ -1017,6 +1021,7 @@ fn flat_type_to_err_type( let err_type = match field_var { Optional(var) => Optional(var_to_err_type(subs, state, var)), Required(var) => Required(var_to_err_type(subs, state, var)), + Demanded(var) => Demanded(var_to_err_type(subs, state, var)), }; err_fields.insert(field, err_type); diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 0fab169b0b..f1159fa38d 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -13,10 +13,20 @@ pub const TYPE_NUM: &str = "Num"; pub const TYPE_INTEGER: &str = "Integer"; pub const TYPE_FLOATINGPOINT: &str = "FloatingPoint"; +/// +/// Intuitively +/// +/// - Demanded: only introduced by pattern matches, e.g. { x } -> +/// Cannot unify with an Optional field, but can unify with a Required field +/// - Required: introduced by record literals and type annotations. +/// Can unify with Optional and Demanded +/// - Optional: introduced by pattern matches and annotations. +/// Can unify with Required, but not with Demanded #[derive(PartialEq, Eq, Clone)] pub enum RecordField { Optional(T), Required(T), + Demanded(T), } impl Copy for RecordField {} @@ -26,8 +36,9 @@ impl fmt::Debug for RecordField { use RecordField::*; match self { - Optional(typ) => typ.fmt(f), - Required(typ) => typ.fmt(f), + Optional(typ) => write!(f, "Optional({:?})", typ), + Required(typ) => write!(f, "Required({:?})", typ), + Demanded(typ) => write!(f, "Demanded({:?})", typ), } } } @@ -39,6 +50,7 @@ impl RecordField { match self { Optional(t) => t, Required(t) => t, + Demanded(t) => t, } } @@ -50,6 +62,7 @@ impl RecordField { match self { Optional(t) => Optional(f(t)), Required(t) => Required(f(t)), + Demanded(t) => Demanded(f(t)), } } } @@ -61,6 +74,7 @@ impl RecordField { match self { Optional(typ) => typ.substitute(substitutions), Required(typ) => typ.substitute(substitutions), + Demanded(typ) => typ.substitute(substitutions), } } @@ -70,6 +84,7 @@ impl RecordField { match self { Optional(typ) => typ.substitute_alias(rep_symbol, actual), Required(typ) => typ.substitute_alias(rep_symbol, actual), + Demanded(typ) => typ.substitute_alias(rep_symbol, actual), } } @@ -85,6 +100,7 @@ impl RecordField { match self { Optional(typ) => typ.instantiate_aliases(region, aliases, var_store, introduced), Required(typ) => typ.instantiate_aliases(region, aliases, var_store, introduced), + Demanded(typ) => typ.instantiate_aliases(region, aliases, var_store, introduced), } } @@ -94,6 +110,7 @@ impl RecordField { match self { Optional(typ) => typ.contains_symbol(rep_symbol), Required(typ) => typ.contains_symbol(rep_symbol), + Demanded(typ) => typ.contains_symbol(rep_symbol), } } pub fn contains_variable(&self, rep_variable: Variable) -> bool { @@ -102,6 +119,7 @@ impl RecordField { match self { Optional(typ) => typ.contains_variable(rep_variable), Required(typ) => typ.contains_variable(rep_variable), + Demanded(typ) => typ.contains_variable(rep_variable), } } } @@ -187,7 +205,11 @@ impl fmt::Debug for Type { let mut any_written_yet = false; for (label, field_type) in fields { - write!(f, "{:?} : {:?}", label, field_type)?; + match field_type { + RecordField::Optional(_) => write!(f, "{:?} ? {:?}", label, field_type)?, + RecordField::Required(_) => write!(f, "{:?} : {:?}", label, field_type)?, + RecordField::Demanded(_) => write!(f, "{:?} : {:?}", label, field_type)?, + } if any_written_yet { write!(f, ", ")?; @@ -714,6 +736,7 @@ fn symbols_help(tipe: &Type, accum: &mut ImSet) { match field { Optional(arg) => symbols_help(arg, accum), Required(arg) => symbols_help(arg, accum), + Demanded(arg) => symbols_help(arg, accum), } }); } @@ -756,6 +779,7 @@ fn variables_help(tipe: &Type, accum: &mut ImSet) { match field { Optional(x) => variables_help(x, accum), Required(x) => variables_help(x, accum), + Demanded(x) => variables_help(x, accum), }; } variables_help(ext, accum); @@ -843,6 +867,7 @@ pub enum PReason { index: Index, }, PatternGuard, + OptionalField, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -922,6 +947,7 @@ pub enum PatternCategory { Record, EmptyRecord, PatternGuard, + PatternDefault, Set, Map, Ctor(TagName), @@ -1103,6 +1129,10 @@ fn write_error_type_help( buf.push_str(" : "); content } + Demanded(content) => { + buf.push_str(" : "); + content + } }; write_error_type_help(home, interns, content, buf, Parens::Unnecessary); @@ -1243,6 +1273,10 @@ fn write_debug_error_type_help(error_type: ErrorType, buf: &mut String, parens: buf.push_str(" : "); content } + Demanded(content) => { + buf.push_str(" : "); + content + } }; write_debug_error_type_help(content, buf, Parens::Unnecessary); diff --git a/compiler/unify/src/unify.rs b/compiler/unify/src/unify.rs index 78c84c1a1f..72aa41c6d2 100644 --- a/compiler/unify/src/unify.rs +++ b/compiler/unify/src/unify.rs @@ -327,13 +327,24 @@ fn unify_shared_fields( let num_shared_fields = shared_fields.len(); for (name, (actual, expected)) in shared_fields { - let problems = unify_pool(subs, pool, actual.into_inner(), expected.into_inner()); + let local_problems = unify_pool(subs, pool, actual.into_inner(), expected.into_inner()); - if problems.is_empty() { + if local_problems.is_empty() { use RecordField::*; - // If either field is Required, both are Required. + // Unification of optional fields + // + // Demanded does not unify with Optional + // Unifying Required with Demanded => Demanded + // Unifying Optional with Required => Required + // Unifying X with X => X let actual = match (actual, expected) { + (Demanded(_), Optional(_)) | (Optional(_), Demanded(_)) => { + continue; + } + (Demanded(val), Required(_)) + | (Required(val), Demanded(_)) + | (Demanded(val), Demanded(_)) => Demanded(val), (Required(val), Required(_)) => Required(val), (Required(val), Optional(_)) => Required(val), (Optional(val), Required(_)) => Required(val), @@ -614,13 +625,14 @@ fn unify_shared_tags( } } -fn has_no_required_fields<'a, I, T>(fields: &mut I) -> bool +fn has_only_optional_fields<'a, I, T>(fields: &mut I) -> bool where I: Iterator>, T: 'a, { fields.all(|field| match field { RecordField::Required(_) => false, + RecordField::Demanded(_) => false, RecordField::Optional(_) => true, }) } @@ -638,11 +650,11 @@ fn unify_flat_type( match (left, right) { (EmptyRecord, EmptyRecord) => merge(subs, ctx, Structure(left.clone())), - (Record(fields, ext), EmptyRecord) if has_no_required_fields(&mut fields.values()) => { + (Record(fields, ext), EmptyRecord) if has_only_optional_fields(&mut fields.values()) => { unify_pool(subs, pool, *ext, ctx.second) } - (EmptyRecord, Record(fields, ext)) if has_no_required_fields(&mut fields.values()) => { + (EmptyRecord, Record(fields, ext)) if has_only_optional_fields(&mut fields.values()) => { unify_pool(subs, pool, ctx.first, *ext) } From a82cc7f28f5f960c601d17e7213d6f089afcc0e6 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 21 Jul 2020 16:20:03 +0200 Subject: [PATCH 2/2] fix weird region in error messages --- compiler/constrain/src/expr.rs | 2 +- compiler/reporting/tests/test_reporting.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/constrain/src/expr.rs b/compiler/constrain/src/expr.rs index d7441520c5..2737a1e956 100644 --- a/compiler/constrain/src/expr.rs +++ b/compiler/constrain/src/expr.rs @@ -1019,7 +1019,7 @@ fn constrain_def(env: &Env, def: &Def, body_con: Constraint) -> Constraint { expr_type, annotation_expected.clone(), Category::Storage, - annotation.region, + Region::span_across(&annotation.region, &def.loc_expr.region), )); // when a def is annotated, and it's body is a closure, treat this diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index dea96d1c69..6ac20824ad 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -3542,8 +3542,8 @@ mod test_reporting { Something is off with the body of this definition: - 2 ┆ { x, y } : { x : Int, y ? Bool } - ┆ ^^^^^^^^^^^^^^^^^^^^^ + 2 ┆> { x, y } : { x : Int, y ? Bool } + 3 ┆> { x, y } = rec The body is a value of type: