From 0be9a848f4834835695379f0ece221d49a57cda5 Mon Sep 17 00:00:00 2001 From: Jesse Zhang and Xin Zhang Date: Thu, 28 Apr 2016 17:13:17 -0700 Subject: [PATCH 1/2] Divorce CAutoRef from CAutoP [#118594399] --- libgpos/include/gpos/common/CAutoP.h | 51 ++++-------------- .../include/gpos/common/CAutoPointerBase.h | 53 +++++++++++++++++++ libgpos/include/gpos/common/CAutoRef.h | 18 +++---- libgpos/src/common/CBitSet.cpp | 1 + 4 files changed, 73 insertions(+), 50 deletions(-) create mode 100644 libgpos/include/gpos/common/CAutoPointerBase.h diff --git a/libgpos/include/gpos/common/CAutoP.h b/libgpos/include/gpos/common/CAutoP.h index bbac2fd..1af264d 100644 --- a/libgpos/include/gpos/common/CAutoP.h +++ b/libgpos/include/gpos/common/CAutoP.h @@ -24,6 +24,7 @@ #define GPOS_CAutoP_H #include "gpos/base.h" +#include "gpos/common/CAutoPointerBase.h" #include "gpos/common/CStackObject.h" namespace gpos @@ -38,13 +39,12 @@ namespace gpos // //--------------------------------------------------------------------------- template - class CAutoP : public CStackObject + class CAutoP : public CAutoPointerBase { + private: + typedef CAutoPointerBase _base; protected: - - // actual element to point to - T *m_pt; // hidden copy ctor CAutoP @@ -58,52 +58,23 @@ namespace gpos explicit CAutoP() : - m_pt(NULL) + _base(NULL) {} explicit CAutoP(T *pt) : - m_pt(pt) + _base(pt) {} - // dtor - virtual ~CAutoP(); - - // simple assignment - CAutoP const & operator = (T* pt) + CAutoP const& operator = (T* pt) { - m_pt = pt; + _base::m_pt = pt; return *this; } - // deref operator - T &operator * () - { - GPOS_ASSERT(NULL != m_pt); - return *m_pt; - } - - // returns only base pointer, compiler does appropriate deref'ing - T* operator -> () - { - return m_pt; - } - - // return basic pointer - T* Pt() - { - return m_pt; - } - - // unhook pointer from auto object - T* PtReset() - { - T* pt = m_pt; - m_pt = NULL; - return pt; - } - + // dtor + virtual ~CAutoP(); }; // class CAutoP //--------------------------------------------------------------------------- @@ -117,7 +88,7 @@ namespace gpos template CAutoP::~CAutoP() { - GPOS_DELETE(m_pt); + GPOS_DELETE(_base::m_pt); } } diff --git a/libgpos/include/gpos/common/CAutoPointerBase.h b/libgpos/include/gpos/common/CAutoPointerBase.h new file mode 100644 index 0000000..7a33c4b --- /dev/null +++ b/libgpos/include/gpos/common/CAutoPointerBase.h @@ -0,0 +1,53 @@ +#ifndef GPOS_CAUTOPOINTERBASE_H +#define GPOS_CAUTOPOINTERBASE_H + +#include "CStackObject.h" + +namespace gpos +{ + template + class CAutoPointerBase: public CStackObject + { + protected: + CAutoPointerBase() + : m_pt(NULL) + {} + + CAutoPointerBase(T* pt) + : m_pt(pt) + {} + // actual element to point to + T *m_pt; + public: + + // deref operator + T &operator * () + { + GPOS_ASSERT(NULL != m_pt); + return *m_pt; + } + + // returns only base pointer, compiler does appropriate deref'ing + T* operator -> () + { + return m_pt; + } + + // return basic pointer + T* Pt() + { + return m_pt; + } + + // unhook pointer from auto object + T* PtReset() + { + T* pt = m_pt; + m_pt = NULL; + return pt; + } + }; +} + + +#endif //GPOS_CAUTOPOINTERBASE_H diff --git a/libgpos/include/gpos/common/CAutoRef.h b/libgpos/include/gpos/common/CAutoRef.h index 1198fdf..096afdd 100644 --- a/libgpos/include/gpos/common/CAutoRef.h +++ b/libgpos/include/gpos/common/CAutoRef.h @@ -18,7 +18,7 @@ #define GPOS_CAutoRef_H #include "gpos/base.h" -#include "gpos/common/CAutoP.h" +#include "gpos/common/CAutoPointerBase.h" #include "gpos/common/CRefCount.h" namespace gpos @@ -32,10 +32,11 @@ namespace gpos // //--------------------------------------------------------------------------- template - class CAutoRef : public CAutoP + class CAutoRef : public CAutoPointerBase { private: + typedef CAutoPointerBase _base; // hidden copy ctor CAutoRef @@ -49,14 +50,14 @@ namespace gpos explicit CAutoRef() : - CAutoP() + _base() {} // ctor explicit CAutoRef(T *pt) : - CAutoP(pt) + _base(pt) {} virtual ~CAutoRef(); @@ -64,7 +65,7 @@ namespace gpos // simple assignment CAutoRef const & operator = (T* pt) { - CAutoP::m_pt = pt; + _base::m_pt = pt; return *this; } @@ -81,13 +82,10 @@ namespace gpos template CAutoRef::~CAutoRef() { - if (NULL != CAutoP::m_pt) + if (NULL != _base::m_pt) { - reinterpret_cast(CAutoP::m_pt)->Release(); + reinterpret_cast(_base::m_pt)->Release(); } - - // null out pointer before ~CAutoP() gets called - CAutoP::m_pt = NULL; } } diff --git a/libgpos/src/common/CBitSet.cpp b/libgpos/src/common/CBitSet.cpp index 149ceea..627c9a8 100644 --- a/libgpos/src/common/CBitSet.cpp +++ b/libgpos/src/common/CBitSet.cpp @@ -19,6 +19,7 @@ //--------------------------------------------------------------------------- #include "gpos/base.h" +#include "gpos/common/CAutoP.h" #include "gpos/common/CAutoRef.h" #include "gpos/common/CBitSet.h" #include "gpos/common/CBitSetIter.h" From 024fe3ce80953fbe0461e728c83c88a6766baf0b Mon Sep 17 00:00:00 2001 From: Jesse Zhang and Xin Zhang Date: Thu, 28 Apr 2016 17:33:07 -0700 Subject: [PATCH 2/2] Guard against wrong template parameters [#118594399] [#118323445] Both snippets should fail at compile-time: ``` GPOS_RESULT AutoPShouldBlowUpGivenRefCount() { // create memory pool CAutoMemoryPool amp; IMemoryPool *pmp = amp.Pmp(); CAutoP aprf(GPOS_NEW(pmp) CRefCount()); return GPOS_OK; } ``` ``` static GPOS_RESULT AutoRefShouldBlowUpGivenNonRefCount() { // create memory pool CAutoMemoryPool amp; IMemoryPool *pmp = amp.Pmp(); CAutoRef aprf(GPOS_NEW(pmp) int()); return GPOS_OK; } ``` --- CMakeLists.txt | 1 + libgpos/include/gpos/common/CAutoP.h | 5 +++++ libgpos/include/gpos/common/CAutoRef.h | 6 +++++- make/gpo.mk | 2 +- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b61ff57..63bb1c1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -15,6 +15,7 @@ set(GPOS_VERSION_STRING ${GPOS_VERSION_MAJOR}.${GPOS_VERSION_MINOR}) # doubt, do the safe thing and increment this number. set(GPOS_ABI_VERSION 10) +set(CMAKE_CXX_STANDARD 11) # Configure CCache if available find_program(CCACHE_FOUND ccache) if(CCACHE_FOUND) diff --git a/libgpos/include/gpos/common/CAutoP.h b/libgpos/include/gpos/common/CAutoP.h index 1af264d..9d2cca1 100644 --- a/libgpos/include/gpos/common/CAutoP.h +++ b/libgpos/include/gpos/common/CAutoP.h @@ -23,8 +23,11 @@ #ifndef GPOS_CAutoP_H #define GPOS_CAutoP_H +#include + #include "gpos/base.h" #include "gpos/common/CAutoPointerBase.h" +#include "gpos/common/CRefCount.h" #include "gpos/common/CStackObject.h" namespace gpos @@ -42,6 +45,8 @@ namespace gpos class CAutoP : public CAutoPointerBase { private: + static_assert(!std::is_base_of::value, "T must not be a CRefCount"); + typedef CAutoPointerBase _base; protected: diff --git a/libgpos/include/gpos/common/CAutoRef.h b/libgpos/include/gpos/common/CAutoRef.h index 096afdd..c91070b 100644 --- a/libgpos/include/gpos/common/CAutoRef.h +++ b/libgpos/include/gpos/common/CAutoRef.h @@ -17,6 +17,8 @@ #ifndef GPOS_CAutoRef_H #define GPOS_CAutoRef_H +#include + #include "gpos/base.h" #include "gpos/common/CAutoPointerBase.h" #include "gpos/common/CRefCount.h" @@ -36,6 +38,8 @@ namespace gpos { private: + static_assert(std::is_base_of::value, "T must be a CRefCount"); + typedef CAutoPointerBase _base; // hidden copy ctor @@ -84,7 +88,7 @@ namespace gpos { if (NULL != _base::m_pt) { - reinterpret_cast(_base::m_pt)->Release(); + _base::m_pt->Release(); } } } diff --git a/make/gpo.mk b/make/gpo.mk index e70308b..0cd8f7c 100644 --- a/make/gpo.mk +++ b/make/gpo.mk @@ -75,7 +75,7 @@ CFLAGS_WARN = -Wall -Werror -Wextra -pedantic-errors -Wno-variadic-macros -Wconv CFLAGS_debug = -g3 -DGPOS_DEBUG CFLAGS_opt = -O3 -fno-omit-frame-pointer -g3 -CFLAGS_TYPE = $(CFLAGS_$(BLD_TYPE)) +CFLAGS_TYPE = -std=gnu++0x $(CFLAGS_$(BLD_TYPE)) ifeq ($(ARCH_BIT), GPOS_32BIT) ARCH_FLAGS = -m32