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

ppx_deriving_qcheck cannot derive a generator from a recursive type declaration with containers #269

Open
Invizory opened this issue Jan 8, 2023 · 4 comments

Comments

@Invizory
Copy link

Invizory commented Jan 8, 2023

Minimal reproducible example:

type t = Box of t list [@@deriving qcheck]

ppx_deriving_qcheck produces the following generator with unbound gen:

let gen = QCheck.Gen.map (fun gen0 -> Box gen0) (QCheck.Gen.list gen)

This trick (given -rectypes is enabled) make it derive the following generator:

type 'a open_t = Box of 'a list
and t = t open_t
[@@deriving qcheck]
let rec gen_open_t gen_a =
  QCheck.Gen.map (fun gen0 -> Box gen0) (QCheck.Gen.list gen_a)
and gen_sized n = gen_open_t (gen_sized (n / 2))
let gen = QCheck.Gen.sized gen_sized

However, this generatior is non-terminating.

@jmid
Copy link
Collaborator

jmid commented Jan 8, 2023

Thanks for the report!

Ping @vch9 - I think we are not handling type constructors correctly in is_rec_typ

@vch9
Copy link
Contributor

vch9 commented Jan 9, 2023

Thanks for the report again.

Definitely a problem in is_rec_type, t should not be considered recursive (in the latter). I will try to have a look

@vch9
Copy link
Contributor

vch9 commented Jan 9, 2023

I think the deriver fails on a very much simpler example: type t = t list. I did not consider the rectypes flag as I just discovered it 😅.

It creates:

let gen = QCheck.Gen.list gen
let arb = QCheck.make @@ gen

Should we be smarter than:

let rec gen = QCheck.Gen.list gen
let arb = QCheck.make @@ gen

?

@jmid
Copy link
Collaborator

jmid commented Jan 9, 2023

IMO using rectypes is a cornercase that we don't necessarily have to support (I read the report as trying to use it as a workaround).

I suggest prioritising the reported case type t = Box of t list of "vanilla OCaml usage" where it seems we don't identify an occurrence of t as recursive when it is a type parameter to a type constructor such as list (or option or result ...) 🤔

In trying out a potential workaround I just experienced the following error variant:

utop # #require "ppx_deriving";;
utop # #require "ppx_deriving_qcheck";;
utop # type t = Box of foo
       and foo = t list [@@deriving qcheck];;
Error: Unbound value n

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