feature request: better errors #11539

mimoo · 2022-09-11T03:57:17Z

Hello,

I'm trying to debug an issue I'm getting, and the error I'm getting is very verbose and seem to be the concatenation of a number of errors that are somewhat related. Feature request: it'd be nice to have errors that can be parsed by humans.

The error I'm getting, as example:

File "src/lib/mina_base/control.ml", lines 11-15, characters 4-51:
11 | ....type t = Mina_wire_types.Mina_base.Control.V2.t =
12 |       | Proof of Pickles.Side_loaded.Proof.Stable.V2.t
13 |       | Signature of Signature.Stable.V1.t
14 |       | None_given
15 |     [@@deriving sexp, equal, yojson, hash, compare]
Error: This variant or record definition does not match that of type
         Mina_wire_types.Mina_base.Control.V2.t
       Constructors do not match:
         Signature of Mina_wire_types.Mina_base_signature.V1.t
       is not the same as:
         Signature of Mina_base__.Signature.Stable.V1.t
       The type
         Mina_wire_types.Mina_base_signature.V1.t =
           Mina_wire_types.Snark_params.Tick.Field.t *
           Mina_wire_types.Snark_params.Tick.Inner_curve.Scalar.t
       is not equal to the type
         Mina_base__.Signature.Stable.V1.t =
           Snark_params.Tick.Field.t * Snark_params.Tick.Inner_curve.Scalar.t
       Type Mina_wire_types.Snark_params.Tick.Field.t = Pasta_bindings.Fp.t
       is not equal to type
         Snark_params.Tick.Field.t = Pasta_bindings.Fp256.t 
File "src/lib/mina_base/stake_delegation.ml", lines 9-14, characters 4-51:
 9 | ....type t = Mina_wire_types.Mina_base.Stake_delegation.V1.t =
10 |       | Set_delegate of
11 |           { delegator : Public_key.Compressed.Stable.V1.t
12 |           ; new_delegate : Public_key.Compressed.Stable.V1.t
13 |           }
14 |     [@@deriving compare, equal, sexp, hash, yojson]
Error: This variant or record definition does not match that of type
         Mina_wire_types.Mina_base.Stake_delegation.V1.t
       Constructors do not match:
         Set_delegate of {
           delegator : Mina_wire_types.Public_key.Compressed.V1.t;
           new_delegate : Mina_wire_types.Public_key.Compressed.V1.t;
         }
       is not the same as:
         Set_delegate of {
           delegator : Signature_lib.Public_key.Compressed.Stable.V1.t;
           new_delegate : Signature_lib.Public_key.Compressed.Stable.V1.t;
         }
       1. Fields do not match:
         delegator : Mina_wire_types.Public_key.Compressed.V1.t;
       is not the same as:
         delegator : Signature_lib.Public_key.Compressed.Stable.V1.t;
       The type
         Mina_wire_types.Public_key.Compressed.V1.t =
           (Mina_wire_types.Snark_params.Tick.Field.t, bool)
           Mina_wire_types.Public_key.Compressed.Poly.V1.t
       is not equal to the type
         Signature_lib.Public_key.Compressed.Stable.V1.t =
           (Snark_params.Tick.Field.t, bool)
           Mina_wire_types.Public_key.Compressed.Poly.V1.t
       Type Mina_wire_types.Snark_params.Tick.Field.t = Pasta_bindings.Fp.t
       is not equal to type
         Snark_params.Tick.Field.t = Pasta_bindings.Fp256.t 
       2. Fields do not match:
         new_delegate : Mina_wire_types.Public_key.Compressed.V1.t;
       is not the same as:
         new_delegate : Signature_lib.Public_key.Compressed.Stable.V1.t;
       The type
         Mina_wire_types.Public_key.Compressed.V1.t =
           (Mina_wire_types.Snark_params.Tick.Field.t, bool)
           Mina_wire_types.Public_key.Compressed.Poly.V1.t
       is not equal to the type
         Signature_lib.Public_key.Compressed.Stable.V1.t =
           (Snark_params.Tick.Field.t, bool)
           Mina_wire_types.Public_key.Compressed.Poly.V1.t
       Type Mina_wire_types.Snark_params.Tick.Field.t = Pasta_bindings.Fp.t
       is not equal to type
         Snark_params.Tick.Field.t = Pasta_bindings.Fp256.t 
File "src/lib/mina_base/fee_transfer.mli", lines 6-7, characters 9-56:
6 | .........type Single.Stable.V2.t =
7 |       Mina_wire_types.Mina_base.Fee_transfer.Single.V2.t
Error: This variant or record definition does not match that of type
         Mina_wire_types.Mina_base.Fee_transfer.Single.V2.t
       Fields do not match:
         receiver_pk : Mina_wire_types.Public_key.Compressed.V1.t;
       is not the same as:
         receiver_pk : Mina_base_import.Public_key.Compressed.Stable.V1.t;
       The type
         Mina_wire_types.Public_key.Compressed.V1.t =
           (Mina_wire_types.Snark_params.Tick.Field.t, bool)
           Mina_wire_types.Public_key.Compressed.Poly.V1.t
       is not equal to the type
         Mina_base_import.Public_key.Compressed.Stable.V1.t =
           (Snark_params.Tick.Field.t, bool)
           Mina_wire_types.Public_key.Compressed.Poly.V1.t
       Type Mina_wire_types.Snark_params.Tick.Field.t = Pasta_bindings.Fp.t
       is not equal to type
         Snark_params.Tick.Field.t = Pasta_bindings.Fp256.t

