DRTVWR-558: LLEventDispatcher uses LL::apply(), not boost::fusion.

While calling a C++ function with arguments taken from a runtime-variable data
structure necessarily involves a bit of hocus-pocus, the best you can say for
the boost::fusion based implementation is that it worked. Sadly, template
recursion limited its applicability to a handful of function arguments. Now
that we have LL::apply(), use that instead. This implementation is much more
straightforward.

In particular, the LLSDArgsSource class, whose job was to dole out elements of
an LLSD array one at a time for the template recursion, goes away entirely.

Make virtual LLEventDispatcher::DispatchEntry::call() return LLSD instead of
void. All LLEventDispatcher target functions so far have been void; any
function that wants to respond to its invoker must do so explicitly by calling
sendReply() or constructing an LLEventAPI::Response instance. Supporting non-
void functions permits LLEventDispatcher to respond implicitly with the
returned value. Of course this requires a wrapper for void target functions
that returns LLSD::isUndefined().

Break out LLEventDispatcher::reply() from callFail(), so we can reply with
success as well as failure.

Make LLEventDispatcher::try_call_log() prepend the actual leaf class name and
description to any error returned by three-arg try_call(). That try_call()
overload reported "LLEventDispatcher(desc): " for a couple specific errors,
but no others. Hoist to try_call_log() to apply uniformly.

Introduce new try_call_one() method to diagnose name-not-found errors and
catch internal DispatchError and LL::apply_error exceptions. try_call_one()
returns a std::pair, containing either an error message or an LLSD value.

Make try_call_log() and three-arg try_call() accept LLSD 'name' instead of
plain std::string, allowing for the possibility of an array or map. That lets
us extend three-arg try_call() to break out new cases for the function selector
LLSD: isUndefined(), isArray(), isMap() and (current case) scalar String.

If try_call_one() reports an error, log it and try to send reply, as now. If
it returns LLSD::isUndefined(), e.g. from a void target function wrapper, do
nothing. But if it returns an LLSD map, try to send that back to the invoker.
And if it returns an LLSD scalar or array, wrap it in a map with key "data" to
respond to the invoker. Allowing a target function to return its result rather
than explicitly sending it opens the possibility of batched requests
(aggregate 'name') returning batched responses.

Almost every place that constructs LLEventDispatcher's internal DispatchError
exception called stringize() to format the what() string. Simplify calls by
making DispatchError accept variadic arguments and forward to stringize().

Add LL::invoke() to apply.h. Like LL::apply(), this is a (limited) C++14
foreshadowing of std::invoke(), with preprocessor conditionals to switch to
std::invoke() when that's available. Introduce LL::invoke() to handle a
callable that's actually a pointer to method.

Now our C++14 apply() implementation can accept pointer to method, using
invoke() to generalize the actual function call.

Also anticipate std::bind_front() with LL::bind_front(). For apply(func,
std::array) and our extensions apply(func, std::vector) and apply(func, LLSD),
we can't pass a pointer to method as the func unless the second argument
happens to be an array or vector of pointers (or references) to instances of
exactly the right class -- and of course LLSD can't store such at all. It's
tempting to pass std::bind(std::mem_fn(ptr_to_method), instance), but that
won't work: std::bind() requires a value or placeholder for each argument to
pass to the bound function. The bind() expression above would only work for a
nullary method. std::bind_front() would work, but that doesn't arrive until
C++20. Again, once we get there we'll defer to the std:: implementation.

Instead of the generic __cplusplus, check the appropriate feature-test macro
for availability of each of std::invoke(), std::apply() and std::bind_front().

Change apply() error handling from assert() to new LL::apply_error exception.
LLEventDispatcher must be able to intercept apply() errors. Move validation
and synthesis of the relevant error message to new apply.cpp source file.

Add to llptrto.h new LL::get_ref() and LL::get_ptr() template functions to
unify the cases of a calling template accepting either a pointer or a
reference. Wrapping the parameter in either get_ref() or get_ptr() allows
dereferencing the parameter as desired.

Move LL::apply(function, LLSD) argument validation/manipulation to a non-
template function in llsdutil.cpp: no need to replicate that logic in the
template for every CALLABLE specialization.

The trouble with passing bind_front(std::mem_fn(ptr_to_method), instance) to
apply() is that since bind_front() accepts and forwards variadic additional
arguments, apply() can't infer the arity of the bound ptr_to_method. Address
that by introducing apply_n<arity>(function, LLSD), permitting a caller to
infer the arity of ptr_to_method and explicitly pass it to apply_n().

Polish up lleventdispatcher_test.cpp accordingly. Wrong LLSD type and wrong
number of arguments now produce different (somewhat more informative) error
messages. Moreover, passing too many entries in an LLSD array used to work:
the extra arguments used to be ignored. Now we require that the size of the
array match the arity of the target function. Change the too-many-arguments
tests from success testing to error testing.

