This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8506

rust-regex: fix issues with regex anchoring and performance
ClosedPublic

Authored by Alphare on May 7 2020, 6:16 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGad1ec40975aa: rust-regex: fix issues with regex anchoring and performance
rHGeb301282bacc: rust-regex: fix issues with regex anchoring and performance
Summary

It turns out that the way I tried to work around regex's behavior difference
with re2 and Python's re was 1) buggy and 2) much more complicated than
needed.

In a few words:
regex adds .* on either side of patterns when no start or end anchor is
present. My previous workaround put ^ or $ for every pattern, which is
wrong even without the other 2 bugs on top of it.
Using ^(?:<patterns>) right at the end of the regex path fixes the issue.
I've opened an issue to get a build option instead:
https://github.com/rust-lang/regex/issues/675

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Revision Contents
Changeset List

Diff 21368

rust/hg-core/src/filepatterns.rs

fn _build_single_regex(entry: &IgnorePattern) -> Vec<u8> { fn _build_single_regex(entry: &IgnorePattern) -> Vec<u8> {
let IgnorePattern { let IgnorePattern {
syntax, pattern, .. syntax, pattern, ..
} = entry; } = entry;
if pattern.is_empty() { if pattern.is_empty() {
return vec![]; return vec![];
} }
match syntax { match syntax {
// The `regex` crate adds `.*` to the start and end of expressions PatternSyntax::Regexp => pattern.to_owned(),
// if there are no anchors, so add them.
PatternSyntax::Regexp => [b"^", &pattern[..], b"$"].concat(),
PatternSyntax::RelRegexp => { PatternSyntax::RelRegexp => {
// The `regex` crate accepts `**` while `re2` and Python's `re` // The `regex` crate accepts `**` while `re2` and Python's `re`
// do not. Checking for `*` correctly triggers the same error all // do not. Checking for `*` correctly triggers the same error all
// engines. // engines.
if pattern[0] == b'^' || pattern[0] == b'*' { if pattern[0] == b'^' || pattern[0] == b'*' {
return pattern.to_owned(); return pattern.to_owned();
} }
[&b".*"[..], pattern].concat() [&b".*"[..], pattern].concat()
} }
PatternSyntax::Path | PatternSyntax::RelPath => { PatternSyntax::Path | PatternSyntax::RelPath => {
if pattern == b"." { if pattern == b"." {
return vec![]; return vec![];
} }
[escape_pattern(pattern).as_slice(), b"(?:/|$)"].concat() [escape_pattern(pattern).as_slice(), b"(?:/|$)"].concat()
} }
PatternSyntax::RootFiles => { PatternSyntax::RootFiles => {
let mut res = if pattern == b"." { let mut res = if pattern == b"." {
vec![b'^'] vec![]
} else { } else {
// Pattern is a directory name. // Pattern is a directory name.
[b"^", escape_pattern(pattern).as_slice(), b"/"].concat() [escape_pattern(pattern).as_slice(), b"/"].concat()
}; };
// Anything after the pattern must be a non-directory. // Anything after the pattern must be a non-directory.
res.extend(b"[^/]+$"); res.extend(b"[^/]+$");
res.push(b'$');
res res
} }
PatternSyntax::RelGlob => { PatternSyntax::RelGlob => {
let glob_re = glob_to_re(pattern); let glob_re = glob_to_re(pattern);
if let Some(rest) = glob_re.drop_prefix(b"[^/]*") { if let Some(rest) = glob_re.drop_prefix(b"[^/]*") {
[b".*", rest, GLOB_SUFFIX].concat() [b".*", rest, GLOB_SUFFIX].concat()
} else { } else {
[b"(?:.*/)?", glob_re.as_slice(), GLOB_SUFFIX].concat() [b"(?:.*/)?", glob_re.as_slice(), GLOB_SUFFIX].concat()
} }
} }
PatternSyntax::Glob | PatternSyntax::RootGlob => { PatternSyntax::Glob | PatternSyntax::RootGlob => {
[b"^", glob_to_re(pattern).as_slice(), GLOB_SUFFIX].concat() [glob_to_re(pattern).as_slice(), GLOB_SUFFIX].concat()
} }
PatternSyntax::Include | PatternSyntax::SubInclude => unreachable!(), PatternSyntax::Include | PatternSyntax::SubInclude => unreachable!(),
} }
} }
const GLOB_SPECIAL_CHARACTERS: [u8; 7] = const GLOB_SPECIAL_CHARACTERS: [u8; 7] =
[b'*', b'?', b'[', b']', b'{', b'}', b'\\']; [b'*', b'?', b'[', b']', b'{', b'}', b'\\'];

rust/hg-core/src/matchers.rs

/// underlying engine (the `regex` crate), for instance anything with /// underlying engine (the `regex` crate), for instance anything with
/// back-references. /// back-references.
#[timed] #[timed]
fn re_matcher( fn re_matcher(
pattern: &[u8], pattern: &[u8],
) -> PatternResult<impl Fn(&HgPath) -> bool + Sync> { ) -> PatternResult<impl Fn(&HgPath) -> bool + Sync> {
use std::io::Write; use std::io::Write;
let mut escaped_bytes = vec![]; // The `regex` crate adds `.*` to the start and end of expressions if there
// are no anchors, so add the start anchor.
let mut escaped_bytes = vec![b'^', b'(', b'?', b':'];
for byte in pattern { for byte in pattern {
if *byte > 127 { if *byte > 127 {
write!(escaped_bytes, "\\x{:x}", *byte).unwrap(); write!(escaped_bytes, "\\x{:x}", *byte).unwrap();
} else { } else {
escaped_bytes.push(*byte); escaped_bytes.push(*byte);
} }
} }
escaped_bytes.push(b')');
// Avoid the cost of UTF8 checking // Avoid the cost of UTF8 checking
// //
// # Safety // # Safety
// This is safe because we escaped all non-ASCII bytes. // This is safe because we escaped all non-ASCII bytes.
let pattern_string = unsafe { String::from_utf8_unchecked(escaped_bytes) }; let pattern_string = unsafe { String::from_utf8_unchecked(escaped_bytes) };
let re = regex::bytes::RegexBuilder::new(&pattern_string) let re = regex::bytes::RegexBuilder::new(&pattern_string)
.unicode(false) .unicode(false)