The text was updated successfully, but these errors were encountered:

gasche · 2022-09-11T05:09:56Z

There are three errors in your output, each starting with File: ... (location information) then Error:. The first is as follows

File "src/lib/mina_base/control.ml", lines 11-15, characters 4-51:
11 | ....type t = Mina_wire_types.Mina_base.Control.V2.t =
12 |       | Proof of Pickles.Side_loaded.Proof.Stable.V2.t
13 |       | Signature of Signature.Stable.V1.t
14 |       | None_given
15 |     [@@deriving sexp, equal, yojson, hash, compare]
Error: This variant or record definition does not match that of type
         Mina_wire_types.Mina_base.Control.V2.t
       Constructors do not match:
         Signature of Mina_wire_types.Mina_base_signature.V1.t
       is not the same as:
         Signature of Mina_base__.Signature.Stable.V1.t
       The type
         Mina_wire_types.Mina_base_signature.V1.t =
           Mina_wire_types.Snark_params.Tick.Field.t *
           Mina_wire_types.Snark_params.Tick.Inner_curve.Scalar.t
       is not equal to the type
         Mina_base__.Signature.Stable.V1.t =
           Snark_params.Tick.Field.t * Snark_params.Tick.Inner_curve.Scalar.t
       Type Mina_wire_types.Snark_params.Tick.Field.t = Pasta_bindings.Fp.t
       is not equal to type
         Snark_params.Tick.Field.t = Pasta_bindings.Fp256.t

This is an explanation on the fact that the equality Mina_wire_types.Mina_base.Control.V2.t = Proof of ... | Signature of ... | None_given does not hold. It does not hold because the types of the Signature parameter are incompatible, because they are pairs of the form Pasta_bindings.Fp.t * ... in one case and Pasta_bindings.Fp256.t * ... in the other.

gasche · 2022-09-11T05:16:26Z

My impression is that this issue is not in fact actionable:

the error messages produced are a bit long and require some training to parse
this comes from the fact that the codebase uses complex modules with many definitions to follow to find the "real incompatibility" between two types, and
the error message is produced in a systematic way that describes all the steps from the types written in the problematic location to the "real incompatibility"

So I'd say: yes, they can be parsed by humans, but the reporter hasn't become such a human yet. It would of course be nicer to have error messages that can be parsed by more humans with less training, but we don't know how to do that. Is there a concrete, specific suggestion for how to improve error messages here? Otherwise this issue is the error-message equivalent of "it would be nice if programs were faster to execute": correct, but not suggesting any specific improvement we should perform. In that case we may as well close the issue.

mimoo · 2022-09-11T07:36:05Z

Not the best comparison, but in general Rust errors are much easier to parse:

If I would have to analyze why they have better errors, I would say that its a mix of:

