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

automata/meta: revert broadening of reverse suffix optimization #1111

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

BurntSushi
Copy link
Member

This reverts commit 8a8d599 and includes a regression test, as well as a tweak to a log message.

Essentially, the broadening was improper. We have to be careful when dealing with suffixes as opposed to prefixes. Namely, my logic previously was that the broadening was okay because we were already doing it for the reverse inner optimization. But the reverse inner optimization works with prefixes, not suffixes. So the comparison wasn't quite correct.

This goes back to only applying the reverse suffix optimization when there is a non-empty single common suffix.

Fixes #1110, Ref astral-sh/ruff#7980

This reverts commit 8a8d599 and
includes a regression test, as well as a tweak to a log message.

Essentially, the broadening was improper. We have to be careful when
dealing with suffixes as opposed to prefixes. Namely, my logic
previously was that the broadening was okay because we were already
doing it for the reverse inner optimization. But the reverse inner
optimization works with prefixes, not suffixes. So the comparison wasn't
quite correct.

This goes back to only applying the reverse suffix optimization when
there is a non-empty single common suffix.

Fixes #1110
Ref astral-sh/ruff#7980
@BurntSushi BurntSushi merged commit eb950f6 into master Oct 16, 2023
16 checks passed
@BurntSushi BurntSushi deleted the ag/fix-reverse-suffix branch October 16, 2023 14:43
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.

broadening of reverse suffix optimization has led to incorrect matches
1 participant