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

Fix is_rec_typ deriver bug(s) #272

Merged
merged 5 commits into from
May 2, 2023
Merged

Conversation

jmid
Copy link
Collaborator

@jmid jmid commented Feb 23, 2023

This is an attempt at fixing #269.
With the fix in 3c1f147 we are able to derive a straight-forward, type-correct generator for type t = Foo of t list (yay!)

let rec gen_sized n =
  QCheck.Gen.map (fun gen0 -> Foo gen0) (QCheck.Gen.list (gen_sized (n / 2)));;
let gen = QCheck.Gen.sized gen_sized

which blows the stack (less yay):

# Gen.generate1 gen;;
Stack overflow during evaluation (looping recursion?).

I think this is to be expected from the current simple, but predictable approach.
It could potentially be improved if we incorporate passing sizing explicitly to the derived list generator and thus try to reach a base-case that way.

While fixing this I noticed there was also a problem with record declarations in is_rec_constr_decl.
With the fix in daa357e for that, one can derive a functioning generator for a type like type t = Foo | Bar of { baz : t }:

# Gen.generate1 gen;;
- : t = Bar {baz = Foo}
# Gen.generate1 gen;;
- : t = Foo

Overall, this PR should leave the deriver in a slightly better shape than before.
I don't claim it to be bugfree on all language cases though - I think there are still core_type cases in ast.ml that we may not be handling correctly, e.g., for type constraints.

@jmid jmid force-pushed the is_rec_typ_deriver_bug branch from 202b227 to 27f4f8d Compare May 2, 2023 10:00
@jmid
Copy link
Collaborator Author

jmid commented May 2, 2023

Rebased on main to reenable CI

@jmid jmid force-pushed the is_rec_typ_deriver_bug branch from 543a4e4 to d779d26 Compare May 2, 2023 12:33
@jmid jmid merged commit cab908d into c-cube:main May 2, 2023
@jmid jmid deleted the is_rec_typ_deriver_bug branch May 2, 2023 13:07
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

Successfully merging this pull request may close these issues.

1 participant