DRTVWR-587: Fix LL::apply(function, LLSD array).

We define a specialization of LLSDParam<const char*> to support passing an
LLSD object to a const char* function parameter. Needless to remark, passing
object.asString().c_str() would be Bad: destroying the temporary std::string
returned by asString() would immediately invalidate the pointer returned by
its c_str(). But when you pass LLSDParam<const char*>(object) as the
parameter, that specialization itself stores the std::string so the c_str()
pointer remains valid as long as the LLSDParam object does.

Then there's LLSDParam<LLSD>, used when we don't have the parameter type
available to select the LLSDParam specialization. LLSDParam<LLSD> defines a
templated conversion operator T() that constructs an LLSDParam<T> to provide
the actual parameter value. So far, so good.

The trouble was with the implementation of LLSDParam<LLSD>: it constructed a
_temporary_ LLSDParam<T>, implicitly called its operator T() and immediately
destroyed it. Destroying LLSDParam<const char*> destroyed its stored string,
thus invalidating the c_str() pointer before the target function was entered.

Instead, make LLSDParam<LLSD>::operator T() capture each LLSDParam<T> it
constructs, extending its lifespan to the lifespan of the LLSDParam<LLSD>
instance. For this, derive each LLSDParam specialization from LLSDParamBase, a
trivial base class that simply establishes the virtual destructor. We can then
capture any specialization as a pointer to LLSDParamBase.

Also restore LazyEventAPI tests on Mac.

3 files changed, 60 insertions, 38 deletions

diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt
index 0df113e2c8..e02f69126e 100644
--- a/indra/llcommon/CMakeLists.txt
+++ b/indra/llcommon/CMakeLists.txt
@@ -298,13 +298,7 @@ if (LL_TESTS)
   LL_ADD_INTEGRATION_TEST(bitpack "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(classic_callback "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(commonmisc "" "${test_libs}")
-  if (WINDOWS)
-    # As of 2023-07-27, lazyeventapi.h triggers a bug in older clang,
-    # unfortunately the version we run on our TeamCity Mac build agent. As we
-    # move forward, either with an updated TC agent or GitHub builds, remove
-    # this 'if'.
-    LL_ADD_INTEGRATION_TEST(lazyeventapi "" "${test_libs}")
-  endif ()
+  LL_ADD_INTEGRATION_TEST(lazyeventapi "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(llbase64 "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(llcond "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(lldate "" "${test_libs}")
diff --git a/indra/llcommon/llsdutil.h b/indra/llcommon/llsdutil.h
index cd68f938ea..ad54d1b0be 100644
--- a/indra/llcommon/llsdutil.h
+++ b/indra/llcommon/llsdutil.h
@@ -34,7 +34,9 @@
 #include "llsd.h"
 #include <boost/functional/hash.hpp>
 #include <cassert>
+#include <memory>                   // std::shared_ptr
 #include <type_traits>
+#include <vector>
 
 // U32
 LL_COMMON_API LLSD ll_sd_from_U32(const U32);
@@ -302,6 +304,11 @@ LLSD map(Ts&&... vs)
 /*****************************************************************************
 *   LLSDParam
 *****************************************************************************/
+struct LLSDParamBase
+{
+    virtual ~LLSDParamBase() {}
+};
+
 /**
  * LLSDParam is a customization point for passing LLSD values to function
  * parameters of more or less arbitrary type. LLSD provides a small set of
@@ -319,7 +326,7 @@ LLSD map(Ts&&... vs)
  * @endcode
  */
 template <typename T>
-class LLSDParam
+class LLSDParam: public LLSDParamBase
 {
 public:
     /**
@@ -327,13 +334,13 @@ public:
      * value for later retrieval
      */
     LLSDParam(const LLSD& value):
-        _value(value)
+        value_(value)
     {}
 
-    operator T() const { return _value; }
+    operator T() const { return value_; }
 
 private:
-    T _value;
+    T value_;
 };
 
 /**
@@ -343,10 +350,30 @@ private:
  * specialization.
  */
 template <>
-class LLSDParam<LLSD>
+class LLSDParam<LLSD>: public LLSDParamBase
 {
 private:
     LLSD value_;
+    // LLSDParam<LLSD>::operator T() works by instantiating an LLSDParam<T> on
+    // demand. Returning that engages LLSDParam<T>::operator T(), producing
+    // the desired result. But LLSDParam<const char*> owns a std::string whose
+    // c_str() is returned by its operator const char*(). If we return a temp
+    // LLSDParam<const char*>, the compiler can destroy it right away, as soon
+    // as we've called operator const char*(). That's a problem! That
+    // invalidates the const char* we've just passed to the subject function.
+    // This LLSDParam<LLSD> is presumably guaranteed to survive until the
+    // subject function has returned, so we must ensure that any constructed
+    // LLSDParam<T> lives just as long as this LLSDParam<LLSD> does. Putting
+    // each LLSDParam<T> on the heap and capturing a smart pointer in a vector
+    // works. We would have liked to use std::unique_ptr, but vector entries
+    // must be copyable.
+    // (Alternatively we could assume that every instance of LLSDParam<LLSD>
+    // will be asked for at most ONE conversion. We could store a scalar
+    // std::unique_ptr and, when constructing an new LLSDParam<T>, assert that
+    // the unique_ptr is empty. But some future change in usage patterns, and
+    // consequent failure of that assertion, would be very mysterious. Instead
+    // of explaining how to fix it, just fix it now.)
+    mutable std::vector<std::shared_ptr<LLSDParamBase>> converters_;
 
 public:
     LLSDParam(const LLSD& value): value_(value) {}
@@ -358,7 +385,15 @@ public:
     /// otherwise, instantiate a more specific LLSDParam<T> to convert; that
     /// preserves the existing customization mechanism
     template <typename T>
-    operator T() const { return LLSDParam<std::decay_t<T>>(value_); }
+    operator T() const
+    {
+        // capture 'ptr' with the specific subclass type because converters_
+        // only stores LLSDParamBase pointers
+        auto ptr{ std::make_shared<LLSDParam<std::decay_t<T>>>(value_) };
+        // keep the new converter alive until we ourselves are destroyed
+        converters_.push_back(ptr);
+        return *ptr;
+    }
 };
 
 /**
@@ -375,17 +410,17 @@ public:
  */
 #define LLSDParam_for(T, AS)                    \
 template <>                                     \
-class LLSDParam<T>                              \
+class LLSDParam<T>: public LLSDParamBase        \
 {                                               \
 public:                                         \
     LLSDParam(const LLSD& value):               \
-        _value((T)value.AS())                   \
+        value_((T)value.AS())                   \
     {}                                          \
                                                 \
-    operator T() const { return _value; }       \
+    operator T() const { return value_; }       \
                                                 \
 private:                                        \
-    T _value;                                   \
+    T value_;                                   \
 }
 
 LLSDParam_for(float,        asReal);
@@ -401,31 +436,31 @@ LLSDParam_for(LLSD::Binary, asBinary);
  * safely pass an LLSDParam<const char*>(yourLLSD).
  */
 template <>
-class LLSDParam<const char*>
+class LLSDParam<const char*>: public LLSDParamBase
 {
 private:
     // The difference here is that we store a std::string rather than a const
     // char*. It's important that the LLSDParam object own the std::string.
-    std::string _value;
+    std::string value_;
     // We don't bother storing the incoming LLSD object, but we do have to
-    // distinguish whether _value is an empty string because the LLSD object
+    // distinguish whether value_ is an empty string because the LLSD object
     // contains an empty string or because it's isUndefined().
-    bool _undefined;
+    bool undefined_;
 
 public:
     LLSDParam(const LLSD& value):
-        _value(value),
-        _undefined(value.isUndefined())
+        value_(value),
+        undefined_(value.isUndefined())
     {}
 
-    // The const char* we retrieve is for storage owned by our _value member.
+    // The const char* we retrieve is for storage owned by our value_ member.
     // That's how we guarantee that the const char* is valid for the lifetime
     // of this LLSDParam object. Constructing your LLSDParam in the argument
     // list should ensure that the LLSDParam object will persist for the
     // duration of the function call.
     operator const char*() const
     {
-        if (_undefined)
+        if (undefined_)
         {
             // By default, an isUndefined() LLSD object's asString() method
             // will produce an empty string. But for a function accepting
@@ -435,7 +470,7 @@ public:
             // case, though, no LLSD value could pass NULL.
             return NULL;
         }
-        return _value.c_str();
+        return value_.c_str();
     }
 };
 
@@ -592,7 +627,7 @@ namespace LL
 *   apply(function, LLSD array)
 *****************************************************************************/
 // validate incoming LLSD blob, and return an LLSD array suitable to pass to
-// apply_impl()
+// the function of interest
 LLSD apply_llsd_fix(size_t arity, const LLSD& args);
 
 // Derived from https://stackoverflow.com/a/20441189
diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp
index 40643172ee..b0c532887c 100644
--- a/indra/llcommon/tests/lleventdispatcher_test.cpp
+++ b/indra/llcommon/tests/lleventdispatcher_test.cpp
@@ -178,6 +178,7 @@ struct Vars
     /*-------- Arbitrary-params (non-const, const, static) methods ---------*/
     void methodna(NPARAMSa)
     {
+        DEBUG;
         // Because our const char* param cp might be NULL, and because we
         // intend to capture the value in a std::string, have to distinguish
         // between the NULL value and any non-NULL value. Use a convention
@@ -189,7 +190,7 @@ struct Vars
         else
             vcp = std::string("'") + cp + "'";
 
-        debug()("methodna(", b,
+        this->debug()("methodna(", b,
               ", ", i,
               ", ", f,
               ", ", d,
@@ -227,7 +228,8 @@ struct Vars
 
     void cmethodna(NPARAMSa) const
     {
-        debug()('c', NONL);
+        DEBUG;
+        this->debug()('c', NONL);
         const_cast<Vars*>(this)->methodna(NARGSa);
     }
 
@@ -1200,9 +1202,6 @@ namespace tut
     void object::test<20>()
     {
         set_test_name("call array-style functions with right-size arrays");
-#if defined(_MSC_VER)
-        skip("This test fails on VS");
-#endif
         std::vector<U8> binary;
         for (size_t h(0x01), i(0); i < 5; h+= 0x22, ++i)
         {
@@ -1241,9 +1240,6 @@ namespace tut
     void object::test<21>()
     {
         set_test_name("verify that passing LLSD() to const char* sends NULL");
-#if defined(_MSC_VER)
-        skip("This test fails on VS");
-#endif
 
         ensure_equals("Vars::cp init", v.cp, "");
         work("methodna_map_mdft", LLSDMap("cp", LLSD()));
@@ -1257,9 +1253,6 @@ namespace tut
     template<> template<>
     void object::test<22>()
     {
-#if defined(_MSC_VER)
-        skip("This test fails on VS");
-#endif
         set_test_name("call map-style functions with (full | oversized) (arrays | maps)");
         const char binary[] = "\x99\x88\x77\x66\x55";
         LLSD array_full(LLSDMap