Merge pull request #2714 from secondlife/nat/xcode-16

Clean up llpointer.h per previous discussions.

5 files changed, 112 insertions, 15 deletions

diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt
index 78bfaade55..aa8810f00b 100644
--- a/indra/llcommon/CMakeLists.txt
+++ b/indra/llcommon/CMakeLists.txt
@@ -70,6 +70,7 @@ set(llcommon_SOURCE_FILES
     llmetrics.cpp
     llmortician.cpp
     llmutex.cpp
+    llpointer.cpp
     llpredicate.cpp
     llprocess.cpp
     llprocessor.cpp
diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h
index b17b9ff21e..b6d560a121 100644
--- a/indra/llcommon/llerror.h
+++ b/indra/llcommon/llerror.h
@@ -239,17 +239,12 @@ namespace LLError
 
         ~CallSite();
 
-#ifdef LL_LIBRARY_INCLUDE
-        bool shouldLog();
-#else // LL_LIBRARY_INCLUDE
         bool shouldLog()
         {
             return mCached
                     ? mShouldLog
                     : Log::shouldLog(*this);
         }
-            // this member function needs to be in-line for efficiency
-#endif // LL_LIBRARY_INCLUDE
 
         void invalidate();
 
diff --git a/indra/llcommon/llpointer.cpp b/indra/llcommon/llpointer.cpp
new file mode 100755
index 0000000000..adea447caa
--- /dev/null
+++ b/indra/llcommon/llpointer.cpp
@@ -0,0 +1,26 @@
+/**
+ * @file   llpointer.cpp
+ * @author Nat Goodspeed
+ * @date   2024-09-26
+ * @brief  Implementation for llpointer.
+ *
+ * $LicenseInfo:firstyear=2024&license=viewerlgpl$
+ * Copyright (c) 2024, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+// Precompiled header
+#include "linden_common.h"
+// associated header
+#include "llpointer.h"
+// STL headers
+// std headers
+// external library headers
+// other Linden headers
+#include "llerror.h"
+
+void LLPointerBase::wild_dtor(std::string_view msg)
+{
+//  LL_WARNS() << msg << LL_ENDL;
+    llassert_msg(false, msg);
+}
diff --git a/indra/llcommon/llpointer.h b/indra/llcommon/llpointer.h
index 71c955c4c5..b53cfcdd1a 100644
--- a/indra/llcommon/llpointer.h
+++ b/indra/llcommon/llpointer.h
@@ -26,8 +26,8 @@
 #ifndef LLPOINTER_H
 #define LLPOINTER_H
 
-#include "llerror.h"    // *TODO: consider eliminating this
-#include "llmutex.h"
+#include <boost/functional/hash.hpp>
+#include <string_view>
 #include <utility>                  // std::swap()
 
 //----------------------------------------------------------------------------
@@ -43,8 +43,18 @@
 
 //----------------------------------------------------------------------------
 
+class LLPointerBase
+{
+protected:
+    // alert the coder that a referenced type's destructor did something very
+    // strange -- this is in a non-template base class so we can hide the
+    // implementation in llpointer.cpp
+    static void wild_dtor(std::string_view msg);
+};
+
 // Note: relies on Type having ref() and unref() methods
-template <class Type> class LLPointer
+template <class Type>
+class LLPointer: public LLPointerBase
 {
 public:
     template<typename Subclass>
@@ -106,7 +116,6 @@ public:
     const Type& operator*() const               { return *mPointer; }
     Type&   operator*()                         { return *mPointer; }
 
-    operator BOOL() const                       { return (mPointer != nullptr); }
     operator bool() const                       { return (mPointer != nullptr); }
     bool operator!() const                      { return (mPointer == nullptr); }
     bool isNull() const                         { return (mPointer == nullptr); }
@@ -189,10 +198,6 @@ public:
     }
 
 protected:
-#ifdef LL_LIBRARY_INCLUDE
-    void ref();
-    void unref();
-#else
     void ref()
     {
         if (mPointer)
@@ -210,12 +215,11 @@ protected:
             temp->unref();
             if (mPointer != nullptr)
             {
-                LL_WARNS() << "Unreference did assignment to non-NULL because of destructor" << LL_ENDL;
+                wild_dtor("Unreference did assignment to non-NULL because of destructor");
                 unref();
             }
         }
     }
-#endif // LL_LIBRARY_INCLUDE
 
 protected:
     Type*   mPointer;
diff --git a/indra/llcommon/owning_ptr.h b/indra/llcommon/owning_ptr.h
new file mode 100755
index 0000000000..7cf8d3f0ba
--- /dev/null
+++ b/indra/llcommon/owning_ptr.h
@@ -0,0 +1,71 @@
+/**
+ * @file   owning_ptr.h
+ * @author Nat Goodspeed
+ * @date   2024-09-27
+ * @brief  owning_ptr<T> is like std::unique_ptr<T>, but easier to integrate
+ *
+ * $LicenseInfo:firstyear=2024&license=viewerlgpl$
+ * Copyright (c) 2024, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+#if ! defined(LL_OWNING_PTR_H)
+#define LL_OWNING_PTR_H
+
+#include <functional>
+#include <memory>
+
+/**
+ * owning_ptr<T> adapts std::unique_ptr<T> to make it easier to adopt into
+ * older code using dumb pointers.
+ *
+ * Consider a class Outer with a member Thing* mThing. After the constructor,
+ * each time a method wants to assign to mThing, it must test for nullptr and
+ * destroy the previous Thing instance. During Outer's lifetime, mThing is
+ * passed to legacy domain-specific functions accepting plain Thing*. Finally
+ * the destructor must again test for nullptr and destroy the remaining Thing
+ * instance.
+ *
+ * Multiply that by several different Outer members of different types,
+ * possibly with different domain-specific destructor functions.
+ *
+ * Dropping std::unique_ptr<Thing> into Outer is cumbersome for a several
+ * reasons. First, if Thing requires a domain-specific destructor function,
+ * the unique_ptr declaration of mThing must explicitly state the type of that
+ * function (as a function pointer, for a typical legacy function). Second,
+ * every Thing* assignment to mThing must be changed to mThing.reset(). Third,
+ * every time we call a legacy domain-specific function, we must pass
+ * mThing.get().
+ *
+ * owning_ptr<T> is designed to drop into a situation like this. The domain-
+ * specific destructor function, if any, is passed to its constructor; it need
+ * not be encoded into the pointer type. owning_ptr<T> supports plain pointer
+ * assignment, internally calling std::unique_ptr<T>::reset(). It also
+ * supports implicit conversion to plain T*, to pass the owned pointer to
+ * legacy domain-specific functions.
+ *
+ * Obviously owning_ptr<T> must not be used in situations where ownership of
+ * the referenced object is passed on to another pointer: use std::unique_ptr
+ * for that. Similarly, it is not for shared ownership. It simplifies lifetime
+ * management for classes that currently store (and explicitly destroy) plain
+ * T* pointers.
+ */
+template <typename T>
+class owning_ptr
+{
+    using deleter = std::function<void(T*)>;
+public:
+    owning_ptr(T* p=nullptr, const deleter& d=std::default_delete<T>()):
+        mPtr(p, d)
+    {}
+    void reset(T* p=nullptr) { mPtr.reset(p); }
+    owning_ptr& operator=(T* p) { mPtr.reset(p); return *this; }
+    operator T*() const { return mPtr.get(); }
+    T& operator*() const { return *mPtr; }
+    T* operator->() const { return mPtr.operator->(); }
+
+private:
+    std::unique_ptr<T, deleter> mPtr;
+};
+
+#endif /* ! defined(LL_OWNING_PTR_H) */