Rewrite parser as part of new regex-syntax crate. #87

BurntSushi · 2015-05-25T16:53:25Z

This commit introduces a new regex-syntax crate that provides a
regular expression parser and an abstract syntax for regular
expressions. As part of this effort, the parser has been rewritten and
has grown a substantial number of tests.

The regex crate itself hasn't changed too much. I opted for the
smallest possible delta to get it working with the new regex AST.
In most cases, this simplified code because it no longer has to deal
with unwieldy flags. (Instead, flag information is baked into the AST.)

Here is a list of public facing non-breaking changes:

A new regex-syntax crate with a parser, regex AST and lots of tests.
This closes [feature request] Expose libregex's parsing/compiling internals #29 and fixes Attempting to parse {2} causes panic #84.
A new flag, x, has been added. This allows one to write regexes with
insignificant whitespace and comments.
Repetition operators can now be directly applied to zero-width
matches. e.g., \b+ was previously not allowed but now works.
Note that one could always write (\b)+ previously. This change
is mostly about lifting an arbitrary restriction.

And a list of breaking changes:

A new Regex::with_size_limit constructor function, that allows one
to tweak the limit on the size of a compiled regex. This fixes impose a limit on the size of a compiled regex program #67.
The new method isn't a breaking change, but regexes that exceed the
size limit (set to 10MB by default) will no longer compile. To fix,
simply call Regex::with_size_limit with a bigger limit.
Capture group names cannot start with a number. This is a breaking
change because regexes that previously compiled (e.g., (?P<1a>.))
will now return an error. This fixes Underscore in replacement string treated as backspace #69.
The regex::Error type has been changed to reflect the better error
reporting in the regex-syntax crate, and a new error for limiting
regexes to a certain size. This is a breaking change. Most folks just
call unwrap() on Regex::new, so I expect this to have minimal
impact.

Closes #29, #67, #69, #79, #84.

[breaking-change]

This commit introduces a new `regex-syntax` crate that provides a regular expression parser and an abstract syntax for regular expressions. As part of this effort, the parser has been rewritten and has grown a substantial number of tests. The `regex` crate itself hasn't changed too much. I opted for the smallest possible delta to get it working with the new regex AST. In most cases, this simplified code because it no longer has to deal with unwieldy flags. (Instead, flag information is baked into the AST.) Here is a list of public facing non-breaking changes: * A new `regex-syntax` crate with a parser, regex AST and lots of tests. This closes #29 and fixes #84. * A new flag, `x`, has been added. This allows one to write regexes with insignificant whitespace and comments. * Repetition operators can now be directly applied to zero-width matches. e.g., `\b+` was previously not allowed but now works. Note that one could always write `(\b)+` previously. This change is mostly about lifting an arbitrary restriction. And a list of breaking changes: * A new `Regex::with_size_limit` constructor function, that allows one to tweak the limit on the size of a compiled regex. This fixes #67. The new method isn't a breaking change, but regexes that exceed the size limit (set to 10MB by default) will no longer compile. To fix, simply call `Regex::with_size_limit` with a bigger limit. * Capture group names cannot start with a number. This is a breaking change because regexes that previously compiled (e.g., `(?P<1a>.)`) will now return an error. This fixes #69. * The `regex::Error` type has been changed to reflect the better error reporting in the `regex-syntax` crate, and a new error for limiting regexes to a certain size. This is a breaking change. Most folks just call `unwrap()` on `Regex::new`, so I expect this to have minimal impact. Closes #29, #67, #69, #79, #84. [breaking-change]

rust-highfive · 2015-05-25T16:53:37Z

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

BurntSushi · 2015-05-25T16:59:19Z

cc @alexcrichton @huonw @andrew-d

I think the thing I am most concerned about here is the definition of the Error type. It is a publicly exposed enum that I could envision changing in the future. (For example, if the regex-syntax crate grew to accept a superset of the syntax supported by regex, then there would need to be an additional error for rejecting unsupported syntax.) My instinct is that I should not expose the error as an enum and instead provide methods. (e.g., fn too_big(&self) -> Option<usize> and fn syntax_error(&self) -> Option<&regex_syntax::Error>.)

Also, part of the enum exposes the regex_syntax::Error type, which could also evolve. I think this means that if there's a breaking change in regex_syntax::{Error, ErrorKind}, then the major version number of regex would also need to be bumped. Not sure how to deal with this one.

alexcrichton · 2015-05-27T18:53:32Z

Cargo.toml

@@ -21,8 +21,16 @@ path = "regex_macros/benches/bench_dynamic.rs"
 test = false
 bench = true

+[dependencies.regex-syntax]
+path = "regex_syntax"
+version = "*"


I personally prefer to write out this section like so nowadays:

[dependencies] regex-syntax = { path = "regex_syntax", version = "*" }

Also, perhaps this could have a more strict version requirement than *? And finally, perhaps the folder could be called regex-syntax to match the crate name?

And finally, perhaps the folder could be called regex-syntax to match the crate name?

