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

Add initial coarse reduction pass for reducing alternatives #4216

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

Conversation

DRMacIver
Copy link
Member

@DRMacIver DRMacIver commented Dec 27, 2024

This adds an initial phase to shrinking that is allowed to make changes that would be bad to make as part of the main shrink pass, with the main goal of producing better results for one_of.

This isn't as good a pass for this as I wanted to add. In particular it results in no new guarantees about shrinking, because we can't iterate this pass to a fixed point along with the others, but it should start the shrinking off from something that is much more likely to already be the minimal usable branch for this strategy.

When trying to add a more reliable pass I kept running into issues that I think will be easier to solve after we've made some of the discussed changes and generally fully moved over to the new IR.

@DRMacIver DRMacIver requested a review from Zac-HD as a code owner December 27, 2024 13:34
@DRMacIver DRMacIver marked this pull request as draft December 27, 2024 14:57
@DRMacIver DRMacIver force-pushed the DRMacIver/more-shrinking-of-alternatives branch 4 times, most recently from 9d15a6d to e7d91e8 Compare December 28, 2024 16:25
@DRMacIver DRMacIver marked this pull request as ready for review December 28, 2024 16:59
Copy link
Member

@tybug tybug left a comment

Choose a reason for hiding this comment

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

lgtm! Shrinking benchmark is neutral: shrinking

Which isn't a bad thing, as this pass is likely to increase shrink quality, not just speed.

Instead of guessing where the one_of choices are, could we match the label of the node's example against OneOfStrategy? That should reduce false positives to zero.

This adds an initial phase to shrinking that is allowed to make
changes that would be bad to make as part of the main shrink pass,
with the main goal of producing better results for ``one_of``.
@DRMacIver DRMacIver force-pushed the DRMacIver/more-shrinking-of-alternatives branch from 911206a to 15f040d Compare December 29, 2024 11:00
@DRMacIver
Copy link
Member Author

Instead of guessing where the one_of choices are, could we match the label of the node's example against OneOfStrategy? That should reduce false positives to zero.

So I thought about that, but I decided it was plausible that there were other patterns that had sufficiently similar shape that it was actually good to match things that aren't OneOfStrategy. e.g. you could implement the same sort of thing in a composite, but also this currently matches length parameters and that struck me as plausibly good for some hard to reduce cases of that.

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