From c9953129cb3d2ec11a900e48daf7d8e3ebc25768 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 8 Nov 2022 13:19:43 -0600 Subject: [PATCH] Remove problem storage in Type::Erroneous --- crates/ast/src/lang/core/types.rs | 4 +- crates/compiler/can/src/annotation.rs | 42 +++++++------- crates/compiler/can/src/def.rs | 4 +- crates/compiler/problem/src/can.rs | 2 + crates/compiler/solve/src/solve.rs | 6 +- crates/compiler/types/src/subs.rs | 9 +-- crates/compiler/types/src/types.rs | 64 +++++++++++++--------- crates/reporting/src/error/canonicalize.rs | 48 ++++++++++++++++ crates/reporting/src/error/type.rs | 6 -- 9 files changed, 114 insertions(+), 71 deletions(-) diff --git a/crates/ast/src/lang/core/types.rs b/crates/ast/src/lang/core/types.rs index 25ce0cc0bf..6576ed9264 100644 --- a/crates/ast/src/lang/core/types.rs +++ b/crates/ast/src/lang/core/types.rs @@ -744,7 +744,7 @@ fn to_type_apply<'a>( Err(problem) => { env.problem(roc_problem::can::Problem::RuntimeError(problem)); - return TypeApply::Erroneous(Problem::UnrecognizedIdent(ident.into())); + return TypeApply::Erroneous(Problem::CanonicalizationProblem); } } } else { @@ -755,7 +755,7 @@ fn to_type_apply<'a>( // it was imported but it doesn't expose this ident. env.problem(roc_problem::can::Problem::RuntimeError(problem)); - return TypeApply::Erroneous(Problem::UnrecognizedIdent((*ident).into())); + return TypeApply::Erroneous(Problem::CanonicalizationProblem); } } }; diff --git a/crates/compiler/can/src/annotation.rs b/crates/compiler/can/src/annotation.rs index e8932184b0..ef59cb49cf 100644 --- a/crates/compiler/can/src/annotation.rs +++ b/crates/compiler/can/src/annotation.rs @@ -391,8 +391,7 @@ pub(crate) fn make_apply_symbol( Err(problem) => { env.problem(roc_problem::can::Problem::RuntimeError(problem)); - let ident: Ident = (*ident).into(); - Err(Type::Erroneous(Problem::UnrecognizedIdent(ident))) + Err(Type::Erroneous) } } } else { @@ -405,7 +404,7 @@ pub(crate) fn make_apply_symbol( // A failed import should have already been reported through // roc_can::env::Env::qualified_lookup's checks - Err(Type::Erroneous(Problem::SolvedTypeError)) + Err(Type::Erroneous) } } } @@ -626,14 +625,16 @@ fn can_annotation_help( // use a known alias if alias.type_variables.len() != args.len() { - let error = Type::Erroneous(Problem::BadTypeArguments { - symbol, - region, - alias_needs: alias.type_variables.len() as u8, - type_got: args.len() as u8, - alias_kind: alias.kind, - }); - return error; + env.problem(roc_problem::can::Problem::BadType( + Problem::BadTypeArguments { + symbol, + region, + alias_needs: alias.type_variables.len() as u8, + type_got: args.len() as u8, + alias_kind: alias.kind, + }, + )); + return Type::Erroneous; } let mut type_var_to_arg = Vec::new(); @@ -717,15 +718,13 @@ fn can_annotation_help( Ok(symbol) => symbol, Err((shadowed_symbol, shadow, _new_symbol)) => { - let problem = Problem::Shadowed(shadowed_symbol.region, shadow.clone()); - env.problem(roc_problem::can::Problem::Shadowing { original_region: shadowed_symbol.region, shadow, kind: ShadowKind::Variable, }); - return Type::Erroneous(problem); + return Type::Erroneous; } }; @@ -992,7 +991,7 @@ fn can_annotation_help( region: Region::across_all(clauses.iter().map(|clause| &clause.region)), }); - Type::Erroneous(Problem::CanonicalizationProblem) + Type::Erroneous } Malformed(string) => { malformed(env, region, string); @@ -1044,13 +1043,13 @@ fn canonicalize_has_clause( && !scope.abilities_store.is_ability(symbol) { env.problem(roc_problem::can::Problem::HasClauseIsNotAbility { region }); - return Err(Type::Erroneous(Problem::HasClauseIsNotAbility(region))); + return Err(Type::Erroneous); } symbol } _ => { env.problem(roc_problem::can::Problem::HasClauseIsNotAbility { region }); - return Err(Type::Erroneous(Problem::HasClauseIsNotAbility(region))); + return Err(Type::Erroneous); } }; @@ -1070,10 +1069,7 @@ fn canonicalize_has_clause( shadow: shadow.clone(), kind: ShadowKind::Variable, }); - return Err(Type::Erroneous(Problem::Shadowed( - shadowing.first_seen(), - shadow, - ))); + return Err(Type::Erroneous); } let var = var_store.fresh(); @@ -1099,13 +1095,13 @@ fn can_extension_type<'a>( // Include erroneous types so that we don't overreport errors. matches!( typ, - Type::EmptyRec | Type::Record(..) | Type::Variable(..) | Type::Erroneous(..) + Type::EmptyRec | Type::Record(..) | Type::Variable(..) | Type::Erroneous ) } fn valid_tag_ext_type(typ: &Type) -> bool { matches!( typ, - Type::EmptyTagUnion | Type::TagUnion(..) | Type::Variable(..) | Type::Erroneous(..) + Type::EmptyTagUnion | Type::TagUnion(..) | Type::Variable(..) | Type::Erroneous ) } diff --git a/crates/compiler/can/src/def.rs b/crates/compiler/can/src/def.rs index 7310464bb0..74335770b0 100644 --- a/crates/compiler/can/src/def.rs +++ b/crates/compiler/can/src/def.rs @@ -2819,6 +2819,7 @@ fn correct_mutual_recursive_type_alias<'a>( alias_type.instantiate_aliases( alias_region, &can_instantiate_symbol, + &mut |problem| env.problems.push(Problem::BadType(problem)), var_store, &mut new_lambda_sets, &mut new_infer_ext_vars, @@ -3109,8 +3110,7 @@ fn mark_cyclic_alias<'a>( others: Vec, report: bool, ) { - let problem = roc_types::types::Problem::CyclicAlias(symbol, region, others.clone()); - *typ = Type::Erroneous(problem); + *typ = Type::Erroneous; if report { let problem = Problem::CyclicAlias(symbol, region, others, alias_kind); diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index 5a37dc9890..bf7a11df54 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -188,6 +188,7 @@ pub enum Problem { MultipleListRestPattern { region: Region, }, + BadType(roc_types::types::Problem), } impl Problem { @@ -330,6 +331,7 @@ impl Problem { | Problem::RuntimeError(RuntimeError::ExposedButNotDefined(_)) | Problem::RuntimeError(RuntimeError::NoImplementationNamed { .. }) | Problem::ExposedButNotDefined(_) => None, + Problem::BadType(..) => None, } } } diff --git a/crates/compiler/solve/src/solve.rs b/crates/compiler/solve/src/solve.rs index 4779bfede6..15a2543d20 100644 --- a/crates/compiler/solve/src/solve.rs +++ b/crates/compiler/solve/src/solve.rs @@ -2980,11 +2980,7 @@ fn type_to_variable<'a>( result } Erroneous => { - // TODO: remove `Erroneous`, `Error` can always be used, and type problems known at - // this point can be reported during canonicalization. - let problem_index = - SubsIndex::push_new(&mut subs.problems, types.get_problem(&typ).clone()); - let content = Content::Structure(FlatType::Erroneous(problem_index)); + let content = Content::Error; register_with_known_var(subs, destination, rank, pools, content) } diff --git a/crates/compiler/types/src/subs.rs b/crates/compiler/types/src/subs.rs index 709cf3e643..9b5f8a25d3 100644 --- a/crates/compiler/types/src/subs.rs +++ b/crates/compiler/types/src/subs.rs @@ -21,7 +21,7 @@ roc_error_macros::assert_sizeof_all!(FlatType, 3 * 8); roc_error_macros::assert_sizeof_all!(UnionTags, 12); roc_error_macros::assert_sizeof_all!(RecordFields, 2 * 8); -roc_error_macros::assert_sizeof_aarch64!(Problem, 6 * 8); +roc_error_macros::assert_sizeof_aarch64!(Problem, 5 * 8); roc_error_macros::assert_sizeof_wasm!(Problem, 32); roc_error_macros::assert_sizeof_default!(Problem, 6 * 8); @@ -4040,12 +4040,7 @@ fn flat_type_to_err_type( } } - Erroneous(problem_index) => { - let problem = subs.problems[problem_index.index as usize].clone(); - state.problems.push(problem); - - ErrorType::Error - } + Erroneous(_) => ErrorType::Error, } } diff --git a/crates/compiler/types/src/types.rs b/crates/compiler/types/src/types.rs index 417747f64e..9355f01f72 100644 --- a/crates/compiler/types/src/types.rs +++ b/crates/compiler/types/src/types.rs @@ -173,6 +173,7 @@ impl RecordField { &mut self, region: Region, aliases: &'a F, + push_problem: &mut impl FnMut(Problem), var_store: &mut VarStore, new_lambda_sets: &mut ImSet, new_infer_ext_vars: &mut ImSet, @@ -182,6 +183,7 @@ impl RecordField { self.as_inner_mut().instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_sets, new_infer_ext_vars, @@ -228,6 +230,7 @@ impl LambdaSet { &mut self, region: Region, aliases: &'a F, + push_problem: &mut impl FnMut(Problem), var_store: &mut VarStore, new_lambda_sets: &mut ImSet, new_infer_ext_vars: &mut ImSet, @@ -237,6 +240,7 @@ impl LambdaSet { self.0.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_sets, new_infer_ext_vars, @@ -1332,7 +1336,7 @@ pub enum Type { Variable(Variable), RangedNumber(NumericRange), /// A type error, which will code gen to a runtime error - Erroneous(Problem), + Erroneous, } /// A lambda set under an arrow in a ability member signature. For example, in @@ -1428,7 +1432,7 @@ impl Clone for Type { Self::Apply(arg0, arg1, arg2) => Self::Apply(*arg0, arg1.clone(), *arg2), Self::Variable(arg0) => Self::Variable(*arg0), Self::RangedNumber(arg1) => Self::RangedNumber(*arg1), - Self::Erroneous(arg0) => Self::Erroneous(arg0.clone()), + Self::Erroneous => Self::Erroneous, } } } @@ -1544,13 +1548,7 @@ impl fmt::Debug for Type { write!(f, ")") } - Type::Erroneous(problem) => { - write!(f, "Erroneous(")?; - - problem.fmt(f)?; - - write!(f, ")") - } + Type::Erroneous => write!(f, "Erroneous"), Type::DelayedAlias(AliasCommon { symbol, type_arguments, @@ -1910,7 +1908,7 @@ impl Type { ); } - EmptyRec | EmptyTagUnion | Erroneous(_) => {} + EmptyRec | EmptyTagUnion | Erroneous => {} } } } @@ -2040,7 +2038,7 @@ impl Type { ); } - EmptyRec | EmptyTagUnion | Erroneous(_) => {} + EmptyRec | EmptyTagUnion | Erroneous => {} } } } @@ -2143,7 +2141,7 @@ impl Type { } RangedNumber(_) => Ok(()), UnspecializedLambdaSet { .. } => Ok(()), - EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous(_) | Variable(_) => Ok(()), + EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous | Variable(_) => Ok(()), } } @@ -2205,7 +2203,7 @@ impl Type { UnspecializedLambdaSet { unspecialized: Uls(_, sym, _), } => *sym == rep_symbol, - EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous(_) | Variable(_) => false, + EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous | Variable(_) => false, } } @@ -2261,7 +2259,7 @@ impl Type { .iter() .any(|arg| arg.value.contains_variable(rep_variable)), RangedNumber(_) => false, - EmptyRec | EmptyTagUnion | Erroneous(_) => false, + EmptyRec | EmptyTagUnion | Erroneous => false, } } @@ -2295,6 +2293,7 @@ impl Type { &mut self, region: Region, aliases: &'a F, + push_problem: &mut impl FnMut(Problem), var_store: &mut VarStore, new_lambda_set_variables: &mut ImSet, new_infer_ext_vars: &mut ImSet, @@ -2309,6 +2308,7 @@ impl Type { arg.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2317,6 +2317,7 @@ impl Type { closure.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2324,6 +2325,7 @@ impl Type { ret.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2334,6 +2336,7 @@ impl Type { ext.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2346,6 +2349,7 @@ impl Type { x.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2357,6 +2361,7 @@ impl Type { ext.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2368,6 +2373,7 @@ impl Type { x.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2378,6 +2384,7 @@ impl Type { ext.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2400,6 +2407,7 @@ impl Type { t.value.typ.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2416,6 +2424,7 @@ impl Type { arg.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2426,6 +2435,7 @@ impl Type { arg.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2435,6 +2445,7 @@ impl Type { actual_type.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2450,6 +2461,7 @@ impl Type { arg.typ.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2460,6 +2472,7 @@ impl Type { arg.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2469,6 +2482,7 @@ impl Type { actual_type.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2522,13 +2536,14 @@ impl Type { *self = alias; } else { if args.len() != alias.type_variables.len() { - *self = Type::Erroneous(Problem::BadTypeArguments { + push_problem(Problem::BadTypeArguments { symbol: *symbol, region, type_got: args.len() as u8, alias_needs: alias.type_variables.len() as u8, alias_kind: AliasKind::Structural, }); + *self = Type::Erroneous; return; } @@ -2555,6 +2570,7 @@ impl Type { filler.value.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2591,6 +2607,7 @@ impl Type { actual.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2631,6 +2648,7 @@ impl Type { x.value.instantiate_aliases( region, aliases, + push_problem, var_store, new_lambda_set_variables, new_infer_ext_vars, @@ -2640,7 +2658,7 @@ impl Type { } RangedNumber(_) => {} UnspecializedLambdaSet { .. } => {} - EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous(_) | Variable(_) => {} + EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous | Variable(_) => {} } } @@ -2770,16 +2788,13 @@ fn symbols_help(initial: &Type) -> Vec { output.push(*symbol); stack.extend(args.iter().map(|t| &t.value)); } - Erroneous(Problem::CyclicAlias(alias, _, _)) => { - output.push(*alias); - } RangedNumber(_) => {} UnspecializedLambdaSet { unspecialized: Uls(_, _sym, _), } => { // ignore the member symbol because unspecialized lambda sets are internal-only } - EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous(_) | Variable(_) => {} + EmptyRec | EmptyTagUnion | ClosureTag { .. } | Erroneous | Variable(_) => {} } } @@ -2793,7 +2808,7 @@ fn variables_help(tipe: &Type, accum: &mut ImSet) { use Type::*; match tipe { - EmptyRec | EmptyTagUnion | Erroneous(_) => (), + EmptyRec | EmptyTagUnion | Erroneous => (), Variable(v) => { accum.insert(*v); @@ -2923,7 +2938,7 @@ fn variables_help_detailed(tipe: &Type, accum: &mut VariableDetail) { use Type::*; match tipe { - EmptyRec | EmptyTagUnion | Erroneous(_) => (), + EmptyRec | EmptyTagUnion | Erroneous => (), Variable(v) => { accum.type_variables.insert(*v); @@ -3319,8 +3334,6 @@ impl Alias { pub enum Problem { CanonicalizationProblem, CircularType(Symbol, Box, Region), - CyclicAlias(Symbol, Region, Vec), - UnrecognizedIdent(Ident), Shadowed(Region, Loc), BadTypeArguments { symbol: Symbol, @@ -3330,7 +3343,6 @@ pub enum Problem { alias_kind: AliasKind, }, InvalidModule, - SolvedTypeError, HasClauseIsNotAbility(Region), } @@ -4151,7 +4163,7 @@ fn instantiate_lambda_sets_as_unspecialized( } Type::Variable(_) => {} Type::RangedNumber(_) => {} - Type::Erroneous(_) => {} + Type::Erroneous => {} } } } diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index 4664d4eec4..8479a05d25 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -1037,6 +1037,54 @@ pub fn can_problem<'b>( title = "MULTIPLE LIST REST PATTERNS".to_string(); severity = Severity::RuntimeError; } + Problem::BadType(type_problem) => { + use roc_types::types::Problem::*; + match type_problem { + BadTypeArguments { + symbol, + region, + type_got, + alias_needs, + alias_kind, + } => { + let needed_arguments = if alias_needs == 1 { + alloc.reflow("1 type argument") + } else { + alloc + .text(alias_needs.to_string()) + .append(alloc.reflow(" type arguments")) + }; + + let found_arguments = alloc.text(type_got.to_string()); + + doc = alloc.stack([ + alloc.concat([ + alloc.reflow("The "), + alloc.symbol_unqualified(symbol), + alloc.reflow(" "), + alloc.reflow(alias_kind.as_str()), + alloc.reflow(" expects "), + needed_arguments, + alloc.reflow(", but it got "), + found_arguments, + alloc.reflow(" instead:"), + ]), + alloc.region(lines.convert_region(region)), + alloc.reflow("Are there missing parentheses?"), + ]); + + title = if type_got > alias_needs { + "TOO MANY TYPE ARGUMENTS".to_string() + } else { + "TOO FEW TYPE ARGUMENTS".to_string() + }; + + severity = Severity::RuntimeError; + } + + other => panic!("unhandled bad type: {:?}", other), + } + } }; Report { diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index 3fd01de7da..65b4d3ff60 100644 --- a/crates/reporting/src/error/type.rs +++ b/crates/reporting/src/error/type.rs @@ -126,12 +126,6 @@ pub fn type_problem<'b>( report(title, doc, filename) } - SolvedTypeError => None, // Don't re-report cascading errors - see https://github.com/roc-lang/roc/pull/1711 - - // We'll also report these as a canonicalization problem, no need to re-report them. - CyclicAlias(..) => None, - UnrecognizedIdent(..) => None, - other => panic!("unhandled bad type: {:?}", other), } }