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

Unused values when deriving show for private types #171

Open
emillon opened this issue Jun 25, 2018 · 3 comments
Open

Unused values when deriving show for private types #171

emillon opened this issue Jun 25, 2018 · 3 comments

Comments

@emillon
Copy link
Contributor

emillon commented Jun 25, 2018

Hi,

The following piece of code triggers warning 32 (unused value show_x). This is a common pattern, it happens whenever an exposed type uses a type that is not exposed.

module M : sig
  type t
  [@@deriving show]

  val make : int -> t
end = struct
  type x = int
  [@@deriving show]

  type t = C of x
  [@@deriving show]

  let make n = C n
end

The problem is the following:

  • 4 bindings are created: pp_x, show_x, pp, and show.
  • pp, and show are exposed
  • pp_x is used in the generated definition for pp
  • show_x is not, so a warning is triggered.

The root cause is similar to ocaml-ppx/ppx_deriving_yojson#76: there is a "convenience function" (here show) that is not meant to be used recursively.

I am not sure how to best fix this without breaking existing code, but here are some possibilities:

  • generate let _ = show_x to suppress the warning
  • similarly, wrap the generated structure within [@@@warning "-32"]
  • add an option so that we can [@@derive show { only_pp = true }]
  • introduce a [@@deriving pp] that only derives pp. This would enable [%pp: t] as well.

What do you think?

Thanks!

@whitequark
Copy link
Collaborator

I think [@@warning "-32"] and [@deriving pp] both seem like good options; we can simply do both of them.

@NathanReb
Copy link
Collaborator

Hi,

What's the status on this issue?

I'm happy to contribute for the [@deriving pp] if that's fine by you!

@whitequark
Copy link
Collaborator

@NathanReb As mentioned above in #171 (comment), I think both [@@warning "-32"] and [@deriving pp] ought to be implemented. If you can do one of them, great!

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

3 participants