Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ppx_repr error message when shadowing a Stdlib type without @nobuiltin #63

Open
Ngoguey42 opened this issue May 20, 2021 · 3 comments

Comments

@Ngoguey42
Copy link
Contributor

type result = int [@@deriving repr]

type t = result list [@@deriving repr]

causes

19 | type t = result list [@@deriving repr]
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: This expression has type
         'a Repr.ty -> 'b Repr.ty -> ('a, 'b) Stdlib.result Repr.ty
       but an expression was expected of type 'c Repr.ty
@craigfe
Copy link
Member

craigfe commented Jun 7, 2021

At some level, this is expected and is what the [@nobuiltin] attribute is for. (See ppx_deriving conventions and testing of the ppx_repr implementation.)

In this case, we could do better because result is used at a different arity from the one in the standard library, so attempting to use Repr.result isn't going to work. What do you think?

@Ngoguey42
Copy link
Contributor Author

Thanks for the precisions.

The biggest problem here is that the error message is very misleading, it took me a while to understand this was caused by a name clash. Ideally, an error message would have suggested me to use [@nobuiltin].

When Repr sees a result, could we inject into the AST something that asserts that result is Stdlib.result(and likewise for all the other types from stdlib)? This way I suppose that the error message would be clearer.

@craigfe
Copy link
Member

craigfe commented Jun 16, 2021

Yep, good point. This is part of a larger problem with PPXes having no way to hook into the error message mechanism. It's possible we could emit code like the following:

(* In a PPX runtime library *)
type ('a, 'b) expected_equality = Refl : ('a, 'a) expected_equality

(* In user code *)
type (_, _) result = unit 
type foo = int * (bool, string) result

let foo_t = 
  let _ : (('a, 'b) result, ('a, 'b) Stdlib.result) expected_equality = Ppx_repr_runtime.Refl in
  ...

which would produce an error of the following sort:

Line 1, characters 72-76:
Error: This expression has type (unit, unit) expected_equality
       but an expression was expected of type
         (unit, ('a, 'b) Stdlib.result) expected_equality
       Type unit is not compatible with type ('a, 'b) Stdlib.result

It's very possible that there are ways to coerce a better error message out of OCaml.

@craigfe craigfe changed the title ppx_repr doesn't work with types shadowing stdlib types Improve ppx_repr error message when shadowing a Stdlib type without @nobuiltin Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants