commit fcdcf25d1a3209355411c57c342e8db115fc9582
author Paul Wankadia <junyer@google.com> Sat Aug 01 13:00:00 2015 +1000
committer Paul Wankadia <junyer@google.com> Sat Aug 01 03:10:18 2015 +0000
tree 9624dd07f5e55939549e86add68cda90119f8af4
parent 838bc2baee14e354eb10d797d5f684f9d8dddb64

Remove the capture name copies from the coalesce and simplify logic.

The capture name isn't needed for compilation and checking that it was
copied doesn't mean that the capture number was copied. Unfortunately,
neither ToString() nor Dump() surfaces the capture number, so I've set
"tripwires" to catch kRegexpCapture cap() == 0 in debug binary tests.

Change-Id: I0737d6aaef6fbb3138df95d697ca110e51d8b029
Reviewed-on: https://code-review.googlesource.com/3275
Reviewed-by: Paul Wankadia <junyer@google.com>

re2/simplify.cc

diff --git a/re2/simplify.cc b/re2/simplify.cc
index aca86c2..d14483f 100644
--- a/re2/simplify.cc
+++ b/re2/simplify.cc
@@ -236,8 +236,6 @@
       nre->max_ = re->max();
     } else if (re->op() == kRegexpCapture) {
       nre->cap_ = re->cap();
-      if (re->name() != NULL)
-        nre->name_ = new string(*re->name());
     }
     return nre;
   }
@@ -497,8 +495,6 @@
       nre->AllocSub(1);
       nre->sub()[0] = newsub;
       nre->cap_ = re->cap();
-      if (re->name() != NULL)
-        nre->name_ = new string(*re->name());
       nre->simple_ = true;
       return nre;
     }

re2/testing/dump.cc

diff --git a/re2/testing/dump.cc b/re2/testing/dump.cc
index 4bdf714..9703039 100644
--- a/re2/testing/dump.cc
+++ b/re2/testing/dump.cc
@@ -120,6 +120,8 @@
       DumpRegexpAppending(re->sub()[0], s);
       break;
     case kRegexpCapture:
+      if (re->cap() == 0)
+        LOG(DFATAL) << "kRegexpCapture cap() == 0";
       if (re->name()) {
         s->append(*re->name());
         s->append(":");

re2/testing/simplify_test.cc

diff --git a/re2/testing/simplify_test.cc b/re2/testing/simplify_test.cc
index 7604530..9db41ee 100644
--- a/re2/testing/simplify_test.cc
+++ b/re2/testing/simplify_test.cc
@@ -220,15 +220,15 @@
   // Just for fun:
   { "aa*aa+aa?aa{2}aaa{2,}aaa{2,3}a", "aaaaaaaaaaaaaaaa+" },
 
-  // Regression test: During coalescing, the child of the repeat changes, so
-  // we build a new repeat. The new repeat must have the min and max of the
-  // old repeat. Failure to copy them results in min=0 and max=0.
+  // During coalescing, the child of the repeat changes, so we build a new
+  // repeat. The new repeat must have the min and max of the old repeat.
+  // Failure to copy them results in min=0 and max=0 -> empty match.
   { "(?:a*aab){2}", "aa+baa+b" },
 
-  // Regression test: During coalescing, the child of the capture changes, so
-  // we build a new capture. The new capture must have the cap and name of the
-  // old capture. Failure to copy them results in cap=0 and name=NULL.
-  { "(?P<name>a*aab)", "(?P<name>aa+b)" },
+  // During coalescing, the child of the capture changes, so we build a new
+  // capture. The new capture must have the cap of the old capture.
+  // Failure to copy it results in cap=0 -> ToString() logs a fatal error.
+  { "(a*aab)", "(aa+b)" },
 };
 
 TEST(TestSimplify, SimpleRegexps) {

re2/tostring.cc

diff --git a/re2/tostring.cc b/re2/tostring.cc
index 5438c67..c59d4d9 100644
--- a/re2/tostring.cc
+++ b/re2/tostring.cc
@@ -94,6 +94,8 @@
 
     case kRegexpCapture:
       t_->append("(");
+      if (re->cap() == 0)
+        LOG(DFATAL) << "kRegexpCapture cap() == 0";
       if (re->name()) {
         t_->append("?P<");
         t_->append(*re->name());