DRTVWR-418: Xcode 8.3 complains about LLSafeHandle<T> implementation.

The previous LLSafeHandle<T> implementation declares a static data member of
the template class but provides no (generic) definition, relying on particular
specializations to provide the definition. The data member is a function
pointer, which is called in one of the methods to produce a pointer to a
"null" T instance: that is, a dummy instance to be dereferenced in case the
wrapped T* is null.

Xcode 8.3's version of clang is bothered by the call, in a generic method,
through this (usually) uninitialized pointer. It happens that the only
specializations of LLSafeHandle do both provide definitions. I don't know
whether that's formally valid C++03 or not; but I agree with the compiler: I
don't like it.

Instead of declaring a public static function pointer which each
specialization is required to define, add a protected static method to the
template class. This protected static method simply returns a pointer to a
function-static T instance. This is functionally similar to a static
LLPointer<T> set on demand (as in the two specializations), including lazy
instantiation.

Unlike the previous implementation, this approach prohibits a given
specialization from customizing the "null" instance function. Although there
exist reasonable ways to support that (e.g. a related traits template), I
decided not to complicate the LLSafeHandle implementation to make it more
generally useful. I don't really approve of LLSafeHandle, and don't want to
see it proliferate. It's not clear that unconditionally dereferencing
LLSafeHandle<T> is in any way better than conditionally dereferencing
LLPointer<T>. It doesn't even skip the runtime conditional test; it simply
obscures it. (There exist hints in the code that at one time it might have
immediately replaced any wrapped null pointer value with the pointer to the
"null" instance, obviating the test at dereference time, but this is not the
current functionality. Perhaps it was only ever wishful thinking.)

Remove the corresponding functions and static LLPointers from the two classes
that use LLSafeHandle.

5 files changed, 8 insertions, 43 deletions

diff --git a/indra/llcommon/llsafehandle.h b/indra/llcommon/llsafehandle.h
index 4226bf04f0..af1c26dd4f 100644
--- a/indra/llcommon/llsafehandle.h
+++ b/indra/llcommon/llsafehandle.h
@@ -112,10 +112,6 @@ public:
 		return *this; 
 	}
 
-public:
-	typedef Type* (*NullFunc)();
-	static const NullFunc sNullFunc;
-
 protected:
 	void ref()                             
 	{ 
@@ -155,6 +151,12 @@ protected:
 		return ptr == NULL ? sNullFunc() : ptr;
 	}
 
+	static Type* sNullFunc()
+	{
+		static Type sInstance;
+		return &sInstance;
+	}
+
 protected:
 	Type*	mPointer;
 };
diff --git a/indra/newview/llparcelselection.cpp b/indra/newview/llparcelselection.cpp
index 4d1901adc9..5c62159b93 100644
--- a/indra/newview/llparcelselection.cpp
+++ b/indra/newview/llparcelselection.cpp
@@ -31,13 +31,6 @@
 #include "llparcel.h"
 
 
-// static
-LLPointer<LLParcelSelection> LLParcelSelection::sNullSelection;
-
-template<>
-	const LLSafeHandle<LLParcelSelection>::NullFunc
-		LLSafeHandle<LLParcelSelection>::sNullFunc = LLParcelSelection::getNullParcelSelection;
-
 //
 // LLParcelSelection
 //
@@ -87,14 +80,3 @@ bool LLParcelSelection::hasOthersSelected() const
 {
 	return mSelectedOtherCount != 0;
 }
-
-// static
-LLParcelSelection* LLParcelSelection::getNullParcelSelection()
-{
-	if (sNullSelection.isNull())
-	{
-		sNullSelection = new LLParcelSelection;
-	}
-
-	return sNullSelection;
-}
diff --git a/indra/newview/llparcelselection.h b/indra/newview/llparcelselection.h
index 1cbdfc6f74..06d9141efb 100644
--- a/indra/newview/llparcelselection.h
+++ b/indra/newview/llparcelselection.h
@@ -35,6 +35,7 @@ class LLParcel;
 class LLParcelSelection : public LLRefCount
 {
 	friend class LLViewerParcelMgr;
+	friend class LLSafeHandle<LLParcelSelection>;
 
 protected:
 	~LLParcelSelection();
@@ -61,8 +62,6 @@ public:
 	// Is the entire parcel selected, or just a part?
 	BOOL	getWholeParcelSelected() const;
 
-	static LLParcelSelection* getNullParcelSelection();
-
 private:
 	void setParcel(LLParcel* parcel) { mParcel = parcel; }
 
@@ -73,8 +72,6 @@ private:
 	S32			mSelectedSelfCount;
 	S32			mSelectedOtherCount;
 	S32			mSelectedPublicCount;
-
-	static LLPointer<LLParcelSelection> sNullSelection;
 };
 
 typedef LLSafeHandle<LLParcelSelection> LLParcelSelectionHandle;
diff --git a/indra/newview/llselectmgr.cpp b/indra/newview/llselectmgr.cpp
index ee7c22800a..cbcd905ab6 100644
--- a/indra/newview/llselectmgr.cpp
+++ b/indra/newview/llselectmgr.cpp
@@ -132,11 +132,6 @@ LLColor4 LLSelectMgr::sHighlightParentColor;
 LLColor4 LLSelectMgr::sHighlightChildColor;
 LLColor4 LLSelectMgr::sContextSilhouetteColor;
 
-static LLObjectSelection *get_null_object_selection();
-template<> 
-	const LLSafeHandle<LLObjectSelection>::NullFunc 
-		LLSafeHandle<LLObjectSelection>::sNullFunc = get_null_object_selection;
-
 //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 // struct LLDeRezInfo
 //
@@ -156,27 +151,15 @@ struct LLDeRezInfo
 //
 
 
-static LLPointer<LLObjectSelection> sNullSelection;
-
 //
 // Functions
 //
 
 void LLSelectMgr::cleanupGlobals()
 {
-	sNullSelection = NULL;
 	LLSelectMgr::getInstance()->clearSelections();
 }
 
-LLObjectSelection *get_null_object_selection()
-{
-	if (sNullSelection.isNull())
-	{
-		sNullSelection = new LLObjectSelection;
-	}
-	return sNullSelection;
-}
-
 // Build time optimization, generate this function once here
 template class LLSelectMgr* LLSingleton<class LLSelectMgr>::getInstance();
 //-----------------------------------------------------------------------------
diff --git a/indra/newview/llselectmgr.h b/indra/newview/llselectmgr.h
index 2a893af266..fbf889b2f1 100644
--- a/indra/newview/llselectmgr.h
+++ b/indra/newview/llselectmgr.h
@@ -233,6 +233,7 @@ protected:
 class LLObjectSelection : public LLRefCount
 {
 	friend class LLSelectMgr;
+	friend class LLSafeHandle<LLObjectSelection>;
 
 protected:
 	~LLObjectSelection();