Replace 'foreach' aka BOOST_FOREACH macro invocations with range 'for'.
Replace STRINGIZE(item0 << item1 << ...) with stringize(item0, item1, ...).

(cherry picked from commit 9c049563b5480bb7e8ed87d9313822595b479c3b)

1 files changed, 130 insertions, 94 deletions

diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp
index 3e45601429..e7e73125a7 100644
--- a/indra/llcommon/lleventdispatcher.cpp
+++ b/indra/llcommon/lleventdispatcher.cpp
@@ -50,69 +50,14 @@
 *****************************************************************************/
 struct DispatchError: public LLException
 {
-    DispatchError(const std::string& what): LLException(what) {}
+    // template constructor involving strings passes all arguments to
+    // stringize() to construct LLException's what() string
+    template <typename... ARGS>
+    DispatchError(const std::string& arg0, ARGS&&... args):
+        LLException(stringize(arg0, std::forward<ARGS>(args)...)) {}
 };
 
 /*****************************************************************************
-*   LLSDArgsSource
-*****************************************************************************/
-/**
- * Store an LLSD array, producing its elements one at a time. It is an error
- * if the consumer requests more elements than the array contains.
- */
-class LL_COMMON_API LLSDArgsSource
-{
-public:
-    LLSDArgsSource(const std::string function, const LLSD& args);
-    ~LLSDArgsSource();
-
-    LLSD next();
-
-    void done() const;
-
-private:
-    std::string _function;
-    LLSD _args;
-    LLSD::Integer _index;
-};
-
-LLSDArgsSource::LLSDArgsSource(const std::string function, const LLSD& args):
-    _function(function),
-    _args(args),
-    _index(0)
-{
-    if (! (_args.isUndefined() || _args.isArray()))
-    {
-        LLTHROW(DispatchError(stringize(_function, " needs an args array instead of ", _args)));
-    }
-}
-
-LLSDArgsSource::~LLSDArgsSource()
-{
-    done();
-}
-
-LLSD LLSDArgsSource::next()
-{
-    if (_index >= _args.size())
-    {
-        LLTHROW(DispatchError(stringize(_function, " requires more arguments than the ",
-                                        _args.size(), " provided: ", _args)));
-    }
-    return _args[_index++];
-}
-
-void LLSDArgsSource::done() const
-{
-    if (_index < _args.size())
-    {
-        LL_WARNS("LLSDArgsSource") << _function << " only consumed " << _index
-                                   << " of the " << _args.size() << " arguments provided: "
-                                   << _args << LL_ENDL;
-    }
-}
-
-/*****************************************************************************
 *   LLSDArgsMapper
 *****************************************************************************/
 /**
@@ -204,7 +149,7 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function,
 {
     if (! (_names.isUndefined() || _names.isArray()))
     {
-        LLTHROW(DispatchError(stringize(function, " names must be an array, not ", names)));
+        LLTHROW(DispatchError(function, " names must be an array, not ", names));
     }
     auto nparams(_names.size());
     // From _names generate _indexes.
@@ -227,8 +172,8 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function,
         // defaults is a (possibly empty) array. Right-align it with names.
         if (ndefaults > nparams)
         {
-            LLTHROW(DispatchError(stringize(function, " names array ", names,
-                                            " shorter than defaults array ", defaults)));
+            LLTHROW(DispatchError(function, " names array ", names,
+                                  " shorter than defaults array ", defaults));
         }
 
         // Offset by which we slide defaults array right to right-align with
@@ -265,14 +210,14 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function,
         }
         if (bogus.size())
         {
-            LLTHROW(DispatchError(stringize(function, " defaults specified for nonexistent params ",
-                                            formatlist(bogus))));
+            LLTHROW(DispatchError(function, " defaults specified for nonexistent params ",
+                                  formatlist(bogus)));
         }
     }
     else
     {
-        LLTHROW(DispatchError(stringize(function, " defaults must be a map or an array, not ",
-                                        defaults)));
+        LLTHROW(DispatchError(function, " defaults must be a map or an array, not ",
+                              defaults));
     }
 }
 
@@ -280,8 +225,8 @@ LLSD LLSDArgsMapper::map(const LLSD& argsmap) const
 {
     if (! (argsmap.isUndefined() || argsmap.isMap() || argsmap.isArray()))
     {
-        LLTHROW(DispatchError(stringize(_function, " map() needs a map or array, not ",
-                                        argsmap)));
+        LLTHROW(DispatchError(_function, " map() needs a map or array, not ",
+                              argsmap));
     }
     // Initialize the args array. Indexing a non-const LLSD array grows it
     // to appropriate size, but we don't want to resize this one on each
@@ -378,8 +323,8 @@ LLSD LLSDArgsMapper::map(const LLSD& argsmap) const
     // by argsmap, that's a problem.
     if (unfilled.size())
     {
-        LLTHROW(DispatchError(stringize(_function, " missing required arguments ",
-                                        formatlist(unfilled), " from ", argsmap)));
+        LLTHROW(DispatchError(_function, " missing required arguments ",
+                              formatlist(unfilled), " from ", argsmap));
     }
 
     // done
@@ -399,6 +344,9 @@ std::string LLSDArgsMapper::formatlist(const LLSD& list)
     return out.str();
 }
 
+/*****************************************************************************
+*   LLEventDispatcher
+*****************************************************************************/
 LLEventDispatcher::LLEventDispatcher(const std::string& desc, const std::string& key):
     mDesc(desc),
     mKey(key)
