[PATCH] D27318: Support escaping in TrigramIndex.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc 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.
----------------
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"));
+}
+
 }
```


https://reviews.llvm.org/D27318