Remove all the "NULL == this" and "NULL != this" (#202)

Add NULL check before calling IMDId FValid()

CMakeLists.txt

@@ -5,7 +5,7 @@ project(gpopt LANGUAGES CXX C)
 
 set(GPORCA_VERSION_MAJOR 2)
 set(GPORCA_VERSION_MINOR 39)
-set(GPORCA_VERSION_PATCH 1)
+set(GPORCA_VERSION_PATCH 2)
 set(GPORCA_VERSION_STRING "${GPORCA_VERSION_MAJOR}.${GPORCA_VERSION_MINOR}.${GPORCA_VERSION_PATCH}")
 
 # Whenever an ABI-breaking change is made to GPORCA, this should be incremented.

libgpopt/src/base/CFunctionalDependency.cpp

@@ -85,16 +85,6 @@ CFunctionalDependency::FIncluded
 ULONG
 CFunctionalDependency::UlHash() const
 {
-	// FIXME(chasseur): in well-formed C++ code, the implicit 'this' pointer in
-	// a member method can never be NULL; however, some callers may invoke this
-	// method from a (possibly-NULL) pointer with the '->' operator; callers
-	// should be modified to explicitly do NULL-checks on pointers so that this
-	// method does not rely on undefined behavior
-	if (NULL == this)
-	{
-		return 0;
-	}
-
 	return gpos::UlCombineHashes(m_pcrsKey->UlHash(), m_pcrsDetermined->UlHash());
 }
 
@@ -114,14 +104,9 @@ CFunctionalDependency::FEqual
 	)
 	const
 {
-	// FIXME(chasseur): in well-formed C++ code, the implicit 'this' pointer in
-	// a member method can never be NULL; however, some callers may invoke this
-	// method from a (possibly-NULL) pointer with the '->' operator; callers
-	// should be modified to explicitly do NULL-checks on pointers so that this
-	// method does not rely on undefined behavior
-	if (NULL == this || NULL == pfd)
+	if (NULL == pfd)
 	{
-		return (NULL == this && NULL == pfd);
+		return false;
 	}
 
 	return m_pcrsKey->FEqual(pfd->PcrsKey()) && m_pcrsDetermined->FEqual(pfd->PcrsDetermined());

libgpopt/src/base/CKeyCollection.cpp

@@ -122,7 +122,6 @@ CKeyCollection::Add
 	CColRefSet *pcrs
 	)
 {
-	GPOS_ASSERT(NULL != this);
 	GPOS_ASSERT(!FKey(pcrs) && "no duplicates allowed");
 	
 	m_pdrgpcrs->Append(pcrs);
@@ -146,16 +145,6 @@ CKeyCollection::FKey
 	)
 	const
 {
-	// FIXME(chasseur): in well-formed C++ code, the implicit 'this' pointer in
-	// a member method can never be NULL; however, some callers may invoke this
-	// method from a (possibly-NULL) pointer with the '->' operator; callers
-	// should be modified to explicitly do NULL-checks on pointers so that this
-	// method does not rely on undefined behavior
-	if (NULL == this)
-	{
-		return false;
-	}
-
 	const ULONG ulSets = m_pdrgpcrs->UlLength();
 	for (ULONG ul = 0; ul < ulSets; ul++)
 	{
@@ -198,16 +187,6 @@ CKeyCollection::FKey
 	)
 	const
 {
-	// FIXME(chasseur): in well-formed C++ code, the implicit 'this' pointer in
-	// a member method can never be NULL; however, some callers may invoke this
-	// method from a (possibly-NULL) pointer with the '->' operator; callers
-	// should be modified to explicitly do NULL-checks on pointers so that this
-	// method does not rely on undefined behavior
-	if (NULL == this)
-	{
-		return false;
-	}
-
 	CColRefSet *pcrs = GPOS_NEW(pmp) CColRefSet(pmp);
 	pcrs->Include(pdrgpcr);
 	
@@ -234,15 +213,6 @@ CKeyCollection::PdrgpcrTrim
 	)
 	const
 {
-	// FIXME(chasseur): in well-formed C++ code, the implicit 'this' pointer in
-	// a member method can never be NULL; however, some callers may invoke this
-	// method from a (possibly-NULL) pointer with the '->' operator; callers
-	// should be modified to explicitly do NULL-checks on pointers so that this
-	// method does not rely on undefined behavior
-	if (NULL == this) {
-		return NULL;
-	}
-
 	DrgPcr *pdrgpcrTrim = NULL;
 	CColRefSet *pcrs = GPOS_NEW(pmp) CColRefSet(pmp);
 	pcrs->Include(pdrgpcr);

libgpopt/src/base/CPartIndexMap.cpp

@@ -1000,7 +1000,6 @@ CPartIndexMap::OsPrint
 	)
 	const
 {
-	GPOS_ASSERT(NULL != this);
 	PartIndexMapIter pimi(m_pim);
 	while (pimi.FAdvance())
 	{

libgpopt/src/base/CUtils.cpp

@@ -3423,7 +3423,7 @@ CUtils::FComparisonPossible
 	{
 		CColRef *pcr = (*pdrgpcr)[ul];
 		const IMDType *pmdtype = pcr->Pmdtype();
-		if (!pmdtype->PmdidCmp(ecmpt)->FValid())
+		if (!IMDId::FValid(pmdtype->PmdidCmp(ecmpt)))
 		{
 			return false;
 		}

libgpopt/src/mdcache/CMDAccessorUtils.cpp

@@ -113,7 +113,7 @@ CMDAccessorUtils::FCmpExists
 	if (pmdidLeft->FEquals(pmdidRight))
 	{
 		const IMDType *pmdtypeLeft = pmda->Pmdtype(pmdidLeft);
-		return pmdtypeLeft->PmdidCmp(ecmpt)->FValid();
+		return IMDId::FValid(pmdtypeLeft->PmdidCmp(ecmpt));
 	}
 
 	GPOS_TRY

libgpopt/src/operators/CScalarCast.cpp

@@ -80,7 +80,7 @@ CScalarCast::FMatch
 
 		// match if the return type oids are identical
 		return pscop->PmdidType()->FEquals(m_pmdidReturnType) &&
-				((!pscop->PmdidFunc()->FValid() && !m_pmdidFunc->FValid()) || pscop->PmdidFunc()->FEquals(m_pmdidFunc));
+				((!IMDId::FValid(pscop->PmdidFunc()) && !IMDId::FValid(m_pmdidFunc)) || pscop->PmdidFunc()->FEquals(m_pmdidFunc));
 	}
 
 	return false;

libgpopt/src/xforms/CXformUtils.cpp

@@ -1858,7 +1858,7 @@ CXformUtils::FSupportsMinAgg
 	{
 		CColRef *pcr = (*pdrgpcr)[ul];
 		const IMDType *pmdtype = pcr->Pmdtype();
-		if (!pmdtype->PmdidAgg(IMDType::EaggMin)->FValid())
+		if (!IMDId::FValid(pmdtype->PmdidAgg(IMDType::EaggMin)))
 		{
 			return false;
 		}

libnaucrates/include/naucrates/md/CMDIdCast.h

@@ -106,7 +106,7 @@ namespace gpmd
 			virtual
 			BOOL FValid() const
 			{
-				return m_pmdidSrc->FValid() && m_pmdidDest->FValid();
+				return IMDId::FValid(m_pmdidSrc) && IMDId::FValid(m_pmdidDest);
 			}
 
 			// serialize mdid in DXL as the value of the specified attribute 

libnaucrates/include/naucrates/md/CMDIdColStats.h

@@ -102,7 +102,7 @@ namespace gpmd
 			virtual
 			BOOL FValid() const
 			{
-				return m_pmdidRel->FValid();
+				return IMDId::FValid(m_pmdidRel);
 			}
 
 			// serialize mdid in DXL as the value of the specified attribute 

libnaucrates/include/naucrates/md/CMDIdRelStats.h

@@ -100,7 +100,7 @@ namespace gpmd
 			virtual
 			BOOL FValid() const
 			{
-				return m_pmdidRel->FValid();
+				return IMDId::FValid(m_pmdidRel);
 			}
 
 			// serialize mdid in DXL as the value of the specified attribute 

libnaucrates/include/naucrates/md/CMDIdScCmp.h

@@ -102,7 +102,7 @@ namespace gpmd
 			virtual
 			BOOL FValid() const
 			{
-				return m_pmdidLeft->FValid() && m_pmdidRight->FValid() && IMDType::EcmptOther != m_ecmpt;
+				return IMDId::FValid(m_pmdidLeft) && IMDId::FValid(m_pmdidRight) && IMDType::EcmptOther != m_ecmpt;
 			}
 
 			// serialize mdid in DXL as the value of the specified attribute 

libnaucrates/src/md/CMDCastGPDB.cpp

@@ -220,7 +220,7 @@ CMDCastGPDB::DebugPrint
 		os << ", binary-coercible";
 	}
 	
-	if (m_pmdidCastFunc->FValid())
+	if (IMDId::FValid(m_pmdidCastFunc))
 	{
 		os << ", Cast func id: ";
 		PmdidCastFunc()->OsPrint(os);

libnaucrates/src/md/CMDIdGPDB.cpp

@@ -329,12 +329,7 @@ CMDIdGPDB::FEquals
 BOOL
 CMDIdGPDB::FValid() const
 {
-	// FIXME(chasseur): in well-formed C++ code, the implicit 'this' pointer in
-	// a member method can never be NULL; however, some callers may invoke this
-	// method from a (possibly-NULL) pointer with the '->' operator; callers
-	// should be modified to explicitly do NULL-checks on pointers so that this
-	// method does not rely on undefined behavior
-	return NULL != this && !FEquals(&CMDIdGPDB::m_mdidInvalidKey);
+	return !FEquals(&CMDIdGPDB::m_mdidInvalidKey);
 }
 
 //---------------------------------------------------------------------------

libnaucrates/src/md/CMDIdGPDBCtas.cpp

@@ -95,12 +95,7 @@ CMDIdGPDBCtas::FEquals
 //---------------------------------------------------------------------------
 BOOL CMDIdGPDBCtas::FValid() const
 {
-	// FIXME(chasseur): in well-formed C++ code, the implicit 'this' pointer in
-	// a member method can never be NULL; however, some callers may invoke this
-	// method from a (possibly-NULL) pointer with the '->' operator; callers
-	// should be modified to explicitly do NULL-checks on pointers so that this
-	// method does not rely on undefined behavior
-	return NULL != this && !FEquals(&CMDIdGPDBCtas::m_mdidInvalidKey);
+	return !FEquals(&CMDIdGPDBCtas::m_mdidInvalidKey);
 }
 
 //---------------------------------------------------------------------------

server/src/unittest/gpopt/base/CKeyCollectionTest.cpp

@@ -95,7 +95,6 @@ CKeyCollectionTest::EresUnittest_Basics()
 
 	pkc->Add(pcrs);	
 	GPOS_ASSERT(pkc->FKey(pcrs));
-	GPOS_ASSERT(!((CKeyCollection*)NULL)->FKey(pcrs));
 	
 	DrgPcr *pdrgpcr = pkc->PdrgpcrKey(pmp);
 	GPOS_ASSERT(pkc->FKey(pmp, pdrgpcr));
@@ -177,7 +176,6 @@ CKeyCollectionTest::EresUnittest_Subsumes()
 	pkc->Add(pcrs2);
 
 	GPOS_ASSERT(pkc->FKey(pcrs2));
-	GPOS_ASSERT(!((CKeyCollection*)NULL)->FKey(pcrs2));
 
 	// get the second key
 	DrgPcr *pdrgpcr = pkc->PdrgpcrKey(pmp, 1);