@@ -409,6 +357,10 @@ LLEventDispatcher::~LLEventDispatcher()
 {
 }
 
+LLEventDispatcher::DispatchEntry::DispatchEntry(const std::string& desc):
+    mDesc(desc)
+{}
+
 /**
  * DispatchEntry subclass used for callables accepting(const LLSD&)
  */
@@ -423,16 +375,17 @@ struct LLEventDispatcher::LLSDDispatchEntry: public LLEventDispatcher::DispatchE
     Callable mFunc;
     LLSD mRequired;
 
-    virtual void call(const std::string& desc, const LLSD& event) const
+    virtual LLSD call(const std::string& desc, const LLSD& event) const
     {
         // Validate the syntax of the event itself.
         std::string mismatch(llsd_matches(mRequired, event));
         if (! mismatch.empty())
         {
-            LLTHROW(DispatchError(stringize(desc, ": bad request: ", mismatch)));
+            LLTHROW(DispatchError(desc, ": bad request: ", mismatch));
         }
         // Event syntax looks good, go for it!
         mFunc(event);
+        return {};
     }
 
     virtual LLSD addMetadata(LLSD meta) const
@@ -455,10 +408,9 @@ struct LLEventDispatcher::ParamsDispatchEntry: public LLEventDispatcher::Dispatc
 
     invoker_function mInvoker;
 
-    virtual void call(const std::string& desc, const LLSD& event) const
+    virtual LLSD call(const std::string&, const LLSD& event) const
     {
-        LLSDArgsSource src(desc, event);
-        mInvoker(boost::bind(&LLSDArgsSource::next, boost::ref(src)));
+        return mInvoker(event);
     }
 };
 
@@ -541,11 +493,11 @@ struct LLEventDispatcher::MapParamsDispatchEntry: public LLEventDispatcher::Para
     LLSD mRequired;
     LLSD mOptional;
 
-    virtual void call(const std::string& desc, const LLSD& event) const
+    virtual LLSD call(const std::string& desc, const LLSD& event) const
     {
         // Just convert from LLSD::Map to LLSD::Array using mMapper, then pass
         // to base-class call() method.
-        ParamsDispatchEntry::call(desc, mMapper.map(event));
+        return ParamsDispatchEntry::call(desc, mMapper.map(event));
     }
 
     virtual LLSD addMetadata(LLSD meta) const
@@ -617,12 +569,18 @@ void LLEventDispatcher::operator()(const LLSD& event) const
 
 void LLEventDispatcher::callFail(const LLSD& event, const std::string& msg) const
 {
+    // pass back a response that includes an "error" key with the message.
+    reply(llsd::map("error", msg), event);
+}
+
+void LLEventDispatcher::reply(const LLSD& response, const LLSD& event) const
+{
     static LLSD::String key{ "reply" };
     if (event.has(key))
     {
-        // Oh good, the incoming event specifies a reply pump -- pass back a
-        // response that includes an "error" key with the message.
-        sendReply(llsd::map("error", msg), event, key);
+        // Oh good, the incoming event specifies a reply pump -- pass back
+        // our response.
+        sendReply(response, event, key);
     }
 }
 
@@ -631,17 +589,30 @@ bool LLEventDispatcher::try_call(const LLSD& event) const
     return try_call_log(mKey, event[mKey], event).empty();
 }
 
+/*==========================================================================*|
+  TODO:
+
+* When mInvoker returns result.isDefined(), sendReply(llsd::map("data", result))
+* When try_call finds name.isArray(), construct response array from
+  dispatching each call, sendReply() as above
+* When try_call finds name.isMap(), construct response map from dispatching
+  each call, sendReply() as above -- note, caller can't care about order
+* Possible future transactional behavior: look up all names before calling any
+
+|*==========================================================================*/
 bool LLEventDispatcher::try_call(const std::string& name, const LLSD& event) const
 {
     return try_call_log(std::string(), name, event).empty();
 }
 
