Remove tautological undefined comparisons

A "tautological" comparison is a comparison that's only meaningfully
necessary when we consider "undefined" C++ behaviors.

For context, in well-formed C++ code:

  1. references can never be bound to NULL; and
  1. the `this` pointer in a member function can never be NULL

Historically ORCA has relied on implementation-specific (undefined,
actually) behavior where

  1. we might call a member function on a potentially NULL object with
  the `->` operator, or
  1. some callers may bind a (possibly-NULL) pointer to a reference with
  the '*' operator and try to print it into an IOStream

While doing so gives the benefit of centralizing the check, a dependence
on undefined behavior means we risk producing the wrong code. Indeed,
more modern compilers aggressively optimize against undefined behaviors:
e.g. by eliminating the `NULL` checks, or assuming the variable used for
indexing an array is never out of bound.

Commit ee5ef334 is a "tick" towards
reducing such undefined comparisons. This commit is the "tock" that
eliminates them.

For more context GCC 6+ chokes without this:

Running on a macOS iMac:

```
env CC='gcc-6' CXX='g++-6' cmake -GNinja -DCMAKE_BUILD_TYPE=Debug -H. -Bbuild.gcc6.debug
ninja -C build.gcc6.debug
```

GCC produces this error:

```
ninja: Entering directory `build.gcc6.debug'
[548/1027] Building CXX object libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEMap.cpp.o
FAILED: libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEMap.cpp.o
ccache /usr/local/bin/g++-6  -Dgpopt_EXPORTS -I/usr/local/include -I../libgpos/include -I../libgpopt/include -I../libgpopt/../libgpcost/include -I../libgpopt/../libnaucrates/include -Ilibgpos/include -Wall -Werror -Wextra -pedantic-errors -Wno-variadic-macros -Wno-tautological-undefined-compare -fno-omit-frame-pointer -g -g3 -fPIC -MD -MT libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEMap.cpp.o -MF libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEMap.cpp.o.d -o libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEMap.cpp.o -c ../libgpopt/src/base/CCTEMap.cpp
../libgpopt/src/base/CCTEMap.cpp: In function 'gpos::IOstream& gpopt::operator<<(gpos::IOstream&, gpopt::CCTEMap&)':
../libgpopt/src/base/CCTEMap.cpp:432:18: error: the compiler can assume that the address of 'cm' will never be NULL [-Werror=address]
     return (NULL == &cm) ? os : cm.OsPrint(os);
                  ^
At global scope:
cc1plus: all warnings being treated as errors
[549/1027] Building CXX object libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEReq.cpp.o
FAILED: libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEReq.cpp.o
ccache /usr/local/bin/g++-6  -Dgpopt_EXPORTS -I/usr/local/include -I../libgpos/include -I../libgpopt/include -I../libgpopt/../libgpcost/include -I../libgpopt/../libnaucrates/include -Ilibgpos/include -Wall -Werror -Wextra -pedantic-errors -Wno-variadic-macros -Wno-tautological-undefined-compare -fno-omit-frame-pointer -g -g3 -fPIC -MD -MT libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEReq.cpp.o -MF libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEReq.cpp.o.d -o libgpopt/CMakeFiles/gpopt.dir/src/base/CCTEReq.cpp.o -c ../libgpopt/src/base/CCTEReq.cpp
../libgpopt/src/base/CCTEReq.cpp: In function 'gpos::IOstream& gpopt::operator<<(gpos::IOstream&, gpopt::CCTEReq&)':
../libgpopt/src/base/CCTEReq.cpp:569:18: error: the compiler can assume that the address of 'cter' will never be NULL [-Werror=address]
     return (NULL == &cter) ? os : cter.OsPrint(os);
                  ^
