commit 1aef6f55a9866e02901fb1a565f53373d3655646
author Nico Weber <thakis@chromium.org> Thu Jun 26 11:18:57 2014 -0400
committer Russ Cox <rsc@swtch.com> Thu Jun 26 11:18:57 2014 -0400
tree cfa2c781495c09b23f2f48f3d2d888c813555465
parent 7bd3b3b02d1f9d72bf1a421fbc21c0e67f1d936d

Remove comparisons of this with NULL.

Calling functions on objects that are NULL is undefined behavior.
Hence, new versions of clang assume that `this` is never NULL,
warn on comparisons of `this` with NULL, and optimize the comparison away.

There were three comparisons of `this` with NULL in re2:

1. CharClass::Delete(). There are only two callers of this method, and
   as far as I can tell `this` can't be NULL there, so just delete that check.
2. Prefilter::DebugString(). This too has two callers: Prefilter::Info::ToString(),
   which already checks for NULL before calling, and DebugString() itself.
   Since several places check Prefilters for NULL, add explicit checks before
   calling here.
3. Prefilter::Info::ToString(). This has three callers. In 2 cases this can't be
   NULL. In the third case it could conceivably be NULL if op() is
   kRegexpConcat and nchild_args is 0, so add an explicit check here.

Fixes https://code.google.com/p/re2/issues/detail?id=115

LGTM=rsc
R=rsc
CC=re2-dev
https://codereview.appspot.com/107100043

CONTRIBUTORS

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 2a694bc..1a1c848 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -31,6 +31,7 @@
 Dmitriy Vyukov <dvyukov@google.com>
 John Millikin <jmillikin@gmail.com>
 Mike Nazarewicz <mpn@google.com>
+Nico Weber <thakis@chromium.org>
 Pawel Hajdan <phajdan.jr@gmail.com>
 Rob Pike <r@google.com>
 Russ Cox <rsc@swtch.com>

re2/prefilter.cc

diff --git a/re2/prefilter.cc b/re2/prefilter.cc
index 1b12458..f192f9b 100644
--- a/re2/prefilter.cc
+++ b/re2/prefilter.cc
@@ -265,14 +265,6 @@
 
 // Format a Info in string form.
 string Prefilter::Info::ToString() {
-  if (this == NULL) {
-    // Sometimes when iterating on children of a node,
-    // some children might have NULL Info. Adding
-    // the check here for NULL to take care of cases where
-    // the caller is not checking.
-    return "";
-  }
-
   if (is_exact_) {
     int n = 0;
     string s;
@@ -640,7 +632,7 @@
 
   if (Trace) {
     VLOG(0) << "BuildInfo " << re->ToString()
-            << ": " << info->ToString();
+            << ": " << (info ? info->ToString() : "");
   }
 
   return info;
@@ -665,9 +657,6 @@
 }
 
 string Prefilter::DebugString() const {
-  if (this == NULL)
-    return "<nil>";
-
   switch (op_) {
     default:
       LOG(DFATAL) << "Bad op in Prefilter::DebugString: " << op_;
@@ -683,7 +672,8 @@
       for (int i = 0; i < subs_->size(); i++) {
         if (i > 0)
           s += " ";
-        s += (*subs_)[i]->DebugString();
+        Prefilter* sub = (*subs_)[i];
+        s += sub ? sub->DebugString() : "<nil>";
       }
       return s;
     }
@@ -692,7 +682,8 @@
       for (int i = 0; i < subs_->size(); i++) {
         if (i > 0)
           s += "|";
-        s += (*subs_)[i]->DebugString();
+        Prefilter* sub = (*subs_)[i];
+        s += sub ? sub->DebugString() : "<nil>";
       }
       s += ")";
       return s;

re2/regexp.cc

diff --git a/re2/regexp.cc b/re2/regexp.cc
index a74ceec..ed4c3a0 100644
--- a/re2/regexp.cc
+++ b/re2/regexp.cc
@@ -873,8 +873,6 @@
 }
 
 void CharClass::Delete() {
-  if (this == NULL)
-    return;
   uint8 *data = reinterpret_cast<uint8*>(this);
   delete[] data;
 }