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

feat: Add support for converting multiple NFAs into one DFA. #7

Merged
merged 14 commits into from
Dec 8, 2024

Conversation

Louis-He
Copy link
Contributor

@Louis-He Louis-He commented Dec 5, 2024

Description

Validation performed

accept: Vec<State>,
states: HashSet<State>,
transitions: HashMap<State, HashMap<char, Transition>>, // from_state -> symbol -> to_state
dfa_to_accepted_nfa_state_mapping: Option<HashMap<State, Vec<(usize, crate::nfa::nfa::State)>>>, // to determine which NFA gets matched
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why cargo fmt doesn't work, but this line exceeds the 100-char limit, right? Can we put the comment in a separate line before this line? (It's hard to navigate long lines without using mouse, lol)


// Helper functions for converting multiple NFAs to a single DFA
impl DFA {
fn epsilon_closure(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Shouldn't this be a part of NFA?

use std::process::id;

#[derive(Clone, Debug, Eq, Hash, PartialEq)]
struct State(String);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need the state to be a string?

);
}

fn simulate(&self, input: &str) -> (Option<HashSet<usize>>, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need a different API that takes char by char since the lexer emits chars as tokens, we don't know the size of string in advance.
In this way, we might also need an API to reset the current simulation state.

src/dfa/dfa.rs Outdated
Comment on lines 22 to 25
start: State,
accept: Vec<State>,
states: HashSet<State>,
transitions: HashMap<State, HashMap<char, Transition>>, // from_state -> symbol -> to_state
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed offline, there's a performance concern in this part; shall we add a TODO to keep track of the issue?

@LinZhihao-723 LinZhihao-723 changed the title Add DFA, able to merge multiple NFAs and identify which NFA got matched feat: Add support for converting multiple NFAs into one DFA. Dec 8, 2024
@LinZhihao-723 LinZhihao-723 merged commit 8174f5a into Toplogic-Inc:main Dec 8, 2024
2 checks passed
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