At global scope:
cc1plus: all warnings being treated as errors
[557/1027] Linking CXX shared library libnaucrates/libnaucrates.2.47.0.dylib
ninja: build stopped: subcommand failed.
```

CMakeLists.txt

 # Copyright (c) 2015, Pivotal Software, Inc.
 
-cmake_minimum_required(VERSION 3.0 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
 project(gpopt LANGUAGES CXX C)
 
+set(CMAKE_CXX_STANDARD 98)
 set(GPORCA_VERSION_MAJOR 2)
 set(GPORCA_VERSION_MINOR 48)
 set(GPORCA_VERSION_PATCH 0)
@@ -67,16 +68,6 @@ if (COMPILER_HAS_WNO_VARIADIC_MACROS)
   SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-variadic-macros")
 endif()
 
-# Turn off warning about tautological comparisons with NULL in some ill-formed
-# Orca code.
-# TODO(chasseur): This warning should be reinstated once the code in question
-# has been modified to no longer depend on undefined behavior.
-check_cxx_compiler_flag("-Wno-tautological-undefined-compare"
-                        COMPILER_HAS_WNO_TAUTOLOGICAL_UNDEFINED_COMPARE)
-if (COMPILER_HAS_WNO_TAUTOLOGICAL_UNDEFINED_COMPARE)
-  SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-tautological-undefined-compare")
-endif()
-
 # Generate maximum detail for DEBUG information with -g3 if available.
 check_cxx_compiler_flag("-g3" COMPILER_HAS_G3)
 if (COMPILER_HAS_G3)

libgpopt/src/base/CCTEMap.cpp

@@ -424,12 +424,7 @@ namespace gpopt {
 
   IOstream &operator << (IOstream &os, CCTEMap &cm)
   {
-    // FIXME(chasseur): in well-formed C++ code, references can never be bound
-    // to NULL; however, some callers may dereference a (possibly-NULL) pointer
-    // with the '*' operator and try to print it into an IOStream; callers
-    // should be modified to explicitly do NULL-checks on pointers so that this
-    // function does not rely on undefined behavior
-    return (NULL == &cm) ? os : cm.OsPrint(os);
+    return cm.OsPrint(os);
   }
 
 }

libgpopt/src/base/CCTEReq.cpp

@@ -561,12 +561,7 @@ namespace gpopt {
 
   IOstream &operator << (IOstream &os, CCTEReq &cter)
   {
-    // FIXME(chasseur): in well-formed C++ code, references can never be bound
-    // to NULL; however, some callers may dereference a (possibly-NULL) pointer
-    // with the '*' operator and try to print it into an IOStream; callers
-    // should be modified to explicitly do NULL-checks on pointers so that this
-    // function does not rely on undefined behavior
-    return (NULL == &cter) ? os : cter.OsPrint(os);
+    return cter.OsPrint(os);
   }
 
 }

libgpopt/src/base/CDrvdProp.cpp

@@ -29,12 +29,7 @@ namespace gpopt {
 
   IOstream &operator << (IOstream &os, CDrvdProp &drvdprop)
   {
-    // FIXME(chasseur): in well-formed C++ code, references can never be bound
-    // to NULL; however, some callers may dereference a (possibly-NULL) pointer
-    // with the '*' operator and try to print it into an IOStream; callers
-    // should be modified to explicitly do NULL-checks on pointers so that this
-    // function does not rely on undefined behavior
-    return (NULL == &drvdprop) ? os : drvdprop.OsPrint(os);
+    return drvdprop.OsPrint(os);
   }
 
 }

libgpopt/src/base/CDrvdPropCtxt.cpp

@@ -17,12 +17,7 @@ namespace gpopt {
 
   IOstream &operator << (IOstream &os, CDrvdPropCtxt &drvdpropctxt)
   {
-    // FIXME(chasseur): in well-formed C++ code, references can never be bound
-    // to NULL; however, some callers may dereference a (possibly-NULL) pointer
-    // with the '*' operator and try to print it into an IOStream; callers
-    // should be modified to explicitly do NULL-checks on pointers so that this
-    // function does not rely on undefined behavior
-    return (NULL == &drvdpropctxt) ? os : drvdpropctxt.OsPrint(os);
+    return drvdpropctxt.OsPrint(os);
   }
 
 }

libgpopt/src/base/CEnfdProp.cpp

@@ -17,12 +17,7 @@ namespace gpopt {
 
   IOstream &operator << (IOstream &os, CEnfdProp &efdprop)
   {
-    // FIXME(chasseur): in well-formed C++ code, references can never be bound
-    // to NULL; however, some callers may dereference a (possibly-NULL) pointer
-    // with the '*' operator and try to print it into an IOStream; callers
-    // should be modified to explicitly do NULL-checks on pointers so that this
-    // function does not rely on undefined behavior
-    return (NULL == &efdprop) ? os : efdprop.OsPrint(os);
+    return efdprop.OsPrint(os);
   }
 
 }

libgpopt/src/base/CPartIndexMap.cpp

@@ -1066,12 +1066,7 @@ namespace gpopt {
 
   IOstream &operator << (IOstream &os, CPartIndexMap &pim)
   {
-    // FIXME(chasseur): in well-formed C++ code, references can never be bound
-    // to NULL; however, some callers may dereference a (possibly-NULL) pointer
-    // with the '*' operator and try to print it into an IOStream; callers
-    // should be modified to explicitly do NULL-checks on pointers so that this
-    // function does not rely on undefined behavior
-    return (NULL == &pim) ? os : pim.OsPrint(os);
+    return pim.OsPrint(os);
   }
 
 }

libgpopt/src/base/CReqdPropPlan.cpp

@@ -10,6 +10,7 @@
 //---------------------------------------------------------------------------
 
 #include "gpos/base.h"
+#include "gpos/common/CPrintablePointer.h"
 
 #include "gpopt/base/CUtils.h"
 #include "gpopt/base/CColRefSet.h"
@@ -684,7 +685,7 @@ CReqdPropPlan::OsPrint
 	os << "], req order: [" << (*m_peo);
 	os << "], req dist: [" << (*m_ped);
 	os << "], req rewind: [" << (*m_per);	
-	os	<<	"], req partition propagation: [" << (*m_pepp);
+	os	<<	"], req partition propagation: [" << pp(m_pepp);
 	os <<  "]";
 	
 	return os;

libgpos/CMakeLists.txt

@@ -223,7 +223,8 @@ add_library(gpos
             src/test/CTimeSliceTest.cpp
             include/gpos/test/CUnittest.h
             src/test/CUnittest.cpp
-            )
+            include/gpos/common/CPrintablePointer.h
+        )
 target_link_libraries(gpos ${CMAKE_THREAD_LIBS_INIT} ${CMAKE_DL_LIBS})
 
 # Extra system libs needed on Solaris.

libgpos/include/gpos/common/CPrintablePointer.h

+//	Copyright (C) 2017 Pivotal Software, Inc.
+
+#ifndef GPOS_CPrintablePointer_H
+#define GPOS_CPrintablePointer_H
+
+#include "gpos/io/IOstream.h"
+
+namespace gpos
+{
+	template <typename T>
+	class CPrintablePointer
+	{
+		private:
+			T *m_pt;
+			friend IOstream &operator << (IOstream &os, CPrintablePointer p)
+			{
+				if (p.m_pt)
+				{
+					return os << *p.m_pt;
+				}
+				else
+				{
+					return os;
+				}
+			}
+
+		public:
+			explicit CPrintablePointer(T *pt) : m_pt(pt) {}
+			CPrintablePointer(const CPrintablePointer &pointer) : m_pt(pointer.m_pt) {}
+	};
+
+	template <typename T>
+	CPrintablePointer<T> pp(T *pt)
+	{
+		return CPrintablePointer<T>(pt);
+	}
+}
+#endif // GPOS_CPrintablePointer_H