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

Shortcut generation for map/bind with lazy small_example #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

art-w
Copy link

@art-w art-w commented Apr 24, 2022

This proposal might not be the right bandage (maybe just expanding the documentation of choose would be better.) I ran into some issues with a recursive generator that was failing to terminate. Reading crowbar source code, there's a size fuel mechanism in place to shortcut the random generation by choosing the first available const/primitive when the generated term grows too much -- but my leaves were wrapped in a map [..] and so the shortcut couldn't trigger (for example, see the xmldiff test where knowledge of the optimization edge-case yields a manual unrolling).

Some random comments:

  • small_examples was a list of candidates, but then the shortcut would always pick the first one (such that it could have been an option):
    if size <= 1 && gen.small_examples <> [] then List.hd gen.small_examples, fun ppf () -> pp ppf "?" else

    Perhaps the plan was to select a random value from that list? (but the current behavior has the advantage of not consuming more random bits)
  • When provided an empty list, choose would previously crash at this point:
    let v, pv = generate size input (List.nth gens n) in

    Perhaps Choose [] could be simplified away by the smart constructors -- but this would push the "non-empty generator" constraint to unlazy/bind, it's not clear that this would be better.
  • If desired, the PR could be adapted to prefer the left-most available const/primitive as before, and default to the map/bind otherwise (but I think it's easier to predict the default value when it's always the first element of choose)
  • Finally, it's not super clear to me that the shortcut doesn't bias the generator toward uninteresting leaves -- hopefully afl-fuzz is able to avoid triggering it too much?

(unrelatedly, the xmldiff test seems to work on my computer -- however the calendar test crashes very fast!)

@lindig
Copy link

lindig commented Jul 11, 2023

I tried to understand how the length of lists is controlled; reading the source I came across the size parameter but could not see how it is initialized. A comment in the code of its role and initialization would be useful.

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.

2 participants