-std::string LLEventDispatcher::try_call_log(const std::string& key, const std::string& name,
+std::string LLEventDispatcher::try_call_log(const std::string& key, const LLSD& name,
                                             const LLSD& event) const
 {
     std::string error{ try_call(key, name, event) };
     if (! error.empty())
     {
+        // If we're a subclass of LLEventDispatcher, e.g. LLEventAPI, report that.
+        error = stringize(LLError::Log::classname(this), "(", mDesc, "): ", error);
         LL_WARNS("LLEventDispatcher") << error << LL_ENDL;
     }
     return error;
@@ -649,33 +620,100 @@ std::string LLEventDispatcher::try_call_log(const std::string& key, const std::s
 
 // This internal method returns empty string if the call succeeded, else
 // non-empty error message.
-std::string LLEventDispatcher::try_call(const std::string& key, const std::string& name,
+std::string LLEventDispatcher::try_call(const std::string& key, const LLSD& name,
                                         const LLSD& event) const
 {
+    if (name.isUndefined())
+    {
+        if (key.empty())
+        {
+            return "attempting to call with no name";
+        }
+        else
+        {
+            return stringize("no ", key);
+        }
+    }
+    else if (name.isArray())
+    {
+        return stringize(key, " array dispatch ", name, " not yet implemented");
+    }
+    else if (name.isMap())
+    {
+        return stringize(key, " map dispatch ", name, " not yet implemented");
+    }
+    else if (! name.isString())
+    {
+        return stringize(key, " bad type ", LLSD::typeString(name.type()), ' ', name,
+                         " -- function names are String");
+    }
+    else                            // name is an LLSD::String
+    {
+        auto success{ try_call_one(key, name, event) };
+        // pretend to unpack
+        std::string& error{ success.first };
+        LLSD& result{ success.second };
+        // did try_call_one() report an error?
+        if (! error.empty())
+        {
+            // if we have a reply key, respond to invoker
+            reply(llsd::map("error", error), event);
+            // now tell caller
+            return error;
+        }
+        // try_call_one() succeeded in calling the target function --
+        // should we reply to invoker?
+        if (result.isUndefined())
+        {
+            // We would get result.isUndefined() if the target function has
+            // void return. In any case, even if the target function returns
+            // LLSD, isUndefined() means "don't bother sending response."
+            return {};
+        }
+        // result.isDefined(): the target function returned something.
+        // Respond to invoker if we have a "reply" key.
+        if (! result.isMap())
+        {
+            // wrap result in a map to play well with sendReply()
+            result = llsd::map("data", result);
+        }
+        reply(result, event);
+        return {};
+    }
+}
+
+std::pair<std::string, LLSD>
+LLEventDispatcher::try_call_one(const std::string& key, const std::string& name,
+                                const LLSD& event) const
+{
     DispatchMap::const_iterator found = mDispatch.find(name);
     if (found == mDispatch.end())
     {
         if (key.empty())
         {
-            return stringize("LLEventDispatcher(", mDesc, "): '", name, "' not found");
+            return { stringize("'", name, "' not found"), {} };
         }
         else
         {
-            return stringize("LLEventDispatcher(", mDesc, "): bad ", key, " value '", name, "'");
+            return { stringize("bad ", key, " value '", name, "'"), {} };
         }
     }
 
     try
     {
         // Found the name, so it's plausible to even attempt the call.
-        found->second->call(stringize("LLEventDispatcher(", mDesc, ") calling '", name, "'"),
-                            event);
+        return { {}, found->second->call(stringize("calling '", name, "'"), event) };
     }
     catch (const DispatchError& err)
     {
-        return err.what();
+        // trouble preparing arguments
+        return { err.what(), {} };
+    }
+    catch (const LL::apply_error& err)
+    {
+        // could also hit runtime errors with LL::apply()
+        return { err.what(), {} };
     }
-    return {};                      // tell caller we were able to call
 }
 
 LLSD LLEventDispatcher::getMetadata(const std::string& name) const
@@ -691,6 +729,9 @@ LLSD LLEventDispatcher::getMetadata(const std::string& name) const
     return found->second->addMetadata(meta);
 }
 
+/*****************************************************************************
+*   LLDispatchListener
+*****************************************************************************/
 LLDispatchListener::LLDispatchListener(const std::string& pumpname, const std::string& key):
     LLEventDispatcher(pumpname, key),
     // Do NOT tweak the passed pumpname. In practice, when someone
@@ -712,8 +753,3 @@ bool LLDispatchListener::process(const LLSD& event)
     (*this)(event);
     return false;
 }
-
-LLEventDispatcher::DispatchEntry::DispatchEntry(const std::string& desc):
-    mDesc(desc)
-{}
-