using colors (meant for human consumption)
reusing the same layout for different types of errors: what's the error code for external discussions on the error, what's the type of error, pointers to where the error is, and call to action (the user does not have to learn a new UI every time)
use diagrams overlayed on top of code (it shows you where exactly in the code the error is)
clickable file pointers (vscode's terminal makes src/file:48 clickable)

if the error is split into several sections (you said there are three errors in my output), the separation between the section should be clearer, perhaps using some pretty ascii art or at the very least some line breaks.

Hopefully this is a more helpful post this time.

XVilka · 2022-09-11T13:33:36Z

For colors, there is this issue: #9328 and this unmerged PR: #10321

gasche · 2022-09-11T15:18:54Z

I think it's not so representative to compare an OCaml error message that requires a non-trivial explanation (because we unfold definitions to explain what is going on to the user) and a Rust error message that can be explained very simply.

OCaml also has a simple error message in this case (with colors in the terminal output):

File "test.ml", line 1, characters 8-13:
1 | let x : int33 = 42
            ^^^^^
Error: Unbound type constructor int33
Hint: Did you mean int32?

And conversely there are of course complex error messages in Rust, see https://gist.github.com/jonhoo/db1e672a5ee7dfcac3ae3d3773dfdbb0 for example.

Now that this is said to establsih a more reasonable comparison approach, let's move to your concrete suggestions. Thanks for the suggestions! (They certainly make the issue more actionable.)

using colors (meant for human consumption)

Yes, the OCaml compiler also uses colors. (Maybe less heavily than Rust; do we have suggestions for doing more?
Concrete suggestions:

Add colors to type clash error message #10321: use bold markup for types (for now we didn't agree on a consistent coloring scheme)
suggested by @Octachron: use a color for hints (not just errors and warnings)

Another thing I thought about on the specific type-conflict message in the first post was to highlight the fact that some type expressions are present / duplicated in several of the explanation items: foo1 * bar1 is not the same as foo2 * bar2, because (next item) foo1 is not the same as foo1. I wondered if we could use markup to highlight that the two occurrences of foo1 are linked, and that the two occurrences of foo2` are linked. I don't have a great idea of how to do that. (Matching colors? but then you need many colors and it looks a bit rainbowy. Footnote-style numbering, not unlike LaTeX equations? meh.)

reusing the same layout for different types of errors

I'm not sure what that means. The way that we explain errors depends on the error. Of course the general format is the same (source location, then the source fragment, then the explanation, including possibly hints at the end.) Do you have something more precise in mind?

what's the error code for external discussions on the error

We don't have error codes. We thougt about it before, it's a nice idea, we should do it, but it requires a bit of implementation work. (More than some other chnages in this space.) More like "it's an internship topic" than a two-hours hacking session.

use diagrams overlayed on top of code (it shows you where exactly in the code the error is)

We also highlight the location of the error. Single-line locations are underlined (in different ways depending on the terminal capabilities), multi-line locations correspond to precisely the part of the source that is quoted.

clickable file pointers (vscode's terminal makes src/file:48 clickable)

That's a question of configuring editors to detect the OCaml compiler source locations. Emacs does it by default, for example. (We have discussed using different source-location formats that are less human-readable but more standard, and more likely to be supported by default). I would expect the OCaml vscode mode to enable that; if it does not, we should fill an issue there!

gasche · 2022-09-11T15:19:34Z

the separation between the section should be clearer, perhaps using some pretty ascii art or at the very least some line breaks.

That's a good point! I think we should include an empty line at the end of each error.

gasche · 2022-09-12T07:55:28Z

Regarding 'diagrams overlaid on top of code': I don't want to give the idea that we are doing "as well as Rust" on this specific thing, we clearly are not: we support multi-location error messages, but the explanations cannot point to specific locations; no nice ASCII diagrams as in Rust borrowing errors for example. On the other hand, most OCaml error messages are in fact not multi-location. I think the Rust static discipline is more likely to require these complex multi-location messages (due to support for linearity). It's not clear to me in which cases OCaml would benefit from the same approach.

(Note: as pointed out to me by Denis Merigoux, it would be nice to have a library in OCaml to print such nice error messages, in the style of miette, ariadne, etc..)

gasche · 2022-09-22T07:20:54Z

Note after thinking a second more about the idea of including a blank line after error messages. Actually I think that the output we observe in the original post (with several error messages one after another) was not produced by one run of the compiler (which in general stops at the first error), but rather by a build tool (dune I presume) collating errors coming from several compiler invocations. The responsibility of presenting this aggregated output clearly lies in the driver tool rather than the compiler, so a "fix" for this issue would not be in the compiler codebase.

gasche · 2022-09-22T15:36:13Z

One interesting aspect of Rust error messages is the inclusion of a short identifier for the error, E0412 in the example. This is something that I vaguely considered proposing for OCaml, but never got around doing. @Octachron and myself discussed this more in reaction to the present issue, and the current result is an RFC proposal: RFC #33: short error identifiers.

gasche · 2023-01-20T09:31:14Z

(Note: as pointed out to me by Denis Merigoux, it would be nice to have a library in OCaml to print such nice error messages, in the style of miette, ariadne, etc..)

There is now a prototype library in OCaml called "Grace" by @johnyob: https://gitlab.com/alistair.obrien/grace

mimoo added the feature-wish label Sep 11, 2022

gasche mentioned this issue Sep 22, 2022

usability: include a blank line between separate compiler messages (errors, warnings) ocaml/dune#6158

Closed

ocaml locked and limited conversation to collaborators Jan 20, 2023

gasche converted this issue into discussion #11924 Jan 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

This issue was moved to a discussion.

feature request: better errors #11539

feature request: better errors #11539

mimoo commented Sep 11, 2022

gasche commented Sep 11, 2022

gasche commented Sep 11, 2022 •

edited

Loading

mimoo commented Sep 11, 2022 •

edited

Loading

XVilka commented Sep 11, 2022 •

edited

Loading

gasche commented Sep 11, 2022

gasche commented Sep 11, 2022

gasche commented Sep 12, 2022

gasche commented Sep 22, 2022

gasche commented Sep 22, 2022

gasche commented Jan 20, 2023

This issue was moved to a discussion.

This issue was moved to a discussion.

feature request: better errors #11539

feature request: better errors #11539

Comments

mimoo commented Sep 11, 2022

gasche commented Sep 11, 2022

gasche commented Sep 11, 2022 • edited Loading

mimoo commented Sep 11, 2022 • edited Loading

XVilka commented Sep 11, 2022 • edited Loading

gasche commented Sep 11, 2022

gasche commented Sep 11, 2022

gasche commented Sep 12, 2022

gasche commented Sep 22, 2022

gasche commented Sep 22, 2022

gasche commented Jan 20, 2023

This issue was moved to a discussion.

gasche commented Sep 11, 2022 •

edited

Loading

mimoo commented Sep 11, 2022 •

edited

Loading

XVilka commented Sep 11, 2022 •

edited

Loading