From e2b30e5301668a52838970bd05cec7603ab3ec6a Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Wed, 2 Nov 2022 16:05:13 -0500 Subject: [PATCH] Constrain + solve crash --- crates/compiler/can/src/copy.rs | 5 +- crates/compiler/can/src/expr.rs | 70 +++++++++++++++++-- crates/compiler/can/src/module.rs | 11 ++- crates/compiler/can/src/traverse.rs | 4 +- crates/compiler/constrain/src/expr.rs | 14 ++-- crates/compiler/mono/src/ir.rs | 2 +- crates/compiler/problem/src/can.rs | 10 ++- crates/compiler/solve/src/solve.rs | 21 ------ .../compiler/test_derive/src/pretty_print.rs | 2 +- crates/compiler/types/src/types.rs | 24 +++---- crates/reporting/src/error/canonicalize.rs | 31 +++++++- crates/reporting/src/error/type.rs | 36 ++++++++++ crates/reporting/tests/test_reporting.rs | 24 +++++-- 13 files changed, 194 insertions(+), 60 deletions(-) diff --git a/crates/compiler/can/src/copy.rs b/crates/compiler/can/src/copy.rs index 27ffdc9758..83e8ab9d71 100644 --- a/crates/compiler/can/src/copy.rs +++ b/crates/compiler/can/src/copy.rs @@ -376,7 +376,10 @@ fn deep_copy_expr_help(env: &mut C, copied: &mut Vec, expr *called_via, ) } - Crash => Crash, + Crash { msg, ret_var } => Crash { + msg: Box::new(msg.map(|m| go_help!(m))), + ret_var: sub!(*ret_var), + }, RunLowLevel { op, args, ret_var } => RunLowLevel { op: *op, args: args diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index d3f500c8a8..bf3101df22 100644 --- a/crates/compiler/can/src/expr.rs +++ b/crates/compiler/can/src/expr.rs @@ -167,7 +167,10 @@ pub enum Expr { EmptyRecord, /// The "crash" keyword - Crash, + Crash { + msg: Box>, + ret_var: Variable, + }, /// Look up exactly one field on a record, e.g. (expr).foo. Access { @@ -312,7 +315,7 @@ impl Expr { } Self::Expect { .. } => Category::Expect, Self::ExpectFx { .. } => Category::Expect, - Self::Crash => Category::Crash, + Self::Crash { .. } => Category::Crash, Self::Dbg { .. } => Category::Expect, @@ -788,6 +791,47 @@ pub fn canonicalize_expr<'a>( } } } + } else if let ast::Expr::Crash = loc_fn.value { + // We treat crash specially, since crashing must be applied with one argument. + + debug_assert!(!args.is_empty()); + + let mut args = Vec::new(); + let mut output = Output::default(); + + for loc_arg in loc_args.iter() { + let (arg_expr, arg_out) = + canonicalize_expr(env, var_store, scope, loc_arg.region, &loc_arg.value); + + args.push(arg_expr); + output.references.union_mut(&arg_out.references); + } + + let crash = if args.len() > 1 { + let args_region = Region::span_across( + &loc_args.first().unwrap().region, + &loc_args.last().unwrap().region, + ); + env.problem(Problem::OverAppliedCrash { + region: args_region, + }); + // Still crash, just with our own message, and drop the references. + Crash { + msg: Box::new(Loc::at( + region, + Expr::Str(String::from("hit a crash!").into_boxed_str()), + )), + ret_var: var_store.fresh(), + } + } else { + let msg = args.pop().unwrap(); + Crash { + msg: Box::new(msg), + ret_var: var_store.fresh(), + } + }; + + (crash, output) } else { // Canonicalize the function expression and its arguments let (fn_expr, fn_expr_output) = @@ -878,7 +922,22 @@ pub fn canonicalize_expr<'a>( (RuntimeError(problem), Output::default()) } - ast::Expr::Crash => (Crash, Output::default()), + ast::Expr::Crash => { + // Naked crashes aren't allowed; we'll admit this with our own message, but yield an + // error. + env.problem(Problem::UnappliedCrash { region }); + + ( + Crash { + msg: Box::new(Loc::at( + region, + Expr::Str(String::from("hit a crash!").into_boxed_str()), + )), + ret_var: var_store.fresh(), + }, + Output::default(), + ) + } ast::Expr::Defs(loc_defs, loc_ret) => { // The body expression gets a new scope for canonicalization, scope.inner_scope(|inner_scope| { @@ -1726,7 +1785,7 @@ pub fn inline_calls(var_store: &mut VarStore, expr: Expr) -> Expr { | other @ TypedHole { .. } | other @ ForeignCall { .. } | other @ OpaqueWrapFunction(_) - | other @ Crash => other, + | other @ Crash { .. } => other, List { elem_var, @@ -2834,8 +2893,7 @@ fn get_lookup_symbols(expr: &Expr) -> Vec { | Expr::EmptyRecord | Expr::TypedHole(_) | Expr::RuntimeError(_) - | Expr::OpaqueWrapFunction(_) - | Expr::Crash => {} + | Expr::OpaqueWrapFunction(_) => {} } } diff --git a/crates/compiler/can/src/module.rs b/crates/compiler/can/src/module.rs index 9dcfdf9eae..998e3e0f06 100644 --- a/crates/compiler/can/src/module.rs +++ b/crates/compiler/can/src/module.rs @@ -981,6 +981,14 @@ fn fix_values_captured_in_closure_expr( ); } + Crash { msg, ret_var: _ } => { + fix_values_captured_in_closure_expr( + &mut msg.value, + no_capture_symbols, + closure_captures, + ); + } + Closure(ClosureData { captured_symbols, name, @@ -1056,8 +1064,7 @@ fn fix_values_captured_in_closure_expr( | TypedHole { .. } | RuntimeError(_) | ZeroArgumentTag { .. } - | Accessor { .. } - | Crash => {} + | Accessor { .. } => {} List { loc_elems, .. } => { for elem in loc_elems.iter_mut() { diff --git a/crates/compiler/can/src/traverse.rs b/crates/compiler/can/src/traverse.rs index 0c8f392b17..cefb62a24f 100644 --- a/crates/compiler/can/src/traverse.rs +++ b/crates/compiler/can/src/traverse.rs @@ -185,7 +185,6 @@ pub fn walk_expr(visitor: &mut V, expr: &Expr, var: Variable) { } Expr::Var(..) => { /* terminal */ } Expr::AbilityMember(..) => { /* terminal */ } - Expr::Crash => { /* terminal */ } Expr::If { cond_var, branches, @@ -204,6 +203,9 @@ pub fn walk_expr(visitor: &mut V, expr: &Expr, var: Variable) { let (fn_var, loc_fn, _closure_var, _ret_var) = &**f; walk_call(visitor, *fn_var, loc_fn, args); } + Expr::Crash { msg, .. } => { + visitor.visit_expr(&msg.value, msg.region, Variable::STR); + } Expr::RunLowLevel { op: _, args, diff --git a/crates/compiler/constrain/src/expr.rs b/crates/compiler/constrain/src/expr.rs index aa1c9f2adc..8c84540fc6 100644 --- a/crates/compiler/constrain/src/expr.rs +++ b/crates/compiler/constrain/src/expr.rs @@ -482,11 +482,17 @@ pub fn constrain_expr( let and_constraint = constraints.and_constraint(and_cons); constraints.exists(vars, and_constraint) } - Expr::Crash => { - let crash_type_index = constraints.push_type(Type::Crash); - let expected_index = constraints.push_expected_type(expected); + Expr::Crash { msg, ret_var } => { + let expected_msg = Expected::ForReason(Reason::CrashArg, str_type(), msg.region); + let expected_ret = constraints.push_expected_type(expected); - constraints.equal_types(crash_type_index, expected_index, Category::Crash, region) + let msg_is_str = constrain_expr(constraints, env, msg.region, &msg.value, expected_msg); + let magic = + constraints.equal_types_var(*ret_var, expected_ret, Category::Crash, region); + + let and = constraints.and_constraint([msg_is_str, magic]); + + constraints.exists([*ret_var], and) } Var(symbol, variable) => { // Save the expectation in the variable, then lookup the symbol's type in the environment diff --git a/crates/compiler/mono/src/ir.rs b/crates/compiler/mono/src/ir.rs index 7a1fe7c7df..9fa8043b1c 100644 --- a/crates/compiler/mono/src/ir.rs +++ b/crates/compiler/mono/src/ir.rs @@ -5562,7 +5562,7 @@ pub fn with_hole<'a>( } TypedHole(_) => Stmt::RuntimeError("Hit a blank"), RuntimeError(e) => Stmt::RuntimeError(env.arena.alloc(e.runtime_message())), - Crash => todo!(), + Crash { .. } => todo!(), } } diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index c49efaab6e..2fd5a9483e 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -195,6 +195,12 @@ pub enum Problem { type_got: u8, alias_kind: AliasKind, }, + UnappliedCrash { + region: Region, + }, + OverAppliedCrash { + region: Region, + }, } impl Problem { @@ -325,7 +331,9 @@ impl Problem { } | Problem::MultipleListRestPattern { region } | Problem::BadTypeArguments { region, .. } - | Problem::UnnecessaryOutputWildcard { region } => Some(*region), + | Problem::UnnecessaryOutputWildcard { region } + | Problem::OverAppliedCrash { region } + | Problem::UnappliedCrash { region } => Some(*region), Problem::RuntimeError(RuntimeError::CircularDef(cycle_entries)) | Problem::BadRecursion(cycle_entries) => { cycle_entries.first().map(|entry| entry.expr_region) diff --git a/crates/compiler/solve/src/solve.rs b/crates/compiler/solve/src/solve.rs index 737153c26d..631f41aef5 100644 --- a/crates/compiler/solve/src/solve.rs +++ b/crates/compiler/solve/src/solve.rs @@ -2961,27 +2961,6 @@ fn type_to_variable<'a>( register_with_known_var(subs, destination, rank, pools, content) } - Crash => { - let magic_return = subs.fresh(Descriptor { - content: Content::FlexVar(None), - rank, - mark: Mark::NONE, - copy: OptVariable::NONE, - }); - let magic_lambda_set = subs.fresh(Descriptor { - content: Content::FlexVar(None), - rank, - mark: Mark::NONE, - copy: OptVariable::NONE, - }); - let magic_crash = Content::Structure(FlatType::Func( - Subs::STR_SLICE, - magic_lambda_set, - magic_return, - )); - - register_with_known_var(subs, destination, rank, pools, magic_crash) - } }; } diff --git a/crates/compiler/test_derive/src/pretty_print.rs b/crates/compiler/test_derive/src/pretty_print.rs index dc94aec3dd..1b0a3e96d1 100644 --- a/crates/compiler/test_derive/src/pretty_print.rs +++ b/crates/compiler/test_derive/src/pretty_print.rs @@ -279,7 +279,7 @@ fn expr<'a>(c: &Ctx, p: EPrec, f: &'a Arena<'a>, e: &'a Expr) -> DocBuilder<'a, ) .group() ), - Crash => f.text("crash"), + Crash { .. } => todo!(), ZeroArgumentTag { .. } => todo!(), OpaqueRef { .. } => todo!(), Dbg { .. } => todo!(), diff --git a/crates/compiler/types/src/types.rs b/crates/compiler/types/src/types.rs index 40bcf5ed37..f8335e6ca8 100644 --- a/crates/compiler/types/src/types.rs +++ b/crates/compiler/types/src/types.rs @@ -994,7 +994,6 @@ impl Types { self.set_type_tag(index, TypeTag::RangedNumber(*range), Slice::default()) } Type::Error => self.set_type_tag(index, TypeTag::Error, Slice::default()), - Type::Crash => todo!(), } } @@ -1669,7 +1668,6 @@ pub enum Type { Apply(Symbol, Vec>, Region), Variable(Variable), RangedNumber(NumericRange), - Crash, // Str -> a /// A type error, which will code gen to a runtime error Error, } @@ -1712,7 +1710,6 @@ impl Clone for Type { match self { Self::EmptyRec => Self::EmptyRec, Self::EmptyTagUnion => Self::EmptyTagUnion, - Self::Crash => Self::Crash, Self::Function(arg0, arg1, arg2) => { Self::Function(arg0.clone(), arg1.clone(), arg2.clone()) } @@ -2081,7 +2078,6 @@ impl fmt::Debug for Type { Type::RangedNumber(range_vars) => { write!(f, "Ranged({:?})", range_vars) } - Type::Crash => write!(f, "Crash"), Type::UnspecializedLambdaSet { unspecialized } => { write!(f, "{:?}", unspecialized) } @@ -2245,7 +2241,7 @@ impl Type { ); } - EmptyRec | EmptyTagUnion | Crash | Error => {} + EmptyRec | EmptyTagUnion | Error => {} } } } @@ -2375,7 +2371,7 @@ impl Type { ); } - EmptyRec | EmptyTagUnion | Crash | Error => {} + EmptyRec | EmptyTagUnion | Error => {} } } } @@ -2478,7 +2474,7 @@ impl Type { } RangedNumber(_) => Ok(()), UnspecializedLambdaSet { .. } => Ok(()), - EmptyRec | EmptyTagUnion | ClosureTag { .. } | Error | Variable(_) | Crash => Ok(()), + EmptyRec | EmptyTagUnion | ClosureTag { .. } | Error | Variable(_) => Ok(()), } } @@ -2540,7 +2536,7 @@ impl Type { UnspecializedLambdaSet { unspecialized: Uls(_, sym, _), } => *sym == rep_symbol, - EmptyRec | EmptyTagUnion | ClosureTag { .. } | Error | Variable(_) | Crash => false, + EmptyRec | EmptyTagUnion | ClosureTag { .. } | Error | Variable(_) => false, } } @@ -2596,7 +2592,7 @@ impl Type { .iter() .any(|arg| arg.value.contains_variable(rep_variable)), RangedNumber(_) => false, - EmptyRec | EmptyTagUnion | Error | Crash => false, + EmptyRec | EmptyTagUnion | Error => false, } } @@ -2970,7 +2966,7 @@ impl Type { } RangedNumber(_) => {} UnspecializedLambdaSet { .. } => {} - EmptyRec | EmptyTagUnion | ClosureTag { .. } | Error | Variable(_) | Crash => {} + EmptyRec | EmptyTagUnion | ClosureTag { .. } | Error | Variable(_) => {} } } @@ -3106,7 +3102,7 @@ fn symbols_help(initial: &Type) -> Vec { } => { // ignore the member symbol because unspecialized lambda sets are internal-only } - EmptyRec | EmptyTagUnion | ClosureTag { .. } | Error | Variable(_) | Crash => {} + EmptyRec | EmptyTagUnion | ClosureTag { .. } | Error | Variable(_) => {} } } @@ -3222,7 +3218,7 @@ fn variables_help(tipe: &Type, accum: &mut ImSet) { } variables_help(actual, accum); } - RangedNumber(_) | Crash => {} + RangedNumber(_) => {} Apply(_, args, _) => { for x in args { variables_help(&x.value, accum); @@ -3250,7 +3246,7 @@ fn variables_help_detailed(tipe: &Type, accum: &mut VariableDetail) { use Type::*; match tipe { - EmptyRec | EmptyTagUnion | Error | Crash => (), + EmptyRec | EmptyTagUnion | Error => (), Variable(v) => { accum.type_variables.insert(*v); @@ -3490,6 +3486,7 @@ pub enum Reason { member_name: Symbol, def_region: Region, }, + CrashArg, } #[derive(PartialEq, Eq, Debug, Clone)] @@ -4377,7 +4374,6 @@ fn instantiate_lambda_sets_as_unspecialized( match typ { Type::EmptyRec => {} Type::EmptyTagUnion => {} - Type::Crash => {} Type::Function(args, lambda_set, ret) => { debug_assert!( matches!(**lambda_set, Type::Variable(..)), diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index b5b26e4125..cf96c09fcc 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -1086,7 +1086,36 @@ pub fn can_problem<'b>( } else { "TOO FEW TYPE ARGUMENTS".to_string() }; - + severity = Severity::RuntimeError; + } + Problem::UnappliedCrash { region } => { + doc = alloc.stack([ + alloc.concat([ + alloc.reflow("This "), alloc.keyword("crash"), alloc.reflow(" doesn't have a message given to it:") + ]), + alloc.region(lines.convert_region(region)), + alloc.concat([ + alloc.keyword("crash"), alloc.reflow(" must be passed a message to crash with at the exact place it's used. "), + alloc.keyword("crash"), alloc.reflow(" can't be used as a value that's passed around, like functions can be - it must be applied immediately!"), + ]) + ]); + title = "UNAPPLIED CRASH".to_string(); + severity = Severity::RuntimeError; + } + Problem::OverAppliedCrash { region } => { + doc = alloc.stack([ + alloc.concat([ + alloc.reflow("This "), + alloc.keyword("crash"), + alloc.reflow(" has too many values given to it:"), + ]), + alloc.region(lines.convert_region(region)), + alloc.concat([ + alloc.keyword("crash"), + alloc.reflow(" must be given exacly one message to crash with."), + ]), + ]); + title = "OVERAPPLIED CRASH".to_string(); severity = Severity::RuntimeError; } }; diff --git a/crates/reporting/src/error/type.rs b/crates/reporting/src/error/type.rs index df39747f53..097d8faddc 100644 --- a/crates/reporting/src/error/type.rs +++ b/crates/reporting/src/error/type.rs @@ -1375,6 +1375,42 @@ fn to_expr_report<'b>( } } + Reason::CrashArg => { + let this_is = alloc.reflow("The value is"); + + let wanted = alloc.concat([ + alloc.reflow("But I can only "), + alloc.keyword("crash"), + alloc.reflow(" with messages of type"), + ]); + + let details = None; + + let lines = [ + alloc + .reflow("This value passed to ") + .append(alloc.keyword("crash")) + .append(alloc.reflow(" is not a string:")), + alloc.region(lines.convert_region(region)), + type_comparison( + alloc, + found, + expected_type, + ExpectationContext::WhenCondition, + add_category(alloc, this_is, &category), + wanted, + details, + ), + ]; + + Report { + filename, + title: "TYPE MISMATCH".to_string(), + doc: alloc.stack(lines), + severity: Severity::RuntimeError, + } + } + Reason::LowLevelOpArg { op, arg_index } => { panic!( "Compiler bug: argument #{} to low-level operation {:?} was the wrong type!", diff --git a/crates/reporting/tests/test_reporting.rs b/crates/reporting/tests/test_reporting.rs index 556a1fc29e..8eef315236 100644 --- a/crates/reporting/tests/test_reporting.rs +++ b/crates/reporting/tests/test_reporting.rs @@ -12431,16 +12431,16 @@ All branches in an `if` must have the same type! @r###" ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─ - This 1st argument to this function has an unexpected type: + This value passed to `crash` is not a string: 4│ crash {} ^^ - The argument is a record of type: + The value is a record of type: {} - But this function needs its 1st argument to be: + But I can only `crash` with messages of type Str "### @@ -12454,6 +12454,16 @@ All branches in an `if` must have the same type! "# ), @r###" + ── UNAPPLIED CRASH ─────────────────────────────────────── /code/proj/Main.roc ─ + + This `crash` doesn't have a message given to it: + + 4│ crash + ^^^^^ + + `crash` must be passed a message to crash with at the exact place it's + used. `crash` can't be used as a value that's passed around, like + functions can be - it must be applied immediately! "### ); @@ -12465,14 +12475,14 @@ All branches in an `if` must have the same type! "# ), @r###" - ── TOO MANY ARGS ───────────────────────────────────────── /code/proj/Main.roc ─ + ── OVERAPPLIED CRASH ───────────────────────────────────── /code/proj/Main.roc ─ - This function expects 1 argument, but it got 2 instead: + This `crash` has too many values given to it: 4│ crash "" "" - ^^^^^ + ^^^^^ - Are there any missing commas? Or missing parentheses? + `crash` must be given exacly one message to crash with. "### ); }