From facce68b25890580b70610531bd1eac63cb63495 Mon Sep 17 00:00:00 2001 From: Louis Gesbert Date: Thu, 2 May 2024 16:20:54 +0200 Subject: [PATCH] Make refactored runtime error messages clearer mostly reverting to the ones the interpreter was printing ; for the case of divisions, we choose to point to the denominator instead of the operator as it's where the only possible error (division by zero) comes from. --- compiler/lcalc/to_ocaml.ml | 5 ++-- compiler/shared_ast/interpreter.ml | 29 +++++++++++-------- runtimes/jsoo/runtime.ml | 13 ++++++--- runtimes/ocaml/runtime.ml | 14 +++++---- .../arithmetic/bad/division_by_zero.catala_en | 21 ++++++++------ .../date/bad/uncomparable_duration.catala_en | 16 +++++----- tests/default/bad/conflict.catala_en | 2 +- tests/default/bad/empty.catala_en | 3 +- tests/default/bad/empty_with_rules.catala_en | 3 +- tests/exception/bad/two_exceptions.catala_en | 3 +- tests/func/bad/bad_func.catala_en | 3 +- tests/modules/good/output/mod_def.ml | 2 +- tests/scope/bad/scope.catala_en | 3 +- 13 files changed, 70 insertions(+), 47 deletions(-) diff --git a/compiler/lcalc/to_ocaml.ml b/compiler/lcalc/to_ocaml.ml index 796d9fa1..9b7a8e8b 100644 --- a/compiler/lcalc/to_ocaml.ml +++ b/compiler/lcalc/to_ocaml.ml @@ -437,10 +437,11 @@ let rec format_expr (ctx : decl_ctx) (fmt : Format.formatter) (e : 'm expr) : Format.fprintf fmt "@[%s@ %t%a@]" (Operator.name op) (fun ppf -> match op with - | Map2 | Div_int_int | Div_rat_rat | Div_mon_mon | Div_mon_rat - | Div_dur_dur | Lt_dur_dur | Lte_dur_dur | Gt_dur_dur | Gte_dur_dur + | Map2 | Lt_dur_dur | Lte_dur_dur | Gt_dur_dur | Gte_dur_dur | Eq_dur_dur -> Format.fprintf ppf "%a@ " format_pos pos + | Div_int_int | Div_rat_rat | Div_mon_mon | Div_mon_rat | Div_dur_dur -> + Format.fprintf ppf "%a@ " format_pos (Expr.pos (List.nth args 1)) | _ -> ()) (Format.pp_print_list ~pp_sep:(fun fmt () -> Format.fprintf fmt "@ ") diff --git a/compiler/shared_ast/interpreter.ml b/compiler/shared_ast/interpreter.ml index 25427daf..0c1a2ccc 100644 --- a/compiler/shared_ast/interpreter.ml +++ b/compiler/shared_ast/interpreter.ml @@ -114,7 +114,12 @@ let rec evaluate_operator lang args = let pos = Expr.mark_pos m in - let rpos = Expr.pos_to_runtime opos in + let rpos () = Expr.pos_to_runtime opos in + let div_pos () = + (* Division by 0 errors point to their 2nd operand *) + Expr.pos_to_runtime + @@ match args with _ :: denom :: _ -> Expr.pos denom | _ -> opos + in let err () = Message.error ~extra_pos: @@ -287,15 +292,15 @@ let rec evaluate_operator | Mult_dur_int, [(ELit (LDuration x), _); (ELit (LInt y), _)] -> ELit (LDuration (o_mult_dur_int x y)) | Div_int_int, [(ELit (LInt x), _); (ELit (LInt y), _)] -> - ELit (LRat (o_div_int_int rpos x y)) + ELit (LRat (o_div_int_int (div_pos ()) x y)) | Div_rat_rat, [(ELit (LRat x), _); (ELit (LRat y), _)] -> - ELit (LRat (o_div_rat_rat rpos x y)) + ELit (LRat (o_div_rat_rat (div_pos ()) x y)) | Div_mon_mon, [(ELit (LMoney x), _); (ELit (LMoney y), _)] -> - ELit (LRat (o_div_mon_mon rpos x y)) + ELit (LRat (o_div_mon_mon (div_pos ()) x y)) | Div_mon_rat, [(ELit (LMoney x), _); (ELit (LRat y), _)] -> - ELit (LMoney (o_div_mon_rat rpos x y)) + ELit (LMoney (o_div_mon_rat (div_pos ()) x y)) | Div_dur_dur, [(ELit (LDuration x), _); (ELit (LDuration y), _)] -> - ELit (LRat (o_div_dur_dur rpos x y)) + ELit (LRat (o_div_dur_dur (div_pos ()) x y)) | Lt_int_int, [(ELit (LInt x), _); (ELit (LInt y), _)] -> ELit (LBool (o_lt_int_int x y)) | Lt_rat_rat, [(ELit (LRat x), _); (ELit (LRat y), _)] -> @@ -305,7 +310,7 @@ let rec evaluate_operator | Lt_dat_dat, [(ELit (LDate x), _); (ELit (LDate y), _)] -> ELit (LBool (o_lt_dat_dat x y)) | Lt_dur_dur, [(ELit (LDuration x), _); (ELit (LDuration y), _)] -> - ELit (LBool (o_lt_dur_dur rpos x y)) + ELit (LBool (o_lt_dur_dur (rpos ()) x y)) | Lte_int_int, [(ELit (LInt x), _); (ELit (LInt y), _)] -> ELit (LBool (o_lte_int_int x y)) | Lte_rat_rat, [(ELit (LRat x), _); (ELit (LRat y), _)] -> @@ -315,7 +320,7 @@ let rec evaluate_operator | Lte_dat_dat, [(ELit (LDate x), _); (ELit (LDate y), _)] -> ELit (LBool (o_lte_dat_dat x y)) | Lte_dur_dur, [(ELit (LDuration x), _); (ELit (LDuration y), _)] -> - ELit (LBool (o_lte_dur_dur rpos x y)) + ELit (LBool (o_lte_dur_dur (rpos ()) x y)) | Gt_int_int, [(ELit (LInt x), _); (ELit (LInt y), _)] -> ELit (LBool (o_gt_int_int x y)) | Gt_rat_rat, [(ELit (LRat x), _); (ELit (LRat y), _)] -> @@ -325,7 +330,7 @@ let rec evaluate_operator | Gt_dat_dat, [(ELit (LDate x), _); (ELit (LDate y), _)] -> ELit (LBool (o_gt_dat_dat x y)) | Gt_dur_dur, [(ELit (LDuration x), _); (ELit (LDuration y), _)] -> - ELit (LBool (o_gt_dur_dur rpos x y)) + ELit (LBool (o_gt_dur_dur (rpos ()) x y)) | Gte_int_int, [(ELit (LInt x), _); (ELit (LInt y), _)] -> ELit (LBool (o_gte_int_int x y)) | Gte_rat_rat, [(ELit (LRat x), _); (ELit (LRat y), _)] -> @@ -335,7 +340,7 @@ let rec evaluate_operator | Gte_dat_dat, [(ELit (LDate x), _); (ELit (LDate y), _)] -> ELit (LBool (o_gte_dat_dat x y)) | Gte_dur_dur, [(ELit (LDuration x), _); (ELit (LDuration y), _)] -> - ELit (LBool (o_gte_dur_dur rpos x y)) + ELit (LBool (o_gte_dur_dur (rpos ()) x y)) | Eq_int_int, [(ELit (LInt x), _); (ELit (LInt y), _)] -> ELit (LBool (o_eq_int_int x y)) | Eq_rat_rat, [(ELit (LRat x), _); (ELit (LRat y), _)] -> @@ -345,7 +350,7 @@ let rec evaluate_operator | Eq_dat_dat, [(ELit (LDate x), _); (ELit (LDate y), _)] -> ELit (LBool (o_eq_dat_dat x y)) | Eq_dur_dur, [(ELit (LDuration x), _); (ELit (LDuration y), _)] -> - ELit (LBool (o_eq_dur_dur rpos x y)) + ELit (LBool (o_eq_dur_dur (rpos ()) x y)) | HandleDefault, [(EArray excepts, _); just; cons] -> ( (* This case is for lcalc with exceptions: we rely OCaml exception handling here *) @@ -846,7 +851,7 @@ let evaluate_expr_safe : with Runtime.Error (err, rpos) -> Message.error ~extra_pos:(List.map (fun rp -> "", Expr.runtime_to_pos rp) rpos) - "Error during evaluation: %a." Format.pp_print_text + "During evaluation: %a." Format.pp_print_text (Runtime.error_message err) (* Typing shenanigan to add custom terms to the AST type. *) diff --git a/runtimes/jsoo/runtime.ml b/runtimes/jsoo/runtime.ml index 53399c55..36a8c335 100644 --- a/runtimes/jsoo/runtime.ml +++ b/runtimes/jsoo/runtime.ml @@ -60,11 +60,16 @@ let date_of_js d = if String.contains d 'T' then d |> String.split_on_char 'T' |> List.hd else d in + let fail () = failwith "date_of_js: invalid date" in match String.split_on_char '-' d with - | [year; month; day] -> - R_ocaml.date_of_numbers (int_of_string year) (int_of_string month) - (int_of_string day) - | _ -> failwith "date_of_js: invalid date" + | [year; month; day] -> ( + match + R_ocaml.date_of_numbers (int_of_string year) (int_of_string month) + (int_of_string day) + with + | Some d -> d + | None -> fail ()) + | _ -> fail () let date_to_js d = Js.string @@ R_ocaml.date_to_string d diff --git a/runtimes/ocaml/runtime.ml b/runtimes/ocaml/runtime.ml index e7e90b22..4d626efd 100644 --- a/runtimes/ocaml/runtime.ml +++ b/runtimes/ocaml/runtime.ml @@ -64,13 +64,17 @@ let error_to_string = function | IndivisibleDurations -> "IndivisibleDurations" let error_message = function - | AssertionFailed -> "this assertion doesn't hold" - | NoValue -> "no computation with valid conditions found" - | Conflict -> "two or more concurring valid computations" - | DivisionByZero -> "division by zero" + | AssertionFailed -> "an assertion doesn't hold" + | NoValue -> "no applicable rule to define this variable in this situation" + | Conflict -> + "conflict between multiple valid consequences for assigning the same \ + variable" + | DivisionByZero -> + "a value is being used as denominator in a division and it computed to zero" | NotSameLength -> "traversing multiple lists of different lengths" | UncomparableDurations -> - "comparing durations in different units (e.g. months vs. days)" + "ambiguous comparison between durations in different units (e.g. months \ + vs. days)" | IndivisibleDurations -> "dividing durations that are not in days" exception Error of error * source_position list diff --git a/tests/arithmetic/bad/division_by_zero.catala_en b/tests/arithmetic/bad/division_by_zero.catala_en index ae6fd562..2d69ea63 100644 --- a/tests/arithmetic/bad/division_by_zero.catala_en +++ b/tests/arithmetic/bad/division_by_zero.catala_en @@ -33,12 +33,13 @@ scope Money: ```catala-test-inline $ catala test-scope Dec -[ERROR] Error during evaluation: division by zero. +[ERROR] During evaluation: a value is being used as denominator in a division + and it computed to zero. -┌─⯈ tests/arithmetic/bad/division_by_zero.catala_en:20.26-20.27: +┌─⯈ tests/arithmetic/bad/division_by_zero.catala_en:20.28-20.30: └──┐ 20 │ definition i equals 1. / 0. - │ ‾ + │ ‾‾ └┬ `Division_by_zero` exception management └─ with decimals #return code 123# @@ -46,12 +47,13 @@ $ catala test-scope Dec ```catala-test-inline $ catala test-scope Int -[ERROR] Error during evaluation: division by zero. +[ERROR] During evaluation: a value is being used as denominator in a division + and it computed to zero. -┌─⯈ tests/arithmetic/bad/division_by_zero.catala_en:10.25-10.26: +┌─⯈ tests/arithmetic/bad/division_by_zero.catala_en:10.27-10.28: └──┐ 10 │ definition i equals 1 / 0 - │ ‾ + │ ‾ └┬ `Division_by_zero` exception management └─ with integers #return code 123# @@ -59,12 +61,13 @@ $ catala test-scope Int ```catala-test-inline $ catala test-scope Money -[ERROR] Error during evaluation: division by zero. +[ERROR] During evaluation: a value is being used as denominator in a division + and it computed to zero. -┌─⯈ tests/arithmetic/bad/division_by_zero.catala_en:30.29-30.30: +┌─⯈ tests/arithmetic/bad/division_by_zero.catala_en:30.31-30.35: └──┐ 30 │ definition i equals $10.0 / $0.0 - │ ‾ + │ ‾‾‾‾ └┬ `Division_by_zero` exception management └─ with money #return code 123# diff --git a/tests/date/bad/uncomparable_duration.catala_en b/tests/date/bad/uncomparable_duration.catala_en index 3e51f6a3..90eda783 100644 --- a/tests/date/bad/uncomparable_duration.catala_en +++ b/tests/date/bad/uncomparable_duration.catala_en @@ -42,8 +42,8 @@ scope Ge: ```catala-test-inline $ catala test-scope Ge -[ERROR] Error during evaluation: comparing durations in different units (e.g. - months vs. days). +[ERROR] During evaluation: ambiguous comparison between durations in + different units (e.g. months vs. days). ┌─⯈ tests/date/bad/uncomparable_duration.catala_en:40.31-40.33: └──┐ @@ -56,8 +56,8 @@ $ catala test-scope Ge ```catala-test-inline $ catala test-scope Gt -[ERROR] Error during evaluation: comparing durations in different units (e.g. - months vs. days). +[ERROR] During evaluation: ambiguous comparison between durations in + different units (e.g. months vs. days). ┌─⯈ tests/date/bad/uncomparable_duration.catala_en:30.31-30.32: └──┐ @@ -70,8 +70,8 @@ $ catala test-scope Gt ```catala-test-inline $ catala test-scope Le -[ERROR] Error during evaluation: comparing durations in different units (e.g. - months vs. days). +[ERROR] During evaluation: ambiguous comparison between durations in + different units (e.g. months vs. days). ┌─⯈ tests/date/bad/uncomparable_duration.catala_en:20.31-20.33: └──┐ @@ -84,8 +84,8 @@ $ catala test-scope Le ```catala-test-inline $ catala test-scope Lt -[ERROR] Error during evaluation: comparing durations in different units (e.g. - months vs. days). +[ERROR] During evaluation: ambiguous comparison between durations in + different units (e.g. months vs. days). ┌─⯈ tests/date/bad/uncomparable_duration.catala_en:10.31-10.32: └──┐ diff --git a/tests/default/bad/conflict.catala_en b/tests/default/bad/conflict.catala_en index 5ba659ca..066749d1 100644 --- a/tests/default/bad/conflict.catala_en +++ b/tests/default/bad/conflict.catala_en @@ -11,7 +11,7 @@ scope A: ```catala-test-inline $ catala Interpret -s A --message=gnu -tests/default/bad/conflict.catala_en:8.56-8.57: [ERROR] Error during evaluation: two or more concurring valid computations. +tests/default/bad/conflict.catala_en:8.56-8.57: [ERROR] During evaluation: conflict between multiple valid consequences for assigning the same variable. tests/default/bad/conflict.catala_en:8.56-8.57: [ERROR] tests/default/bad/conflict.catala_en:9.56-9.57: [ERROR] #return code 123# diff --git a/tests/default/bad/empty.catala_en b/tests/default/bad/empty.catala_en index b7bc3483..5fc1fab1 100644 --- a/tests/default/bad/empty.catala_en +++ b/tests/default/bad/empty.catala_en @@ -19,7 +19,8 @@ $ catala test-scope A 6 │ output y content boolean │ ‾ └─ Article -[ERROR] Error during evaluation: no computation with valid conditions found. +[ERROR] During evaluation: no applicable rule to define this variable in this + situation. ┌─⯈ tests/default/bad/empty.catala_en:6.10-6.11: └─┐ diff --git a/tests/default/bad/empty_with_rules.catala_en b/tests/default/bad/empty_with_rules.catala_en index ad185d8b..45a3918c 100644 --- a/tests/default/bad/empty_with_rules.catala_en +++ b/tests/default/bad/empty_with_rules.catala_en @@ -14,7 +14,8 @@ scope A: ```catala-test-inline $ catala test-scope A -[ERROR] Error during evaluation: no computation with valid conditions found. +[ERROR] During evaluation: no applicable rule to define this variable in this + situation. ┌─⯈ tests/default/bad/empty_with_rules.catala_en:5.10-5.11: └─┐ diff --git a/tests/exception/bad/two_exceptions.catala_en b/tests/exception/bad/two_exceptions.catala_en index 79c87874..04231252 100644 --- a/tests/exception/bad/two_exceptions.catala_en +++ b/tests/exception/bad/two_exceptions.catala_en @@ -17,7 +17,8 @@ scope A: ```catala-test-inline $ catala test-scope A -[ERROR] Error during evaluation: two or more concurring valid computations. +[ERROR] During evaluation: conflict between multiple valid consequences for + assigning the same variable. ┌─⯈ tests/exception/bad/two_exceptions.catala_en:12.23-12.24: └──┐ diff --git a/tests/func/bad/bad_func.catala_en b/tests/func/bad/bad_func.catala_en index be91f999..51ab0729 100644 --- a/tests/func/bad/bad_func.catala_en +++ b/tests/func/bad/bad_func.catala_en @@ -29,7 +29,8 @@ $ catala test-scope R ```catala-test-inline $ catala test-scope S -[ERROR] Error during evaluation: two or more concurring valid computations. +[ERROR] During evaluation: conflict between multiple valid consequences for + assigning the same variable. ┌─⯈ tests/func/bad/bad_func.catala_en:14.65-14.70: └──┐ diff --git a/tests/modules/good/output/mod_def.ml b/tests/modules/good/output/mod_def.ml index ca9ca82d..ebeabc56 100644 --- a/tests/modules/good/output/mod_def.ml +++ b/tests/modules/good/output/mod_def.ml @@ -66,7 +66,7 @@ let half_ : integer -> decimal = fun (x_: integer) -> o_div_int_int {filename="tests/modules/good/mod_def.catala_en"; - start_line=21; start_column=12; end_line=21; end_column=13; + start_line=21; start_column=14; end_line=21; end_column=15; law_headings=["Test modules + inclusions 1"]} x_ (integer_of_string "2") diff --git a/tests/scope/bad/scope.catala_en b/tests/scope/bad/scope.catala_en index 3fee7128..207f187d 100644 --- a/tests/scope/bad/scope.catala_en +++ b/tests/scope/bad/scope.catala_en @@ -16,7 +16,8 @@ scope A: ```catala-test-inline $ catala test-scope A -[ERROR] Error during evaluation: two or more concurring valid computations. +[ERROR] During evaluation: conflict between multiple valid consequences for + assigning the same variable. ┌─⯈ tests/scope/bad/scope.catala_en:13.57-13.61: └──┐