Happy to oblige. I originally had that, but it looks very weird next to regex_macros, which I think is stuck with an underscore? :-(

👍 to everything else!

Yeah unfortunately we can't switch underscore-to-dash after the fact just yet :(

I would like to enable this kind of feature on crates.io though as I suspect it comes up more often than not!

OK, hypens it is!

alexcrichton · 2015-05-27T19:03:19Z

This looks awesome, thanks so much @BurntSushi!

And a list of breaking changes:

These all seem minor enough that I would be fine releasing this version as a minor update to this library (not a major version change).

I think the thing I am most concerned about here is the definition of the Error type. It is a publicly exposed enum that I could envision changing in the future.

I personally like having an enum for errors like this, and I think that they should be considered "defacto extensible" in the sense that although it's a breaking change to add a variant, I would not consider it one. You could add a #[doc(hidden)] variant named something like __DoNotMatchMeIfYouWishToNotBreak which may also deter exhaustive matches.

Also, part of the enum exposes the regex_syntax::Error type, which could also evolve. I think this means that if there's a breaking change in regex_syntax::{Error, ErrorKind}, then the major version number of regex would also need to be bumped. Not sure how to deal with this one.

Hm, this is interesting! I would believe that there's definitely a good deal of API leakage between crates in terms of a major version bump of your deps may require a major version bump for you, but I think with a dependency like 0.1.0 on regex-syntax that this crate will be safe. That basically means that regex 0.1.x will use the 0.1.x version of syntax, and it can evolve to a more updated syntax later if necessary.

alexcrichton · 2015-05-27T19:04:52Z

Cargo.toml

 [dev-dependencies]
 rand = "0.3"

 [features]
 pattern = []
+
+[profile.bench]
+opt-level = 3


I think that 3 is the default, so perhaps this could just be specified to set lto = true

The big change here is the addition of a non-public variant in the error enums. This will hint to users that one shouldn't exhaustively match the enums in case new variants are added.

BurntSushi · 2015-05-27T22:44:18Z

I think I've addressed everything. Just followed all of your suggestions. :-) I can merge once Travis gives the OK!

alexcrichton · 2015-05-27T23:02:09Z

Sounds good to me!

Rewrite parser as part of new regex-syntax crate.

frewsxcv · 2015-05-28T00:20:06Z

Welp, time to AFL :)

BurntSushi · 2015-05-28T00:21:22Z

Haha. Do your worst. This time, quickcheck has my back. :P

BurntSushi assigned alexcrichton May 25, 2015

rust-highfive assigned huonw and unassigned alexcrichton May 25, 2015

Test regex-syntax crate in Travis.

69e242d

alexcrichton reviewed May 27, 2015
View reviewed changes

Fixes from code review.

6d5e909

The big change here is the addition of a non-public variant in the error enums. This will hint to users that one shouldn't exhaustively match the enums in case new variants are added.

s/regex_syntax/regex-syntax

068514f

BurntSushi added a commit that referenced this pull request May 27, 2015

Merge pull request #87 from rust-lang/new-parser

3ba38c4

Rewrite parser as part of new regex-syntax crate.

BurntSushi merged commit 3ba38c4 into master May 27, 2015

BurntSushi mentioned this pull request May 27, 2015

Optimize case_fold_and_combine_ranges? #79

Closed

BurntSushi deleted the new-parser branch May 28, 2015 14:21

BurntSushi unassigned huonw Sep 21, 2015

BurntSushi mentioned this pull request Jul 24, 2019

Invalid capture group name doesn't fail #602

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite parser as part of new regex-syntax crate. #87

Rewrite parser as part of new regex-syntax crate. #87

BurntSushi commented May 25, 2015

rust-highfive commented May 25, 2015

BurntSushi commented May 25, 2015

alexcrichton May 27, 2015

BurntSushi May 27, 2015

alexcrichton May 27, 2015

BurntSushi May 27, 2015

alexcrichton commented May 27, 2015

alexcrichton May 27, 2015

BurntSushi commented May 27, 2015

alexcrichton commented May 27, 2015

frewsxcv commented May 28, 2015

BurntSushi commented May 28, 2015

Rewrite parser as part of new regex-syntax crate. #87

Rewrite parser as part of new regex-syntax crate. #87

Conversation

BurntSushi commented May 25, 2015

rust-highfive commented May 25, 2015

BurntSushi commented May 25, 2015

alexcrichton May 27, 2015

Choose a reason for hiding this comment

BurntSushi May 27, 2015

Choose a reason for hiding this comment

alexcrichton May 27, 2015

Choose a reason for hiding this comment

BurntSushi May 27, 2015

Choose a reason for hiding this comment

alexcrichton commented May 27, 2015

alexcrichton May 27, 2015

Choose a reason for hiding this comment

BurntSushi commented May 27, 2015

alexcrichton commented May 27, 2015

frewsxcv commented May 28, 2015

BurntSushi commented May 28, 2015