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.

Sign up for GitHub

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

Jump to bottom

automata: Fix broken universal start states with sparse DFA #1195

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

automata: Fix broken universal start states with sparse DFA #1195

wants to merge 1 commit into from

Conversation

ishitatsuyuki
Copy link

The state IDs were not remapped, which will usually result in an index out of range error.

@BurntSushi
Copy link
Member

Is it possible to add a regression test?

@ishitatsuyuki
Copy link
Author

Sure, let me give it a try tomorrow (it's late here today).

@ishitatsuyuki
Copy link
Author

Hi, I added a test based on examples in Automaton::is_special_state. I'm asserting that the initial state is .is_start_state(), but this seems to be broken for some reason. Could you help figuring out why? I'm afraid I'm not familiar enough with how the start state ranges are being assigned.

@ishitatsuyuki
Copy link
Author

Alternatively, I could remove the assert for now and file an issue for it to be fixed later.

@ishitatsuyuki
automata: Fix broken universal start states with sparse DFA
35cee1b 
The state IDs were not remapped, which will usually result in an index
out of range error.

Add a test based on is_special_state's doctest, which will validate the
start state's behavior with a custom searcher.
@ishitatsuyuki
Copy link
Author

It looks like start states were simply being removed with set_no_special_start_states, so this was the intended behavior. I have removed the assert now. Let me know if the tests look good to you.

@ishitatsuyuki
Copy link
Author

Hi, mind giving this a review?

@ishitatsuyuki
Copy link
Author

Friendly ping, let me know if there's something else I need to address

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ishitatsuyuki @BurntSushi