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

Regex: Inform user about slow regex! macro #595

Closed
killercup opened this issue Jan 29, 2016 · 6 comments · Fixed by #642
Closed

Regex: Inform user about slow regex! macro #595

killercup opened this issue Jan 29, 2016 · 6 comments · Fixed by #642

Comments

@killercup
Copy link
Member

As discussed in #587, the regex! compiler plugin is currently slower than the runtime based Regex::new. We should inform the user about that.

@llogiq
Copy link
Contributor

llogiq commented Feb 3, 2016

I have built the lint, but I still have to stop it from matching all parts of the macro expansion. @BurntSushi is there an expression that all regex! expansions have in common?

@BurntSushi
Copy link
Member

@llogiq Hmm, good question! That's actually pretty tricky to answer. The code is here. I think for the forseeable future, if you find a type with the letters nfa in them (ignoring case), then that should be sufficient.

@llogiq
Copy link
Contributor

llogiq commented Feb 3, 2016

Oh, I can already check if an expression is part of the macro expansion. Unfortunately, the lint matches multiple times per regex!(_).

@BurntSushi
Copy link
Member

@llogiq Ah, so you need a uniquely identifying expression? Can you use a type definition? e.g.,

struct Nfa<'t>

@llogiq
Copy link
Contributor

llogiq commented Feb 4, 2016

Hmmm... I think I'll try to use the outer block then (and a Visitor to skip walking when found).

@llogiq
Copy link
Contributor

llogiq commented Feb 8, 2016

I have a regex_macro branch with my current code. Even using blocks doesn't keep it from being run multiple times. Either I'll have to go crate-(or at least module-) wide instead of function wide and keep the matched positions in some kind of set to remove the duplicate warnings, or someone finds a better solution. cc @Manishearth

llogiq added a commit that referenced this issue Feb 8, 2016
@llogiq llogiq mentioned this issue Feb 8, 2016
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 a pull request may close this issue.

3 participants