'this == NULL' check smells bad

[GoogleCodeExporter]

Clang's -Wtautological-undefined-compare flag reports several instances of 
'this == NULL' comparison in re2 code (see below).
These checks are either unnecessary (because 'this' cannot be NULL) or denote 
that some code is calling non-static methods via the NULL pointer, which is 
undefined behavior.

FAILED: /path/to/clang++ -MMD -MF obj/third_party/re2/re2/re2.prefilter.o.d 
-DV8_DEPRECATION_WARNINGS -DBLINK_SCALE_FILTERS_AT_RECORD_TIME 
-D_FILE_OFFSET_BITS=64 -DNO_TCMALLOC -DDISABLE_NACL -DCHROMIUM_BUILD 
-DCR_CLANG_REVISION=209387 -DTOOLKIT_VIEWS=1 -DUI_COMPOSITOR_IMAGE_TRANSPORT 
-DUSE_AURA=1 -DUSE_ASH=1 -DUSE_CAIRO=1 -DUSE_GLIB=1 
-DUSE_DEFAULT_RENDER_THEME=1 -DUSE_LIBJPEG_TURBO=1 -DUSE_X11=1 
-DUSE_CLIPBOARD_AURAX11=1 -DENABLE_ONE_CLICK_SIGNIN -DUSE_XI2_MT=2 
-DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_PEPPER_CDMS 
-DENABLE_CONFIGURATION_POLICY -DENABLE_NOTIFICATIONS -DUSE_UDEV 
-DENABLE_EGLIMAGE=1 -DENABLE_TASK_MANAGER=1 -DENABLE_EXTENSIONS=1 
-DENABLE_PLUGINS=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 
-DENABLE_AUTOFILL_DIALOG=1 -DENABLE_BACKGROUND=1 -DENABLE_GOOGLE_NOW=1 
-DCLD_VERSION=2 -DENABLE_FULL_PRINTING=1 -DENABLE_PRINTING=1 
-DENABLE_SPELLCHECK=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_APP_LIST=1 
-DENABLE_SETTINGS_APP=1 -DENABLE_MANAGED_USERS=1 -DENABLE_MDNS=1 
-DENABLE_SERVICE_DISCOVERY=1 -DUSE_NSS=1 -DMEMORY_TOOL_REPLACES_ALLOCATOR 
-DTHREAD_SANITIZER -DDYNAMIC_ANNOTATIONS_EXTERNAL_IMPL=1 
-DWTF_USE_DYNAMIC_ANNOTATIONS_NOIMPL=1 -DNDEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 
-DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../../third_party/re2 -I../.. 
'-I../../third_party/libc++/trunk/include' 
'-I../../third_party/libc++abi/trunk/include' -fstack-protector 
--param=ssp-buffer-size=4 -Werror -pthread -fno-exceptions -fno-strict-aliasing 
-Wno-unused-parameter -Wno-missing-field-initializers -fvisibility=hidden -pipe 
-fPIC -Wheader-hygiene -Wno-char-subscripts -Wno-unneeded-internal-declaration 
-Wno-covered-switch-default -Wstring-conversion -Wno-c++11-narrowing 
-Wno-reserved-user-defined-literal -Wno-deprecated-register -fcolor-diagnostics 
-B/another/path/to/binutils/Linux_x64/Release/bin -Wno-format 
-Wno-unused-result -m64 -march=x86-64 -fno-omit-frame-pointer 
-gline-tables-only -fsanitize=thread -fPIC 
-fsanitize-blacklist=../../tools/valgrind/tsan_v2/ignores.txt -O2 
-fdata-sections -ffunction-sections -funwind-tables -fno-rtti 
-fno-threadsafe-statics -fvisibility-inlines-hidden -std=gnu++11 
-Wno-deprecated -nostdinc++  -c ../../third_party/re2/re2/prefilter.cc -o 
obj/third_party/re2/re2/re2.prefilter.o
../../third_party/re2/re2/prefilter.cc:268:7: error: 'this' pointer cannot be 
null in well-defined C++ code; comparison may be assumed to always evaluate to 
false [-Werror,-Wtautological-undefined-compare]
  if (this == NULL) {
      ^~~~    ~~~~
../../third_party/re2/re2/prefilter.cc:668:7: error: 'this' pointer cannot be 
null in well-defined C++ code; comparison may be assumed to always evaluate to 
false [-Werror,-Wtautological-undefined-compare]
  if (this == NULL)
      ^~~~    ~~~~
2 errors generated.
FAILED: /path/to/clang++ -MMD -MF obj/third_party/re2/re2/re2.regexp.o.d 
-DV8_DEPRECATION_WARNINGS -DBLINK_SCALE_FILTERS_AT_RECORD_TIME 
-D_FILE_OFFSET_BITS=64 -DNO_TCMALLOC -DDISABLE_NACL -DCHROMIUM_BUILD 
-DCR_CLANG_REVISION=209387 -DTOOLKIT_VIEWS=1 -DUI_COMPOSITOR_IMAGE_TRANSPORT 
-DUSE_AURA=1 -DUSE_ASH=1 -DUSE_CAIRO=1 -DUSE_GLIB=1 
-DUSE_DEFAULT_RENDER_THEME=1 -DUSE_LIBJPEG_TURBO=1 -DUSE_X11=1 
-DUSE_CLIPBOARD_AURAX11=1 -DENABLE_ONE_CLICK_SIGNIN -DUSE_XI2_MT=2 
-DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_PEPPER_CDMS 
-DENABLE_CONFIGURATION_POLICY -DENABLE_NOTIFICATIONS -DUSE_UDEV 
-DENABLE_EGLIMAGE=1 -DENABLE_TASK_MANAGER=1 -DENABLE_EXTENSIONS=1 
-DENABLE_PLUGINS=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 
-DENABLE_AUTOFILL_DIALOG=1 -DENABLE_BACKGROUND=1 -DENABLE_GOOGLE_NOW=1 
-DCLD_VERSION=2 -DENABLE_FULL_PRINTING=1 -DENABLE_PRINTING=1 
-DENABLE_SPELLCHECK=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_APP_LIST=1 
-DENABLE_SETTINGS_APP=1 -DENABLE_MANAGED_USERS=1 -DENABLE_MDNS=1 
-DENABLE_SERVICE_DISCOVERY=1 -DUSE_NSS=1 -DMEMORY_TOOL_REPLACES_ALLOCATOR 
-DTHREAD_SANITIZER -DDYNAMIC_ANNOTATIONS_EXTERNAL_IMPL=1 
-DWTF_USE_DYNAMIC_ANNOTATIONS_NOIMPL=1 -DNDEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 
-DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../../third_party/re2 -I../.. 
'-I../../third_party/libc++/trunk/include' 
'-I../../third_party/libc++abi/trunk/include' -fstack-protector 
--param=ssp-buffer-size=4 -Werror -pthread -fno-exceptions -fno-strict-aliasing 
-Wno-unused-parameter -Wno-missing-field-initializers -fvisibility=hidden -pipe 
-fPIC -Wheader-hygiene -Wno-char-subscripts -Wno-unneeded-internal-declaration 
-Wno-covered-switch-default -Wstring-conversion -Wno-c++11-narrowing 
-Wno-reserved-user-defined-literal -Wno-deprecated-register -fcolor-diagnostics 
-B/another/path/to/binutils/Linux_x64/Release/bin -Wno-format 
-Wno-unused-result -m64 -march=x86-64 -fno-omit-frame-pointer 
-gline-tables-only -fsanitize=thread -fPIC 
-fsanitize-blacklist=../../tools/valgrind/tsan_v2/ignores.txt -O2 
-fdata-sections -ffunction-sections -funwind-tables -fno-rtti 
-fno-threadsafe-statics -fvisibility-inlines-hidden -std=gnu++11 
-Wno-deprecated -nostdinc++  -c ../../third_party/re2/re2/regexp.cc -o 
obj/third_party/re2/re2/re2.regexp.o
../../third_party/re2/re2/regexp.cc:876:7: error: 'this' pointer cannot be null 
in well-defined C++ code; comparison may be assumed to always evaluate to false 
[-Werror,-Wtautological-undefined-compare]
  if (this == NULL)
      ^~~~    ~~~~

Original issue reported on code.google.com by gli...@chromium.org on 9 Jun 2014 at 9:03

[GoogleCodeExporter]

Original comment by gli...@chromium.org on 9 Jun 2014 at 9:05

[GoogleCodeExporter]

Patch: https://codereview.appspot.com/107100043/

Original comment by thakis@chromium.org on 13 Jun 2014 at 11:37

[GoogleCodeExporter]

Fixed: https://code.google.com/p/re2/source/detail?r=b92ce81f1e25

Original comment by thakis@chromium.org on 30 Jun 2014 at 4:42

[GoogleCodeExporter]

Original comment by thakis@chromium.org on 7 Sep 2014 at 2:36

• Changed state: Fixed