MAINT-5232: Normalize LLSingleton subclasses.

A shocking number of LLSingleton subclasses had public constructors -- and in
several instances, were being explicitly instantiated independently of the
LLSingleton machinery. This breaks the new LLSingleton dependency-tracking
machinery. It seems only fair that if you say you want an LLSingleton, there
should only be ONE INSTANCE!

Introduce LLSINGLETON() and LLSINGLETON_EMPTY_CTOR() macros. These handle the
friend class LLSingleton<whatevah>;
and explicitly declare a private nullary constructor.

To try to enforce the LLSINGLETON() convention, introduce a new pure virtual
LLSingleton method you_must_use_LLSINGLETON_macro() which is, as you might
suspect, defined by the macro. If you declare an LLSingleton subclass without
using LLSINGLETON() or LLSINGLETON_EMPTY_CTOR() in the class body, you can't
instantiate the subclass for lack of a you_must_use_LLSINGLETON_macro()
implementation -- which will hopefully remind the coder.

Trawl through ALL LLSingleton subclass definitions, sprinkling in
LLSINGLETON() or LLSINGLETON_EMPTY_CTOR() as appropriate. Remove all explicit
constructor declarations, public or private, along with relevant 'friend class
LLSingleton<myself>' declarations. Where destructors are declared, move them
into private section as well. Where the constructor was inline but nontrivial,
move out of class body.

Fix several LLSingleton abuses revealed by making ctors/dtors private:

LLGlobalEconomy was both an LLSingleton and the base class for
LLRegionEconomy, a non-LLSingleton. (Therefore every LLRegionEconomy instance
contained another instance of the LLGlobalEconomy "singleton.") Extract
LLBaseEconomy; LLGlobalEconomy is now a trivial subclass of that.
LLRegionEconomy, as you might suspect, now derives from LLBaseEconomy.

LLToolGrab, an LLSingleton, was also explicitly instantiated by
LLToolCompGun's constructor. Extract LLToolGrabBase, explicitly instantiated,
with trivial subclass LLToolGrab, the LLSingleton instance.

(WARNING: LLToolGrabBase methods have an unnerving tendency to go after
LLToolGrab::getInstance(). I DO NOT KNOW what should be the relationship
between the instance in LLToolCompGun and the LLToolGrab singleton instance.)

LLGridManager declared a variant constructor accepting (const std::string&),
with the comment:
// initialize with an explicity grid file for testing.
As there is no evidence of this being called from anywhere, delete it.

LLChicletBar's constructor accepted an optional (const LLSD&). As the LLSD
parameter wasn't used, and as there is no evidence of it being passed from
anywhere, delete the parameter.

LLViewerWindow::shutdownViews() was checking LLNavigationBar::
instanceExists(), then deleting its getInstance() pointer -- leaving a
dangling LLSingleton instance pointer, a land mine if any subsequent code
should attempt to reference it. Use deleteSingleton() instead.

~LLAppViewer() was calling LLViewerEventRecorder::instance() and then
explicitly calling ~LLViewerEventRecorder() on that instance -- leaving the
LLSingleton instance pointer pointing to an allocated-but-destroyed instance.
Use deleteSingleton() instead.

10 files changed, 98 insertions, 38 deletions

diff --git a/indra/llcommon/llassettype.cpp b/indra/llcommon/llassettype.cpp
index 5ae2df3994..4304db36be 100644
--- a/indra/llcommon/llassettype.cpp
+++ b/indra/llcommon/llassettype.cpp
@@ -63,8 +63,7 @@ struct AssetEntry : public LLDictionaryEntry
 class LLAssetDictionary : public LLSingleton<LLAssetDictionary>,
 						  public LLDictionary<LLAssetType::EType, AssetEntry>
 {
-public:
-	LLAssetDictionary();
+	LLSINGLETON(LLAssetDictionary);
 };
 
 LLAssetDictionary::LLAssetDictionary()
diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h
index 0da7a3a6e4..bbe2d22af4 100644
--- a/indra/llcommon/llcoros.h
+++ b/indra/llcommon/llcoros.h
@@ -85,6 +85,7 @@ class Suspending;
  */
 class LL_COMMON_API LLCoros: public LLSingleton<LLCoros>
 {
+    LLSINGLETON(LLCoros);
 public:
     /// Canonical boost::dcoroutines::coroutine signature we use
     typedef boost::dcoroutines::coroutine<void()> coro;
@@ -175,8 +176,6 @@ public:
     class Future;
 
 private:
-    LLCoros();
-    friend class LLSingleton<LLCoros>;
     friend class llcoro::Suspending;
     friend llcoro::id llcoro::get_id();
     std::string generateDistinctName(const std::string& prefix) const;
diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index a34b50f816..2ef748e3e4 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -374,9 +374,8 @@ namespace
 
 	class Globals : public LLSingleton<Globals>
 	{
+		LLSINGLETON(Globals);
 	public:
-		Globals();
-
 		std::ostringstream messageStream;
 		bool messageStreamInUse;
 
@@ -449,9 +448,8 @@ namespace LLError
 
 	class Settings : public LLSingleton<Settings>
 	{
+		LLSINGLETON(Settings);
 	public:
-		Settings();
-
 		SettingsConfigPtr getSettingsConfig();
 
 		void reset();
@@ -486,8 +484,7 @@ namespace LLError
 		mRecorders.clear();
 	}
 
-	Settings::Settings()
-		: LLSingleton<Settings>(),
+	Settings::Settings():
 		mSettingsConfig(new SettingsConfig())
 	{
 	}
diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h
index ba4fcd766e..6c8b66f596 100644
--- a/indra/llcommon/llevents.h
+++ b/indra/llcommon/llevents.h
@@ -209,7 +209,7 @@ class LLEventPump;
  */
 class LL_COMMON_API LLEventPumps: public LLSingleton<LLEventPumps>
 {
-    friend class LLSingleton<LLEventPumps>;
+    LLSINGLETON(LLEventPumps);
 public:
     /**
      * Find or create an LLEventPump instance with a specific name. We return
@@ -252,7 +252,6 @@ private:
     void unregister(const LLEventPump&);
 
 private:
-    LLEventPumps();
     ~LLEventPumps();
 
 testable:
diff --git a/indra/llcommon/llinitdestroyclass.h b/indra/llcommon/llinitdestroyclass.h
index 9c66211475..5f979614fe 100644
--- a/indra/llcommon/llinitdestroyclass.h
+++ b/indra/llcommon/llinitdestroyclass.h
@@ -78,9 +78,7 @@ class LLInitClassList :
 	public LLCallbackRegistry, 
 	public LLSingleton<LLInitClassList>
 {
-	friend class LLSingleton<LLInitClassList>;
-private:
-	LLInitClassList() {}
+	LLSINGLETON_EMPTY_CTOR(LLInitClassList);
 };
 
 /**
@@ -94,9 +92,7 @@ class LLDestroyClassList :
 	public LLCallbackRegistry, 
 	public LLSingleton<LLDestroyClassList>
 {
-	friend class LLSingleton<LLDestroyClassList>;
-private:
-	LLDestroyClassList() {}
+	LLSINGLETON_EMPTY_CTOR(LLDestroyClassList);
 };
 
 /**
diff --git a/indra/llcommon/llpounceable.h b/indra/llcommon/llpounceable.h
index 77b711bdc6..0421ce966a 100644
--- a/indra/llcommon/llpounceable.h
+++ b/indra/llcommon/llpounceable.h
@@ -76,7 +76,8 @@ template <typename T>
 class LLPounceableQueueSingleton:
     public LLSingleton<LLPounceableQueueSingleton<T> >
 {
-private:
+    LLSINGLETON_EMPTY_CTOR(LLPounceableQueueSingleton);
+
     typedef LLPounceableTraits<T, LLPounceableStatic> traits;
     typedef typename traits::owner_ptr owner_ptr;
     typedef typename traits::signal_t signal_t;
diff --git a/indra/llcommon/llregistry.h b/indra/llcommon/llregistry.h
index fde729f8f9..750fe9fdc8 100644
--- a/indra/llcommon/llregistry.h
+++ b/indra/llcommon/llregistry.h
@@ -247,7 +247,10 @@ class LLRegistrySingleton
 	:	public LLRegistry<KEY, VALUE, COMPARATOR>,
 		public LLSingleton<DERIVED_TYPE>
 {
-	friend class LLSingleton<DERIVED_TYPE>;
+	// This LLRegistrySingleton doesn't use LLSINGLETON(LLRegistrySingleton)
+	// because the concrete class is actually DERIVED_TYPE, not
+	// LLRegistrySingleton. So each concrete subclass needs
+	// LLSINGLETON(whatever) -- not this intermediate base class.
 public:
 	typedef LLRegistry<KEY, VALUE, COMPARATOR>		registry_t;
 	typedef const KEY&								ref_const_key_t;
diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp
index 24ccc8ddb4..9025e53bb2 100644
--- a/indra/llcommon/llsingleton.cpp
+++ b/indra/llcommon/llsingleton.cpp
@@ -57,8 +57,7 @@ bool oktolog();
 class LLSingletonBase::MasterList:
     public LLSingleton<LLSingletonBase::MasterList>
 {
-private:
-    friend class LLSingleton<LLSingletonBase::MasterList>;
+    LLSINGLETON_EMPTY_CTOR(MasterList);
 
 public:
     // No need to make this private with accessors; nobody outside this source
diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h
index 92fba4c1a8..1b915dfd6e 100644
--- a/indra/llcommon/llsingleton.h
+++ b/indra/llcommon/llsingleton.h
@@ -223,7 +223,13 @@ LLSingletonBase::LLSingletonBase(tag<DERIVED_TYPE>):
  * Derive your class from LLSingleton, passing your subclass name as
  * LLSingleton's template parameter, like so:
  *
- *   class Foo: public LLSingleton<Foo>{};
+ *   class Foo: public LLSingleton<Foo>
+ *   {
+ *       // use this macro at start of every LLSingleton subclass
+ *       LLSINGLETON(Foo);
+ *   public:
+ *       // ...
+ *   };
  *
  *   Foo& instance = Foo::instance();
  *
@@ -279,6 +285,16 @@ private:
         return new DERIVED_TYPE();
     }
 
+    // We know of no way to instruct the compiler that every subclass
+    // constructor MUST be private. However, we can make the LLSINGLETON()
+    // macro both declare a private constructor and provide the required
+    // friend declaration. How can we ensure that every subclass uses
+    // LLSINGLETON()? By making that macro provide a definition for this pure
+    // virtual method. If you get "can't instantiate class due to missing pure
+    // virtual method" for this method, then add LLSINGLETON(yourclass) in the
+    // subclass body.
+    virtual void you_must_use_LLSINGLETON_macro() = 0;
+
     // stores pointer to singleton instance
     struct SingletonLifetimeManager
     {
@@ -450,4 +466,54 @@ private:
 template<typename T>
 typename LLSingleton<T>::SingletonData LLSingleton<T>::sData;
 
+/**
+ * Use LLSINGLETON(Foo); at the start of an LLSingleton<Foo> subclass body
+ * when you want to declare an out-of-line constructor:
+ *
+ * @code
+ *   class Foo: public LLSingleton<Foo>
+ *   {
+ *       // use this macro at start of every LLSingleton subclass
+ *       LLSINGLETON(Foo);
+ *   public:
+ *       // ...
+ *   };
+ *   // ...
+ *   [inline]
+ *   Foo::Foo() { ... }
+ * @endcode
+ *
+ * Unfortunately, this mechanism does not permit you to define even a simple
+ * (but nontrivial) constructor within the class body. If it's literally
+ * trivial, use LLSINGLETON_EMPTY_CTOR(); if not, use LLSINGLETON() and define
+ * the constructor outside the class body. If you must define it in a header
+ * file, use 'inline' (unless it's a template class) to avoid duplicate-symbol
+ * errors at link time.
+ */
+#define LLSINGLETON(DERIVED_CLASS)                                      \
+private:                                                                \
+    /* implement LLSingleton pure virtual method whose sole purpose */  \
+    /* is to remind people to use this macro */                         \
+    virtual void you_must_use_LLSINGLETON_macro() {}                    \
+    friend class LLSingleton<DERIVED_CLASS>;                            \
+    DERIVED_CLASS()
+
+/**
+ * Use LLSINGLETON_EMPTY_CTOR(Foo); at the start of an LLSingleton<Foo>
+ * subclass body when the constructor is trivial:
+ *
+ * @code
+ *   class Foo: public LLSingleton<Foo>
+ *   {
+ *       // use this macro at start of every LLSingleton subclass
+ *       LLSINGLETON_EMPTY_CTOR(Foo);
+ *   public:
+ *       // ...
+ *   };
+ * @endcode
+ */
+#define LLSINGLETON_EMPTY_CTOR(DERIVED_CLASS)                           \
+    /* LLSINGLETON() is carefully implemented to permit exactly this */ \
+    LLSINGLETON(DERIVED_CLASS) {}
+
 #endif
diff --git a/indra/llcommon/tests/llsingleton_test.cpp b/indra/llcommon/tests/llsingleton_test.cpp
index a05f650f25..56886bc73f 100644
--- a/indra/llcommon/tests/llsingleton_test.cpp
+++ b/indra/llcommon/tests/llsingleton_test.cpp
@@ -34,21 +34,22 @@
 // Capture execution sequence by appending to log string.
 std::string sLog;
 
-#define DECLARE_CLASS(CLS)                      \
-struct CLS: public LLSingleton<CLS>             \
-{                                               \
-    static enum dep_flag {                      \
-        DEP_NONE, /* no dependency */           \
-        DEP_CTOR, /* dependency in ctor */      \
+#define DECLARE_CLASS(CLS)                          \
+struct CLS: public LLSingleton<CLS>                 \
+{                                                   \
+    LLSINGLETON(CLS);                               \
+    ~CLS();                                         \
+public:                                             \
+    static enum dep_flag {                          \
+        DEP_NONE, /* no dependency */               \
+        DEP_CTOR, /* dependency in ctor */          \
         DEP_INIT  /* dependency in initSingleton */ \
-    } sDepFlag;                                 \
-                                                \
-    CLS();                                      \
-    void initSingleton();                       \
-    void cleanupSingleton();                    \
-    ~CLS();                                     \
-};                                              \
-                                                \
+    } sDepFlag;                                     \
+                                                    \
+    void initSingleton();                           \
+    void cleanupSingleton();                        \
+};                                                  \
+                                                    \
 CLS::dep_flag CLS::sDepFlag = DEP_NONE
 
 DECLARE_CLASS(A);
@@ -93,7 +94,7 @@ namespace tut
         // We need a class created with the LLSingleton template to test with.
         class LLSingletonTest: public LLSingleton<LLSingletonTest>
         {
-
+            LLSINGLETON_EMPTY_CTOR(LLSingletonTest);
         };
     };