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.
2024-11-08 07:51:43 +03:00 · 2024-05-02 16:20:54 +02:00 · 2024-05-02 16:20:54 +02:00 · facce68b25
commit facce68b25
parent 4d412027d0
13 changed files with 70 additions and 47 deletions
--- 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 "@[<hov 2>%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 "@ ")
--- 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. *)
--- 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

--- 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
--- 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#
--- 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:
 └──┐
--- 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#
--- 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:
 └─┐
--- 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:
 └─┐
--- 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:
 └──┐
--- 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:
 └──┐
--- 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")

--- 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:
 └──┐