[PATCH] D27318: Support escaping in TrigramIndex.
Ivan Krasin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 2 12:23:18 PST 2016
krasin added inline comments.
================
Comment at: lib/Support/TrigramIndex.cpp:60-68
+ case 'n':
+ Char = '\n';
+ break;
+ case 't':
+ Char = '\t';
+ break;
+ // Decimal escapes are backreferences.
----------------
pcc wrote:
> krasin wrote:
> > pcc wrote:
> > > This part doesn't look right. Neither 'n', 't' nor '0' are special. See: http://llvm-cs.pcc.me.uk/lib/Support/regcomp.c#379
> > >
> > > I confirmed it with this passing test:
> > > ```
> > > diff --git a/llvm/unittests/Support/RegexTest.cpp b/llvm/unittests/Support/RegexTest.cpp
> > > index 5e3ce39..e151f47 100644
> > > --- a/llvm/unittests/Support/RegexTest.cpp
> > > +++ b/llvm/unittests/Support/RegexTest.cpp
> > > @@ -171,4 +171,16 @@ TEST_F(RegexTest, MatchInvalid) {
> > > EXPECT_FALSE(r1.match("X"));
> > > }
> > >
> > > +TEST_F(RegexTest, foo) {
> > > + Regex r1("\\t");
> > > + EXPECT_TRUE(r1.match("t"));
> > > + EXPECT_FALSE(r1.match("\t"));
> > > + Regex r2("\\n");
> > > + EXPECT_TRUE(r2.match("n"));
> > > + EXPECT_FALSE(r2.match("\n"));
> > > + Regex r3("\\0");
> > > + EXPECT_TRUE(r3.match("0"));
> > > + EXPECT_FALSE(r3.match("x"));
> > > +}
> > > +
> > > }
> > >
> > > ```
> > You're right. Thank you for the catch. Fixed.
> What about '1' through '9'?
> ```
> diff --git a/llvm/unittests/Support/RegexTest.cpp b/llvm/unittests/Support/RegexTest.cpp
> index 5e3ce39..bc4995f 100644
> --- a/llvm/unittests/Support/RegexTest.cpp
> +++ b/llvm/unittests/Support/RegexTest.cpp
> @@ -171,4 +171,19 @@ TEST_F(RegexTest, MatchInvalid) {
> EXPECT_FALSE(r1.match("X"));
> }
>
> +TEST_F(RegexTest, foo) {
> + Regex r1("\\t");
> + EXPECT_TRUE(r1.match("t"));
> + EXPECT_FALSE(r1.match("\t"));
> + Regex r2("\\n");
> + EXPECT_TRUE(r2.match("n"));
> + EXPECT_FALSE(r2.match("\n"));
> + Regex r3("\\0");
> + EXPECT_TRUE(r3.match("0"));
> + EXPECT_FALSE(r3.match("x"));
> + Regex r4("(a)\\1");
> + EXPECT_TRUE(r4.match("aa"));
> + EXPECT_FALSE(r4.match("a1"));
> +}
> +
> }
> ```
I have added \1 to \9 back, and also added tests for these cases.
https://reviews.llvm.org/D27318
More information about the llvm-commits
mailing list