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 non-iteratrable input of yieldEach #455

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

Conversation

Yi2255
Copy link
Contributor

@Yi2255 Yi2255 commented Oct 24, 2024

Logic:

  1. Try to use existing variables and avoid creating new ones proactively, as this can lead to uninteresting and meaningless code.
  2. If the variable passed to the current CodeGenerator is iterable, use it directly.
  3. If it is not iterable, randomly generate an array composed of variables from the context, which, in the worst case, will still be empty, ensuring that what follows yieldEach is an iterable object, thus eliminating the need for additional guard checks.

Copy link

google-cla bot commented Oct 24, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@saelo saelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@@ -1085,7 +1085,7 @@ public let CodeGenerators: [CodeGenerator] = [
}
} else {
// TODO only do this when the value is iterable?
b.yieldEach(val)
b.yieldEach(b.buildIteratorVariable(b, val))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here it would make sense to split this into two generators:

    CodeGenerator("YieldGenerator", inContext: .generatorFunction, inputs: .one) { b, val in
        assert(b.context.contains(.generatorFunction))
        if probability(0.9) {
            b.yield(val)
        } else {
            b.yield()
        }
    },

    CodeGenerator("YieldEachGenerator", inContext: .generatorFunction, inputs: .required(.iterable)) { b, val in
        assert(b.context.contains(.generatorFunction))
        b.yieldEach(val)
    },

That way it's easy to guarantee that we get a .iterable.

let f = b.buildGeneratorFunction(with: b.randomParameters(), isStrict: probability(0.1)) { _ in
b.buildRecursive()
if probability(0.5) {
b.yield(b.randomVariable())
} else {
b.yieldEach(b.randomVariable())
b.yieldEach(b.buildIteratorVariable(b, it))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and in the AsyncGeneratorFunctionGenerator, I would recommend doing this differently. For one, I think we would usually prefer to yield something that we create in this function (i.e. in the buildRecursive() call), but that's not possible if the it comes from the outside. I would probably recommend just doing something like this:

} else {
    let randomVariables = b.randomVariables(Int.random(in: 1...5))
    let array = b.createArray(with: randomVariables)
    b.yieldEach(array)
}

This will then cause us to always yield a plain array, but we have the other CodeGenerator below for yielding other stuff. Also the InputMutator will quickly shuffle this around to yield from other things.

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