Embed errors in the AST instead of raising

[panglesd]

Currently, when your PPX encounter an error, it uses the raise_errorf function to raise a located error.

The exception is caught by ppxlib, which in this case:

• Catch the error,
• stops the rewriting process
• add the error (as a [%%%ocaml.error] extension node) to the last valid ast
• Use the resulting AST

The interruption of the rewriting is quite bad for the user experience! The implication for the users are:

• Since your PPX runs at the "context-free" phase, the "last valid AST" is before the context-free phase. So, no other derivers/extenders get run, which generates a lot of noise in the errors (such as "uninterpreted extensions" or "unbound identifiers")
• Only one (meaningful) error from your PPX is reported at a time.

Example

For instance:

type t = int -> int [@@deriving yaml]
type u = int -> int [@@deriving yaml]
type v = int [@@deriving yaml]

let _ = v_of_yaml

would generate several errors:

• Cannot derive anything for t
• unbound value v_of_yojson

when the correct set of errors would be:

• Cannot derive anything for t
• Cannot derive anything for u

You can find more information about error reporting in PPXs in this section of the ppxlib manual.

❓ Would you be willing to accept contributions to this issue? I'm considering assigning its resolution as part of an outreachy internship: see more information here.

[nebadesmondc]

Hello, can i work on this?

[patricoferris]

This would be an excellent addition, thank you for taking the time to right a really detailed issue @panglesd :))

Hi @Dezzy12 ! I see no reason why not, thank you for wanting to contribute, let us know if you need any further help or assistance.

[nebadesmondc]

okay thank you. i will get to back to you in case i need some assistance

[nebadesmondc]

hi @patricoferris I opened a opened a pull request for this issue, please review

[panglesd]

Note that we decided that we will change the ppxlib behaviour regarding the handling of exceptions.
Catching an exception will no longer stop the rewriting process.

So, the example I gave in the original issue is not relevant any more.
However, embedding errors still have advantages! It allows to report multiple errors, while still outputing valid AST for the part that were successful.

An updated example would be that, with embedding,

type t = int -> int
and u = int -> int
and v = int [@@deriving yaml]

let _ = v_of_yaml

will show errors for both t and u, and still define the v_of_yaml value; while currently it would show only one error and not define v_of_yaml.

[nebadesmondc]

Okay thank you