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

Deriving show: Functor application not allowed here. #232

Open
brokenpylons opened this issue Sep 23, 2020 · 2 comments
Open

Deriving show: Functor application not allowed here. #232

brokenpylons opened this issue Sep 23, 2020 · 2 comments

Comments

@brokenpylons
Copy link

brokenpylons commented Sep 23, 2020

I have this code:

module type LABEL = sig
  type t
  val equal: t -> t -> bool
  val compare: t -> t -> int
  val to_string: t -> string
  val pp: Format.formatter -> t -> unit
end
module Make(Label: LABEL) = struct 
  type t = {
    label: Set.Make(Label).t 
  }
  [@@deriving show]
end

And I get:

Error: broken invariant in parsetree: Functor application not allowed here.

Is there a reason why this shouldn't work? The code compiles if i create the module beforehand module S = Set.Make(Label) and then use S.t instead.

@brokenpylons
Copy link
Author

Meh, I think I figured it out, Set.Make(Label).pp is then called in the body of the generated function, right? So the generator could do let module S = Set.Make(Label) in for each module in the type, however if the module has side effects, this probably wouldn't be a good idea...

@gasche
Copy link
Contributor

gasche commented Sep 24, 2020

You figured it out correctly indeed.

It may be possible to name the module carefully: if each occurrence of a functor application (in a module path that we want to derive from) is turned into a binding on the fly, we should be able to preserve the evaluation behavior of the original program. (What we don't want is to evaluate the functor application several times in the generated code.) But this is a bit delicate, and there may be case where this would change the type signature of the program (type equalities lost in the renaming, this kind of thing).

I think that the more robust approach is to bail out when we see this, and ask the user to do the let-binding themselves, so that they understand this is happening. The error message could be better than it is today (it looks like ppx_deriving is not saying anything here, the error in the generated code is caught by the compiler later on), but it would be the same behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants