From 72aad0d6ea0c11c7cbb722eefb6106a7f1bdd203 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 12 Apr 2014 00:10:54 +0000 Subject: [PATCH] Use the same wxWeakRef implementation for complete and incomplete classes. This fixes an ODR violation which could arise if wxWeakRef was seen both when T was an incomplete (e.g. just forward-defined) class and when it was complete. As different implementations, with different binary layouts, were used in these two cases, this resulted in fatal run-time problems. Fix this by always using the slightly less efficient (because storing an extra pointer) but simpler and safe "dynamic" wxWeakRef implementation. Also get rid of checks for the ancient compilers such as VC6 and g++ < 3.3, they are not supported any longer. Closes #15884. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@76316 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/weakref.h | 201 ++++++++------------------------------ tests/Makefile.in | 10 +- tests/test.bkl | 10 +- tests/weakref/weakref.cpp | 18 ++++ 4 files changed, 75 insertions(+), 164 deletions(-) diff --git a/include/wx/weakref.h b/include/wx/weakref.h index 0aa02900fb..4794c85ef0 100644 --- a/include/wx/weakref.h +++ b/include/wx/weakref.h @@ -13,22 +13,6 @@ #include "wx/tracker.h" -// Some compilers (VC6, Borland, g++ < 3.3) have problem with template specialization. -// However, this is only used for optimization purposes (a smaller wxWeakRef pointer) -// (and the corner case of wxWeakRef). So for those compilers, we can fall -// back to the non-optimal case, where we use the same type of weak ref (static one) -// in all cases. See defs.h for various setting these defines depending on compiler. - -#if !defined(HAVE_PARTIAL_SPECIALIZATION) || \ - !defined(HAVE_TEMPLATE_OVERLOAD_RESOLUTION) || \ - (defined(__GNUC__) && !wxCHECK_GCC_VERSION(3, 3)) - #define USE_ONLY_STATIC_WEAKREF -#endif - - -#ifndef USE_ONLY_STATIC_WEAKREF - -// Avoid including this for simpler compilers #include "wx/meta/convertible.h" #include "wx/meta/int2type.h" @@ -38,79 +22,55 @@ struct wxIsStaticTrackable enum { value = wxConvertibleTo::value }; }; -#endif // !USE_ONLY_STATIC_WEAKREF - -// Weak ref implementation when T has wxTrackable as a known base class +// A weak reference to an object of type T (which must inherit from wxTrackable) template -class wxWeakRefStatic : public wxTrackerNode +class wxWeakRef : public wxTrackerNode { public: - wxWeakRefStatic() : m_pobj(NULL) { } + typedef T element_type; - void Release() + // Default ctor + wxWeakRef() : m_pobj(NULL), m_ptbase(NULL) { } + + // Ctor from the object of this type: this is needed as the template ctor + // below is not used by at least g++4 when a literal NULL is used + wxWeakRef(T *pobj) : m_pobj(NULL), m_ptbase(NULL) { - // Release old object if any - if ( m_pobj ) - { - // Remove ourselves from object tracker list - wxTrackable *pt = static_cast(m_pobj); - pt->RemoveNode(this); - m_pobj = NULL; - } + this->Assign(pobj); } - virtual void OnObjectDestroy() wxOVERRIDE + // When we have the full type here, static_cast<> will always work + // (or give a straight compiler error). + template + wxWeakRef(TDerived* pobj) : m_pobj(NULL), m_ptbase(NULL) { - // Tracked object itself removes us from list of trackers - wxASSERT(m_pobj != NULL); - m_pobj = NULL; + this->Assign(pobj); } -protected: - void Assign(T* pobj) + // We need this copy ctor, since otherwise a default compiler (binary) copy + // happens (if embedded as an object member). + wxWeakRef(const wxWeakRef& wr) : m_pobj(NULL), m_ptbase(NULL) { - if ( m_pobj == pobj ) - return; - - Release(); - - // Now set new trackable object - if ( pobj ) - { - // Add ourselves to object tracker list - wxTrackable *pt = static_cast(pobj); - pt->AddNode(this); - m_pobj = pobj; - } + this->Assign(wr.get()); } - void AssignCopy(const wxWeakRefStatic& wr) + wxWeakRef& operator=(const wxWeakRef& wr) { - Assign( wr.m_pobj ); + this->AssignCopy(wr); + return *this; } - T *m_pobj; -}; + virtual ~wxWeakRef() { this->Release(); } + // Smart pointer functions + T& operator*() const { return *this->m_pobj; } + T* operator->() const { return this->m_pobj; } + T* get() const { return this->m_pobj; } + operator T*() const { return this->m_pobj; } -#ifndef USE_ONLY_STATIC_WEAKREF - -template -struct wxWeakRefImpl; - -// Intermediate class, to select the static case above. -template -struct wxWeakRefImpl : public wxWeakRefStatic -{ - enum { value = 1 }; -}; - -// Weak ref implementation when T does not have wxTrackable as known base class -template -struct wxWeakRefImpl : public wxTrackerNode -{ +public: void Release() { // Release old object if any @@ -132,51 +92,30 @@ struct wxWeakRefImpl : public wxTrackerNode } protected: - wxWeakRefImpl() : m_pobj(NULL), m_ptbase(NULL) { } - // Assign receives most derived class here and can use that template void Assign( TDerived* pobj ) { - AssignHelper( pobj, wxInt2Type::value>() ); - } - - template - void AssignHelper(TDerived* pobj, wxInt2Type) - { + wxCOMPILE_TIME_ASSERT( wxIsStaticTrackable::value, + Tracked_class_should_inherit_from_wxTrackable ); wxTrackable *ptbase = static_cast(pobj); - DoAssign( pobj, ptbase ); + DoAssign(pobj, ptbase); } -#ifndef wxNO_RTTI - void AssignHelper(T* pobj, wxInt2Type) - { - // A last way to get a trackable pointer - wxTrackable *ptbase = dynamic_cast(pobj); - if ( ptbase ) - { - DoAssign( pobj, ptbase ); - } - else - { - wxFAIL_MSG( "Tracked class should inherit from wxTrackable" ); - - Release(); - } - } -#endif // RTTI enabled - - void AssignCopy(const wxWeakRefImpl& wr) + void AssignCopy(const wxWeakRef& wr) { DoAssign(wr.m_pobj, wr.m_ptbase); } - void DoAssign( T* pobj, wxTrackable *ptbase ) { - if( m_pobj==pobj ) return; + void DoAssign(T* pobj, wxTrackable *ptbase) + { + if ( m_pobj == pobj ) + return; + Release(); // Now set new trackable object - if( pobj ) + if ( pobj ) { // Add ourselves to object tracker list wxASSERT( ptbase ); @@ -190,68 +129,6 @@ protected: wxTrackable *m_ptbase; }; -#endif // #ifndef USE_ONLY_STATIC_WEAKREF - - - -// A weak reference to an object of type T, where T has base wxTrackable -// (usually statically but if not dynamic_cast<> is tried). -template -class wxWeakRef : public -#ifdef USE_ONLY_STATIC_WEAKREF - wxWeakRefStatic -#else - wxWeakRefImpl::value != 0> -#endif -{ -public: - typedef T element_type; - - // Default ctor - wxWeakRef() { } - - // Enabling this ctor for VC6 results in mysterious compilation failures in - // wx/window.h when assigning wxWindow pointers (FIXME-VC6) -#ifndef __VISUALC6__ - // Ctor from the object of this type: this is needed as the template ctor - // below is not used by at least g++4 when a literal NULL is used - wxWeakRef(T *pobj) - { - this->Assign(pobj); - } -#endif // !__VISUALC6__ - - // When we have the full type here, static_cast<> will always work - // (or give a straight compiler error). - template - wxWeakRef(TDerived* pobj) - { - this->Assign(pobj); - } - - // We need this copy ctor, since otherwise a default compiler (binary) copy - // happens (if embedded as an object member). - wxWeakRef(const wxWeakRef& wr) - { - this->Assign(wr.get()); - } - - wxWeakRef& operator=(const wxWeakRef& wr) - { - this->AssignCopy(wr); - return *this; - } - - virtual ~wxWeakRef() { this->Release(); } - - // Smart pointer functions - T& operator*() const { return *this->m_pobj; } - T* operator->() const { return this->m_pobj; } - - T* get() const { return this->m_pobj; } - operator T*() const { return this->m_pobj; } -}; - #ifndef wxNO_RTTI diff --git a/tests/Makefile.in b/tests/Makefile.in index 2e1f420e18..bb3b96765f 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -1032,7 +1032,7 @@ test_gui_xrctest.o: $(srcdir)/xml/xrctest.cpp $(TEST_GUI_ODEP) # warnings don't matter when we expect compilation to fail anyhow so we can # use this variable to enable the compilation of code which is supposed to # fail -failtest: failtest_combobox failtest_evthandler +failtest: failtest_combobox failtest_evthandler failtest_weakref failtest_combobox: @$(RM) test_gui_comboboxtest.o @@ -1052,6 +1052,14 @@ failtest_evthandler: done; \ exit 0 +failtest_weakref: + @$(RM) test_weakref.o + if $(MAKE) CXXWARNINGS=-DTEST_INVALID_INCOMPLETE_WEAKREF test_weakref.o 2>/dev/null; then \ + echo "*** Compilation with TEST_INVALID_INCOMPLETE_WEAKREF unexpectedly succeeded.">&2; \ + exit 1; \ + fi; \ + exit 0 + .PHONY: failtest # Include dependency info, if present: diff --git a/tests/test.bkl b/tests/test.bkl index 505f55ca2a..c9cd6da0f4 100644 --- a/tests/test.bkl +++ b/tests/test.bkl @@ -333,7 +333,7 @@ # warnings don't matter when we expect compilation to fail anyhow so we can # use this variable to enable the compilation of code which is supposed to # fail -failtest: failtest_combobox failtest_evthandler +failtest: failtest_combobox failtest_evthandler failtest_weakref failtest_combobox: @$(RM) test_gui_comboboxtest.o @@ -353,6 +353,14 @@ failtest_evthandler: done; \ exit 0 +failtest_weakref: + @$(RM) test_weakref.o + if $(MAKE) CXXWARNINGS=-DTEST_INVALID_INCOMPLETE_WEAKREF test_weakref.o 2>/dev/null; then \ + echo "*** Compilation with TEST_INVALID_INCOMPLETE_WEAKREF unexpectedly succeeded.">&2; \ + exit 1; \ + fi; \ + exit 0 + .PHONY: failtest diff --git a/tests/weakref/weakref.cpp b/tests/weakref/weakref.cpp index 4922daac5e..c01622cc6e 100644 --- a/tests/weakref/weakref.cpp +++ b/tests/weakref/weakref.cpp @@ -83,6 +83,9 @@ wxWeakRef g_incompleteWeakRef; struct ForwardDeclaredClass : wxEvtHandler { }; +// A incomplete class that would be defined in other compilation units +struct IncompleteClass; + void WeakRefTestCase::DeclareTest() { { @@ -128,6 +131,21 @@ void WeakRefTestCase::DeclareTest() CPPUNIT_ASSERT( !g_incompleteWeakRef ); #endif // RTTI enabled + + { + // Construction of a wxWeakRef to an incomplete class should be fine + wxWeakRef p; + + // Copying should be also OK + p = p; + + // Assigning a raw pointer should cause compile error +#ifdef TEST_INVALID_INCOMPLETE_WEAKREF + p = static_cast(0); +#endif + + // Releasing should be OK + } } void WeakRefTestCase::AssignTest()