Skip to content

Commit

Permalink
syntax: fix panics that occur with non-sensical Ast values
Browse files Browse the repository at this point in the history
These panics I do not believe can occur from an actual pattern, since
the parser will either never produce such things or will return an
error. But still, the Ast->Hir translator shouldn't panic in such cases.

Actually, the non-sensical Ast values are actually somewhat sensible,
and they don't map to invalid regexes. These panics were likely the
result of the regex crate not supporting empty patterns or "fail"
patterns particularly well in the fast. But now that we do, we can just
let the Asts through and generate the Hir you'd expect.

Fixes #1047
  • Loading branch information
BurntSushi committed Oct 8, 2023
1 parent 5070021 commit 52140fa
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 4 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ Bug fixes:
* [BUG #1046](https://github.com/rust-lang/regex/issues/1046):
Fix a bug that could result in incorrect match spans when using a Unicode word
boundary and searching non-ASCII strings.
* [BUG(regex-syntax) #1047](https://github.com/rust-lang/regex/issues/1047):
Fix panics that can occur in `Ast->Hir` translation (not reachable from `regex`
crate).
* [BUG(regex-syntax) #1088](https://github.com/rust-lang/regex/issues/1088):
Remove guarantees in the API that connect the `u` flag with a specific HIR
representation.
Expand Down
59 changes: 55 additions & 4 deletions regex-syntax/src/hir/translate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,14 +354,14 @@ impl<'t, 'p> Visitor for TranslatorI<'t, 'p> {
.unwrap_or_else(|| self.flags());
self.push(HirFrame::Group { old_flags });
}
Ast::Concat(ref x) if x.asts.is_empty() => {}
Ast::Concat(_) => {
self.push(HirFrame::Concat);
}
Ast::Alternation(ref x) if x.asts.is_empty() => {}
Ast::Alternation(_) => {
Ast::Alternation(ref x) => {
self.push(HirFrame::Alternation);
self.push(HirFrame::AlternationBranch);
if !x.asts.is_empty() {
self.push(HirFrame::AlternationBranch);
}
}
_ => {}
}
Expand Down Expand Up @@ -3652,4 +3652,55 @@ mod tests {
]),
);
}

#[test]
fn regression_alt_empty_concat() {
use crate::ast::{self, Ast};

let span = Span::splat(Position::new(0, 0, 0));
let ast = Ast::alternation(ast::Alternation {
span,
asts: vec![Ast::concat(ast::Concat { span, asts: vec![] })],
});

let mut t = Translator::new();
assert_eq!(Ok(Hir::empty()), t.translate("", &ast));
}

#[test]
fn regression_empty_alt() {
use crate::ast::{self, Ast};

let span = Span::splat(Position::new(0, 0, 0));
let ast = Ast::concat(ast::Concat {
span,
asts: vec![Ast::alternation(ast::Alternation {
span,
asts: vec![],
})],
});

let mut t = Translator::new();
assert_eq!(Ok(Hir::fail()), t.translate("", &ast));
}

#[test]
fn regression_singleton_alt() {
use crate::{
ast::{self, Ast},
hir::Dot,
};

let span = Span::splat(Position::new(0, 0, 0));
let ast = Ast::concat(ast::Concat {
span,
asts: vec![Ast::alternation(ast::Alternation {
span,
asts: vec![Ast::dot(span)],
})],
});

let mut t = Translator::new();
assert_eq!(Ok(Hir::dot(Dot::AnyCharExceptLF)), t.translate("", &ast));
}
}

0 comments on commit 52140fa

Please sign in to comment.