From b26e516d2b93a442d09f5c3b1b4d8d60139c42f5 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 6 Apr 2022 17:34:28 -0400 Subject: DRTVWR-558: Change LLEventDispatcher error action (also LLEventAPI). Originally the LLEventAPI mechanism was primarily used for VITA testing. In that case it was okay for the viewer to crash with LL_ERRS if the test script passed a bad request. With puppetry, hopefully new LEAP scripts will be written to engage LLEventAPIs in all sorts of interesting ways. Change error handling from LL_ERRS to LL_WARNS. Furthermore, if the incoming request contains a "reply" key, send back an error response to the requester. Update lleventdispatcher_test.cpp accordingly. (cherry picked from commit de0539fcbe815ceec2041ecc9981e3adf59f2806) --- indra/llcommon/tests/lleventdispatcher_test.cpp | 62 ++++++++++++++++++------- 1 file changed, 45 insertions(+), 17 deletions(-) (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 9da1ecfd67..82a0ddf61b 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -20,6 +20,7 @@ #include "../test/lltut.h" #include "llsd.h" #include "llsdutil.h" +#include "llevents.h" #include "stringize.h" #include "tests/wrapllerrs.h" #include "../test/catch_and_store_what_in.h" @@ -644,12 +645,45 @@ namespace tut outer.find(inner) != std::string::npos); } - void call_exc(const std::string& func, const LLSD& args, const std::string& exc_frag) + std::string call_exc(const std::string& func, const LLSD& args, const std::string& exc_frag) { - std::string threw = catch_what([this, &func, &args](){ - work(func, args); - }); - ensure_has(threw, exc_frag); + // This method was written when LLEventDispatcher responded to + // name or argument errors with LL_ERRS, hence the name: we used + // to have to intercept LL_ERRS by making it throw. Now we set up + // to catch an error response instead. But -- for that we need to + // be able to sneak a "reply" key into args, which must be a Map. + if (! (args.isUndefined() or args.isMap())) + fail(stringize("can't test call_exc() with ", args)); + LLEventStream replypump("reply"); + LLSD reply; + LLTempBoundListener bound{ + replypump.listen( + "listener", + [&reply](const LLSD& event) + { + reply = event; + return false; + }) }; + LLSD modargs{ args }; + modargs["reply"] = replypump.getName(); + if (func.empty()) + { + work(modargs); + } + else + { + work(func, modargs); + } + ensure("no error response", reply.has("error")); + ensure_has(reply["error"], exc_frag); + return reply["error"]; + } + + void call_logerr(const std::string& func, const LLSD& args, const std::string& frag) + { + CaptureLog capture; + work(func, args); + capture.messageWith(frag); } LLSD getMetadata(const std::string& name) @@ -1031,13 +1065,7 @@ namespace tut { set_test_name("call with bad name"); call_exc("freek", LLSD(), "not found"); - // We don't have a comparable helper function for the one-arg - // operator() method, and it's not worth building one just for this - // case. Write it out. - std::string threw = catch_what([this](){ - work(LLSDMap("op", "freek")); - }); - ensure_has(threw, "bad"); + std::string threw = call_exc("", LLSDMap("op", "freek"), "bad"); ensure_has(threw, "op"); ensure_has(threw, "freek"); } @@ -1087,7 +1115,7 @@ namespace tut ensure_equals("answer mismatch", tr.llsd, answer); // Should NOT be able to pass 'answer' to Callables registered // with 'required'. - call_exc(tr.name_req, answer, "bad request"); + call_logerr(tr.name_req, answer, "bad request"); // But SHOULD be able to pass 'matching' to Callables registered // with 'required'. work(tr.name_req, matching); @@ -1107,11 +1135,11 @@ namespace tut // args. We should only need to engage it for one map-style // registration and one array-style registration. std::string array_exc("needs an args array"); - call_exc("free0_array", 17, array_exc); - call_exc("free0_array", LLSDMap("pi", 3.14), array_exc); + call_logerr("free0_array", 17, array_exc); + call_logerr("free0_array", LLSDMap("pi", 3.14), array_exc); std::string map_exc("needs a map"); - call_exc("free0_map", 17, map_exc); + call_logerr("free0_map", 17, map_exc); // Passing an array to a map-style function works now! No longer an // error case! // call_exc("free0_map", LLSDArray("a")("b"), map_exc); @@ -1158,7 +1186,7 @@ namespace tut { foreach(const llsd::MapEntry& e, inMap(funcsab)) { - call_exc(e.second, tooshort, "requires more arguments"); + call_logerr(e.second, tooshort, "requires more arguments"); } } } -- cgit v1.3 From 07c5645f5f9130a7fc338df0bc2bb791d43bd702 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 22 Dec 2022 14:53:29 -0500 Subject: 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(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) --- indra/llcommon/CMakeLists.txt | 1 + indra/llcommon/apply.cpp | 29 +++ indra/llcommon/apply.h | 100 +++++++++- indra/llcommon/lleventdispatcher.cpp | 224 ++++++++++++--------- indra/llcommon/lleventdispatcher.h | 255 ++++++++---------------- indra/llcommon/llptrto.h | 88 +++++++- indra/llcommon/llsdutil.cpp | 35 ++++ indra/llcommon/llsdutil.h | 49 ++--- indra/llcommon/tests/lleventdispatcher_test.cpp | 141 ++++++------- 9 files changed, 543 insertions(+), 379 deletions(-) create mode 100644 indra/llcommon/apply.cpp (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 941d2d7baf..33e8301e12 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -16,6 +16,7 @@ include(Tracy) set(llcommon_SOURCE_FILES + apply.cpp indra_constants.cpp lazyeventapi.cpp llallocator.cpp diff --git a/indra/llcommon/apply.cpp b/indra/llcommon/apply.cpp new file mode 100644 index 0000000000..417e23d3b4 --- /dev/null +++ b/indra/llcommon/apply.cpp @@ -0,0 +1,29 @@ +/** + * @file apply.cpp + * @author Nat Goodspeed + * @date 2022-12-21 + * @brief Implementation for apply. + * + * $LicenseInfo:firstyear=2022&license=viewerlgpl$ + * Copyright (c) 2022, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "apply.h" +// STL headers +// std headers +// external library headers +// other Linden headers +#include "stringize.h" + +void LL::apply_validate_size(size_t size, size_t arity) +{ + if (size != arity) + { + LLTHROW(apply_error(stringize("LL::apply(func(", arity, " args), " + "std::vector(", size, " elements))"))); + } +} diff --git a/indra/llcommon/apply.h b/indra/llcommon/apply.h index 9f4c268895..357887dfd7 100644 --- a/indra/llcommon/apply.h +++ b/indra/llcommon/apply.h @@ -12,9 +12,11 @@ #if ! defined(LL_APPLY_H) #define LL_APPLY_H +#include "llexception.h" #include -#include +#include // std::mem_fn() #include +#include // std::is_member_pointer namespace LL { @@ -56,9 +58,39 @@ namespace LL (ARGS)) /***************************************************************************** -* apply(function, tuple) +* invoke() *****************************************************************************/ -#if __cplusplus >= 201703L +#if __cpp_lib_invoke >= 201411L + +// C++17 implementation +using std::invoke; + +#else // no std::invoke + +// Use invoke() to handle pointer-to-method: +// derived from https://stackoverflow.com/a/38288251 +template::type>::value, + int>::type = 0 > +auto invoke(Fn&& f, Args&&... args) +{ + return std::mem_fn(f)(std::forward(args)...); +} + +template::type>::value, + int>::type = 0 > +auto invoke(Fn&& f, Args&&... args) +{ + return std::forward(f)(std::forward(args)...); +} + +#endif // no std::invoke + +/***************************************************************************** +* apply(function, tuple); apply(function, array) +*****************************************************************************/ +#if __cpp_lib_apply >= 201603L // C++17 implementation using std::apply; @@ -71,7 +103,7 @@ template auto apply_impl(CALLABLE&& func, const std::tuple& args, std::index_sequence) { // call func(unpacked args) - return std::forward(func)(std::move(std::get(args))...); + return invoke(std::forward(func), std::get(args)...); } template @@ -85,11 +117,6 @@ auto apply(CALLABLE&& func, const std::tuple& args) std::index_sequence_for{}); } -#endif // C++14 - -/***************************************************************************** -* apply(function, std::array) -*****************************************************************************/ // per https://stackoverflow.com/a/57510428/5533635 template auto apply(CALLABLE&& func, const std::array& args) @@ -97,6 +124,50 @@ auto apply(CALLABLE&& func, const std::array& args) return apply(std::forward(func), std::tuple_cat(args)); } +#endif // C++14 + +/***************************************************************************** +* bind_front() +*****************************************************************************/ +// To invoke a non-static member function with a tuple, you need a callable +// that binds your member function with an instance pointer or reference. +// std::bind_front() is perfect: std::bind_front(&cls::method, instance). +// Unfortunately bind_front() only enters the standard library in C++20. +#if __cpp_lib_bind_front >= 201907L + +// C++20 implementation +using std::bind_front; + +#else // no std::bind_front() + +template::type>::value, + int>::type = 0 > +auto bind_front(Fn&& f, Args&&... args) +{ + // Don't use perfect forwarding for f or args: we must bind them for later. + return [f, pfx_args=std::make_tuple(args...)] + (auto&&... sfx_args) + { + // Use perfect forwarding for sfx_args because we use them as soon as + // we receive them. + return apply( + f, + std::tuple_cat(pfx_args, + std::make_tuple(std::forward(sfx_args)...))); + }; +} + +template::type>::value, + int>::type = 0 > +auto bind_front(Fn&& f, Args&&... args) +{ + return bind_front(std::mem_fn(std::forward(f)), std::forward(args)...); +} + +#endif // C++20 with std::bind_front() + /***************************************************************************** * apply(function, std::vector) *****************************************************************************/ @@ -108,6 +179,15 @@ auto apply_impl(CALLABLE&& func, const std::vector& args, std::index_sequence std::make_tuple(args[I]...)); } +// produce suitable error if apply(func, vector) is the wrong size for func() +void apply_validate_size(size_t size, size_t arity); + +/// possible exception from apply() validation +struct apply_error: public LLException +{ + apply_error(const std::string& what): LLException(what) {} +}; + /** * apply(function, std::vector) goes beyond C++17 std::apply(). For this case * @a function @emph cannot be variadic: the compiler must know at compile @@ -117,7 +197,7 @@ template auto apply(CALLABLE&& func, const std::vector& args) { constexpr auto arity = boost::function_traits::arity; - assert(args.size() == arity); + apply_validate_size(args.size(), arity); return apply_impl(std::forward(func), args, std::make_index_sequence()); 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,68 +50,13 @@ *****************************************************************************/ 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 + DispatchError(const std::string& arg0, ARGS&&... args): + LLException(stringize(arg0, std::forward(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 @@ -616,13 +568,19 @@ 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 +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) -{} - diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index 09b786b69e..cebce618df 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -27,54 +27,23 @@ * * Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA * $/LicenseInfo$ - * - * The invoker machinery that constructs a boost::fusion argument list for use - * with boost::fusion::invoke() is derived from - * http://www.boost.org/doc/libs/1_45_0/libs/function_types/example/interpreter.hpp - * whose license information is copied below: - * - * "(C) Copyright Tobias Schwinger - * - * Use modification and distribution are subject to the boost Software License, - * Version 1.0. (See http://www.boost.org/LICENSE_1_0.txt)." */ #if ! defined(LL_LLEVENTDISPATCHER_H) #define LL_LLEVENTDISPATCHER_H -// nil is too generic a term to be allowed to be a global macro. In -// particular, boost::fusion defines a 'class nil' (properly encapsulated in a -// namespace) that a global 'nil' macro breaks badly. -#if defined(nil) -// Capture the value of the macro 'nil', hoping int is an appropriate type. -static const auto nil_(nil); -// Now forget the macro. -#undef nil -// Finally, reintroduce 'nil' as a properly-scoped alias for the previously- -// defined const 'nil_'. Make it static since otherwise it produces duplicate- -// symbol link errors later. -static const auto& nil(nil_); -#endif - -#include #include +#include #include -#include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include #include // std::function #include // std::unique_ptr #include #include +#include #include "llevents.h" +#include "llptrto.h" #include "llsdutil.h" class LLSD; @@ -94,8 +63,7 @@ public: /// @name Register functions accepting(const LLSD&) //@{ - /// Accept any C++ callable with the right signature, typically a - /// boost::bind() expression + /// Accept any C++ callable with the right signature typedef std::function Callable; /** @@ -126,7 +94,7 @@ public: /** * Special case: a subclass of this class can pass an unbound member * function pointer (of an LLEventDispatcher subclass) without explicitly - * specifying the boost::bind() expression. The passed @a method + * specifying a std::bind() expression. The passed @a method * accepts a single LLSD value, presumably containing other parameters. */ template @@ -158,10 +126,6 @@ public: * Register a free function with arbitrary parameters. (This also works * for static class methods.) * - * @note This supports functions with up to about 6 parameters -- after - * that you start getting dismaying compile errors in which - * boost::fusion::joint_view is mentioned a surprising number of times. - * * When calling this name, pass an LLSD::Array. Each entry in turn will be * converted to the corresponding parameter type using LLSDParam. */ @@ -175,10 +139,6 @@ public: /** * Register a nonstatic class method with arbitrary parameters. * - * @note This supports functions with up to about 6 parameters -- after - * that you start getting dismaying compile errors in which - * boost::fusion::joint_view is mentioned a surprising number of times. - * * To cover cases such as a method on an LLSingleton we don't yet want to * instantiate, instead of directly storing an instance pointer, accept a * nullary callable returning a pointer/reference to the desired class @@ -201,10 +161,6 @@ public: * Register a free function with arbitrary parameters. (This also works * for static class methods.) * - * @note This supports functions with up to about 6 parameters -- after - * that you start getting dismaying compile errors in which - * boost::fusion::joint_view is mentioned a surprising number of times. - * * Pass an LLSD::Array of parameter names, and optionally another * LLSD::Array of default parameter values, a la LLSDArgsMapper. * @@ -222,10 +178,6 @@ public: /** * Register a nonstatic class method with arbitrary parameters. * - * @note This supports functions with up to about 6 parameters -- after - * that you start getting dismaying compile errors in which - * boost::fusion::joint_view is mentioned a surprising number of times. - * * To cover cases such as a method on an LLSingleton we don't yet want to * instantiate, instead of directly storing an instance pointer, accept a * nullary callable returning a pointer/reference to the desired class @@ -290,7 +242,7 @@ private: std::string mDesc; - virtual void call(const std::string& desc, const LLSD& event) const = 0; + virtual LLSD call(const std::string& desc, const LLSD& event) const = 0; virtual LLSD addMetadata(LLSD) const = 0; }; typedef std::map > DispatchMap; @@ -322,17 +274,28 @@ private: void addMethod(const std::string& name, const std::string& desc, const METHOD& method, const LLSD& required) { - CLASS* downcast = static_cast(this); - add(name, desc, boost::bind(method, downcast, _1), required); + CLASS* downcast = dynamic_cast(this); + if (! downcast) + { + addFail(name, typeid(CLASS).name()); + } + else + { + add(name, desc, std::bind(method, downcast, std::placeholders::_1), required); + } } void addFail(const std::string& name, const std::string& classname) const; - std::string try_call_log(const std::string& key, const std::string& name, + std::string try_call_log(const std::string& key, const LLSD& name, const LLSD& event) const; - std::string try_call(const std::string& key, const std::string& name, + std::string try_call(const std::string& key, const LLSD& name, const LLSD& event) const; + // returns either (empty string, LLSD) or (error message, isUndefined) + std::pair + try_call_one(const std::string& key, const std::string& name, const LLSD& event) const; // Implement "it is an error" semantics for attempted call operations: if // the incoming event includes a "reply" key, log and send an error reply. void callFail(const LLSD& event, const std::string& msg) const; + void reply(const LLSD& response, const LLSD& event) const; std::string mDesc, mKey; DispatchMap mDispatch; @@ -347,20 +310,8 @@ private: struct ArrayParamsDispatchEntry; struct MapParamsDispatchEntry; - // Step 2 of parameter analysis. Instantiating invoker - // implicitly sets its From and To parameters to the (compile time) begin - // and end iterators over that function's parameter types. - template< typename Function - , class From = typename boost::mpl::begin< boost::function_types::parameter_types >::type - , class To = typename boost::mpl::end< boost::function_types::parameter_types >::type - > - struct invoker; - - // deliver LLSD arguments one at a time - typedef std::function args_source; - // obtain args from an args_source to build param list and call target - // function - typedef std::function invoker_function; + // call target function with args from LLSD array + typedef std::function invoker_function; template invoker_function make_invoker(Function f); @@ -375,92 +326,17 @@ private: const invoker_function& invoker, const LLSD& params, const LLSD& defaults); + template + struct ReturnLLSD; }; /***************************************************************************** * LLEventDispatcher template implementation details *****************************************************************************/ -// Step 3 of parameter analysis, the recursive case. -template -struct LLEventDispatcher::invoker -{ - template - struct remove_cv_ref - : boost::remove_cv< typename boost::remove_reference::type > - { }; - - // apply() accepts an arbitrary boost::fusion sequence as args. It - // examines the next parameter type in the parameter-types sequence - // bounded by From and To, obtains the next LLSD object from the passed - // args_source and constructs an LLSDParam of appropriate type to try - // to convert the value. It then recurs with the next parameter-types - // iterator, passing the args sequence thus far. - template - static inline - void apply(Function func, const args_source& argsrc, Args const & args) - { - typedef typename boost::mpl::deref::type arg_type; - typedef typename boost::mpl::next::type next_iter_type; - typedef typename remove_cv_ref::type plain_arg_type; - - invoker::apply - ( func, argsrc, boost::fusion::push_back(args, LLSDParam(argsrc()))); - } - - // Special treatment for instance (first) parameter of a non-static member - // function. Accept the instance-getter callable, calling that to produce - // the first args value. Since we know we're at the top of the recursion - // chain, we need not also require a partial args sequence from our caller. - template - static inline - void method_apply(Function func, const args_source& argsrc, const InstanceGetter& getter) - { - typedef typename boost::mpl::next::type next_iter_type; - - // Instead of grabbing the first item from argsrc and making an - // LLSDParam of it, call getter() and pass that as the instance param. - invoker::apply - ( func, argsrc, boost::fusion::push_back(boost::fusion::nil(), bindable(getter()))); - } - - template - static inline - auto bindable(T&& value, - typename std::enable_if::value, bool>::type=true) - { - // if passed a pointer, just return that pointer - return std::forward(value); - } - - template - static inline - auto bindable(T&& value, - typename std::enable_if::value, bool>::type=true) - { - // if passed a reference, wrap it for binding - return std::ref(std::forward(value)); - } -}; - -// Step 4 of parameter analysis, the leaf case. When the general -// invoker logic has advanced From until it matches To, -// the compiler will pick this template specialization. -template -struct LLEventDispatcher::invoker -{ - // the argument list is complete, now call the function - template - static inline - void apply(Function func, const args_source&, Args const & args) - { - boost::fusion::invoke(func, args); - } -}; - template void LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f) { - // Construct an invoker_function, a callable accepting const args_source&. + // Construct an invoker_function, a callable accepting const LLSD&. // Add to DispatchMap an ArrayParamsDispatchEntry that will handle the // caller's LLSD::Array. addArrayParamsDispatchEntry(name, desc, make_invoker(f), @@ -493,33 +369,76 @@ void LLEventDispatcher::add(const std::string& name, const std::string& desc, Me addMapParamsDispatchEntry(name, desc, make_invoker(f, getter), params, defaults); } +// general case, when f() has a non-void return type +template +struct LLEventDispatcher::ReturnLLSD +{ + template + LLSD operator()(Function f, const LLSD& args) + { + return { LL::apply(f, args) }; + } + + template + LLSD operator()(Method f, const InstanceGetter& getter, const LLSD& args) + { + constexpr auto arity = boost::function_types::function_arity< + typename std::remove_reference::type>::value - 1; + + // Use bind_front() to bind the method to (a pointer to) the object + // returned by getter(). It's okay to capture and bind a pointer + // because this bind_front() object will last only as long as this + // operator() call. + return { LL::apply_n(LL::bind_front(f, LL::get_ptr(getter())), args) }; + } +}; + +// specialize for void return type +template <> +struct LLEventDispatcher::ReturnLLSD +{ + template + LLSD operator()(Function f, const LLSD& args) + { + LL::apply(f, args); + return {}; + } + + template + LLSD operator()(Method f, const InstanceGetter& getter, const LLSD& args) + { + constexpr auto arity = boost::function_types::function_arity< + typename std::remove_reference::type>::value - 1; + + // Use bind_front() to bind the method to (a pointer to) the object + // returned by getter(). It's okay to capture and bind a pointer + // because this bind_front() object will last only as long as this + // operator() call. + LL::apply_n(LL::bind_front(f, LL::get_ptr(getter())), args); + return {}; + } +}; + template LLEventDispatcher::invoker_function LLEventDispatcher::make_invoker(Function f) { - // Step 1 of parameter analysis, the top of the recursion. Passing a - // suitable f (see add()'s enable_if condition) to this method causes it - // to infer the function type; specifying that function type to invoker<> - // causes it to fill in the begin/end MPL iterators over the function's - // list of parameter types. - // While normally invoker::apply() could infer its template type from the - // boost::fusion::nil parameter value, here we must be explicit since - // we're boost::bind()ing it rather than calling it directly. - return boost::bind(&invoker::template apply, - f, - _1, - boost::fusion::nil()); + return [f](const LLSD& args) + { + return ReturnLLSD::type>() + (f, args); + }; } template LLEventDispatcher::invoker_function LLEventDispatcher::make_invoker(Method f, const InstanceGetter& getter) { - // Use invoker::method_apply() to treat the instance (first) arg specially. - return boost::bind(&invoker::template method_apply, - f, - _1, - getter); + return [f, getter](const LLSD& args) + { + return ReturnLLSD::type>() + (f, getter, args); + }; } /***************************************************************************** diff --git a/indra/llcommon/llptrto.h b/indra/llcommon/llptrto.h index 4082e30de6..9ef279fdbf 100644 --- a/indra/llcommon/llptrto.h +++ b/indra/llcommon/llptrto.h @@ -33,9 +33,12 @@ #include "llpointer.h" #include "llrefcount.h" // LLRefCount +#include +#include #include #include -#include +#include // std::shared_ptr, std::unique_ptr +#include /** * LLPtrTo::type is either of two things: @@ -55,14 +58,14 @@ struct LLPtrTo /// specialize for subclasses of LLRefCount template -struct LLPtrTo >::type> +struct LLPtrTo::value >::type> { typedef LLPointer type; }; /// specialize for subclasses of LLThreadSafeRefCount template -struct LLPtrTo >::type> +struct LLPtrTo::value >::type> { typedef LLPointer type; }; @@ -83,4 +86,83 @@ struct LLRemovePointer< LLPointer > typedef SOMECLASS type; }; +namespace LL +{ + +/***************************************************************************** +* get_ref() +*****************************************************************************/ + template + struct GetRef + { + // return const ref or non-const ref, depending on whether we can bind + // a non-const lvalue ref to the argument + const auto& operator()(const T& obj) const { return obj; } + auto& operator()(T& obj) const { return obj; } + }; + + template + struct GetRef + { + const auto& operator()(const T* ptr) const { return *ptr; } + }; + + template + struct GetRef + { + auto& operator()(T* ptr) const { return *ptr; } + }; + + template + struct GetRef< LLPointer > + { + auto& operator()(LLPointer ptr) const { return *ptr; } + }; + + /// whether we're passed a pointer or a reference, return a reference + template + auto& get_ref(T& ptr_or_ref) + { + return GetRef::type>()(ptr_or_ref); + } + + template + const auto& get_ref(const T& ptr_or_ref) + { + return GetRef::type>()(ptr_or_ref); + } + +/***************************************************************************** +* get_ptr() +*****************************************************************************/ + // if T is any pointer type we recognize, return it unchanged + template + const T* get_ptr(const T* ptr) { return ptr; } + + template + T* get_ptr(T* ptr) { return ptr; } + + template + const std::shared_ptr& get_ptr(const std::shared_ptr& ptr) { return ptr; } + + template + const std::unique_ptr& get_ptr(const std::unique_ptr& ptr) { return ptr; } + + template + const boost::shared_ptr& get_ptr(const boost::shared_ptr& ptr) { return ptr; } + + template + const boost::intrusive_ptr& get_ptr(const boost::intrusive_ptr& ptr) { return ptr; } + + template + const LLPointer& get_ptr(const LLPointer& ptr) { return ptr; } + + // T is not any pointer type we recognize, take a pointer to the parameter + template + const T* get_ptr(const T& obj) { return &obj; } + + template + T* get_ptr(T& obj) { return &obj; } +} // namespace LL + #endif /* ! defined(LL_LLPTRTO_H) */ diff --git a/indra/llcommon/llsdutil.cpp b/indra/llcommon/llsdutil.cpp index f70bee9903..e98fc0285a 100644 --- a/indra/llcommon/llsdutil.cpp +++ b/indra/llcommon/llsdutil.cpp @@ -1046,3 +1046,38 @@ LLSD llsd_shallow(LLSD value, LLSD filter) return shallow; } + +LLSD LL::apply_llsd_fix(size_t arity, const LLSD& args) +{ + // LLSD supports a number of types, two of which are aggregates: Map and + // Array. We don't try to support Map: supporting Map would seem to + // promise that we could somehow match the string key to 'func's parameter + // names. Uh sorry, maybe in some future version of C++ with reflection. + if (args.isMap()) + { + LLTHROW(LL::apply_error("LL::apply(function, Map LLSD) unsupported")); + } + // We expect an LLSD array, but what the heck, treat isUndefined() as a + // zero-length array for calling a nullary 'func'. + if (args.isUndefined() || args.isArray()) + { + // this works because LLSD().size() == 0 + if (args.size() != arity) + { + LLTHROW(LL::apply_error(stringize("LL::apply(function(", arity, " args), ", + args.size(), "-entry LLSD array)"))); + } + return args; + } + + // args is one of the scalar types + // scalar_LLSD.size() == 0, so don't test that here. + // You can pass a scalar LLSD only to a unary 'func'. + if (arity != 1) + { + LLTHROW(LL::apply_error(stringize("LL::apply(function(", arity, " args), " + "LLSD ", LLSD::typeString(args.type()), ")"))); + } + // make an array of it + return llsd::array(args); +} diff --git a/indra/llcommon/llsdutil.h b/indra/llcommon/llsdutil.h index eddaa64bd2..546e27930d 100644 --- a/indra/llcommon/llsdutil.h +++ b/indra/llcommon/llsdutil.h @@ -29,10 +29,12 @@ #ifndef LL_LLSDUTIL_H #define LL_LLSDUTIL_H +#include "apply.h" // LL::invoke() #include "llsd.h" #include -#include +#include #include +#include // U32 LL_COMMON_API LLSD ll_sd_from_U32(const U32); @@ -589,6 +591,10 @@ namespace LL /***************************************************************************** * apply(function, LLSD array) *****************************************************************************/ +// validate incoming LLSD blob, and return an LLSD array suitable to pass to +// apply_impl() +LLSD apply_llsd_fix(size_t arity, const LLSD& args); + // Derived from https://stackoverflow.com/a/20441189 // and https://en.cppreference.com/w/cpp/utility/apply . // We can't simply make a tuple from the LLSD array and then apply() that @@ -602,6 +608,16 @@ auto apply_impl(CALLABLE&& func, const LLSD& array, std::index_sequence) return std::forward(func)(LLSDParam(array[I])...); } +// use apply_n(function, LLSD) to call a specific arity of a variadic +// function with (that many) items from the passed LLSD array +template +auto apply_n(CALLABLE&& func, const LLSD& args) +{ + return apply_impl(std::forward(func), + apply_llsd_fix(ARITY, args), + std::make_index_sequence()); +} + /** * apply(function, LLSD) goes beyond C++17 std::apply(). For this case * @a function @emph cannot be variadic: the compiler must know at compile @@ -610,32 +626,11 @@ auto apply_impl(CALLABLE&& func, const LLSD& array, std::index_sequence) template auto apply(CALLABLE&& func, const LLSD& args) { - LLSD array; - constexpr auto arity = boost::function_traits::arity; - // LLSD supports a number of types, two of which are aggregates: Map and - // Array. We don't try to support Map: supporting Map would seem to - // promise that we could somehow match the string key to 'func's parameter - // names. Uh sorry, maybe in some future version of C++ with reflection. - assert(! args.isMap()); - // We expect an LLSD array, but what the heck, treat isUndefined() as a - // zero-length array for calling a nullary 'func'. - if (args.isUndefined() || args.isArray()) - { - // this works because LLSD().size() == 0 - assert(args.size() == arity); - array = args; - } - else // args is one of the scalar types - { - // scalar_LLSD.size() == 0, so don't test that here. - // You can pass a scalar LLSD only to a unary 'func'. - assert(arity == 1); - // make an array of it - array = llsd::array(args); - } - return apply_impl(std::forward(func), - array, - std::make_index_sequence()); + // infer arity from the definition of func + constexpr auto arity = boost::function_types::function_arity< + typename std::remove_reference::type>::value; + // now that we have a compile-time arity, apply_n() works + return apply_n(std::forward(func), args); } } // namespace LL diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index b38e47a773..f09dd63316 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -33,8 +33,6 @@ #include #include #include -#include -#define foreach BOOST_FOREACH #include @@ -206,7 +204,7 @@ struct Vars void methodnb(NPARAMSb) { std::ostringstream vbin; - foreach(U8 byte, bin) + for (U8 byte: bin) { vbin << std::hex << std::setfill('0') << std::setw(2) << unsigned(byte); } @@ -462,7 +460,7 @@ namespace tut debug("dft_array_full:\n", dft_array_full); // Partial defaults arrays. - foreach(LLSD::String a, ab) + for (LLSD::String a: ab) { LLSD::Integer partition(std::min(partial_offset, dft_array_full[a].size())); dft_array_partial[a] = @@ -472,7 +470,7 @@ namespace tut debug("dft_array_partial:\n", dft_array_partial); - foreach(LLSD::String a, ab) + for(LLSD::String a: ab) { // Generate full defaults maps by zipping (params, dft_array_full). dft_map_full[a] = zipmap(params[a], dft_array_full[a]); @@ -599,19 +597,14 @@ namespace tut { // Copy descs to a temp map of same type. DescMap forgotten(descs.begin(), descs.end()); - // LLEventDispatcher intentionally provides only const_iterator: - // since dereferencing that iterator generates values on the fly, - // it's meaningless to have a modifiable iterator. But since our - // 'work' object isn't const, by default BOOST_FOREACH() wants to - // use non-const iterators. Persuade it to use the const_iterator. - foreach(LLEventDispatcher::NameDesc nd, const_cast(work)) + for (LLEventDispatcher::NameDesc nd: work) { DescMap::iterator found = forgotten.find(nd.first); - ensure(STRINGIZE("LLEventDispatcher records function '" << nd.first - << "' we didn't enter"), + ensure(stringize("LLEventDispatcher records function '", nd.first, + "' we didn't enter"), found != forgotten.end()); - ensure_equals(STRINGIZE("LLEventDispatcher desc '" << nd.second << - "' doesn't match what we entered: '" << found->second << "'"), + ensure_equals(stringize("LLEventDispatcher desc '", nd.second, + "' doesn't match what we entered: '", found->second, "'"), nd.second, found->second); // found in our map the name from LLEventDispatcher, good, erase // our map entry @@ -622,26 +615,26 @@ namespace tut std::ostringstream out; out << "LLEventDispatcher failed to report"; const char* delim = ": "; - foreach(const DescMap::value_type& fme, forgotten) + for (const DescMap::value_type& fme: forgotten) { out << delim << fme.first; delim = ", "; } - ensure(out.str(), false); + throw failure(out.str()); } } Vars* varsfor(const std::string& name) { VarsMap::const_iterator found = funcvars.find(name); - ensure(STRINGIZE("No Vars* for " << name), found != funcvars.end()); - ensure(STRINGIZE("NULL Vars* for " << name), found->second); + ensure(stringize("No Vars* for ", name), found != funcvars.end()); + ensure(stringize("NULL Vars* for ", name), found->second); return found->second; } void ensure_has(const std::string& outer, const std::string& inner) { - ensure(STRINGIZE("'" << outer << "' does not contain '" << inner << "'").c_str(), + ensure(stringize("'", outer, "' does not contain '", inner, "'").c_str(), outer.find(inner) != std::string::npos); } @@ -689,7 +682,7 @@ namespace tut LLSD getMetadata(const std::string& name) { LLSD meta(work.getMetadata(name)); - ensure(STRINGIZE("No metadata for " << name), meta.isDefined()); + ensure(stringize("No metadata for ", name), meta.isDefined()); return meta; } @@ -869,12 +862,12 @@ namespace tut LLSD req(LLSD::emptyArray()); if (arity) req[arity - 1] = LLSD(); - foreach(LLSD nm, inArray(names)) + for (LLSD nm: inArray(names)) { LLSD metadata(getMetadata(nm)); ensure_equals("name mismatch", metadata["name"], nm); ensure_equals(metadata["desc"].asString(), descs[nm]); - ensure_equals(STRINGIZE("mismatched required for " << nm.asString()), + ensure_equals(stringize("mismatched required for ", nm.asString()), metadata["required"], req); ensure("should not have optional", metadata["optional"].isUndefined()); } @@ -932,8 +925,8 @@ namespace tut ensure_equals("mdft name", mdft, mmeta["name"]); ameta.erase("name"); mmeta.erase("name"); - ensure_equals(STRINGIZE("metadata for " << adft.asString() - << " vs. " << mdft.asString()), + ensure_equals(stringize("metadata for ", adft.asString(), + " vs. ", mdft.asString()), ameta, mmeta); } } @@ -949,7 +942,7 @@ namespace tut // params are required. Also maps containing left requirements for // partial defaults arrays. Also defaults maps from defaults arrays. LLSD allreq, leftreq, rightdft; - foreach(LLSD::String a, ab) + for (LLSD::String a: ab) { // The map in which all params are required uses params[a] as // keys, with all isUndefined() as values. We can accomplish that @@ -977,9 +970,9 @@ namespace tut // Generate maps containing parameter names not provided by the // dft_map_partial maps. LLSD skipreq(allreq); - foreach(LLSD::String a, ab) + for (LLSD::String a: ab) { - foreach(const MapEntry& me, inMap(dft_map_partial[a])) + for (const MapEntry& me: inMap(dft_map_partial[a])) { skipreq[a].erase(me.first); } @@ -1024,7 +1017,7 @@ namespace tut (llsd::array("freenb_map_mdft", "smethodnb_map_mdft", "methodnb_map_mdft"), llsd::array(LLSD::emptyMap(), dft_map_full["b"])))); // required, optional - foreach(LLSD grp, inArray(groups)) + for (LLSD grp: inArray(groups)) { // Internal structure of each group in 'groups': LLSD names(grp[0]); @@ -1037,14 +1030,14 @@ namespace tut optional); // Loop through 'names' - foreach(LLSD nm, inArray(names)) + for (LLSD nm: inArray(names)) { LLSD metadata(getMetadata(nm)); ensure_equals("name mismatch", metadata["name"], nm); ensure_equals(nm.asString(), metadata["desc"].asString(), descs[nm]); - ensure_equals(STRINGIZE(nm << " required mismatch"), + ensure_equals(stringize(nm, " required mismatch"), metadata["required"], required); - ensure_equals(STRINGIZE(nm << " optional mismatch"), + ensure_equals(stringize(nm, " optional mismatch"), metadata["optional"], optional); } } @@ -1107,7 +1100,7 @@ namespace tut // LLSD value matching 'required' according to llsd_matches() rules. LLSD matching(LLSDMap("d", 3.14)("array", llsd::array("answer", true, answer))); // Okay, walk through 'tests'. - foreach(const CallablesTriple& tr, tests) + for (const CallablesTriple& tr: tests) { // Should be able to pass 'answer' to Callables registered // without 'required'. @@ -1129,14 +1122,17 @@ namespace tut set_test_name("passing wrong args to (map | array)-style registrations"); // Pass scalar/map to array-style functions, scalar/array to map-style - // functions. As that validation happens well before we engage the - // argument magic, it seems pointless to repeat this with every - // variation: (free function | non-static method), (no | arbitrary) - // args. We should only need to engage it for one map-style - // registration and one array-style registration. - std::string array_exc("needs an args array"); - call_logerr("free0_array", 17, array_exc); - call_logerr("free0_array", LLSDMap("pi", 3.14), array_exc); + // functions. It seems pointless to repeat this with every variation: + // (free function | non-static method), (no | arbitrary) args. We + // should only need to engage it for one map-style registration and + // one array-style registration. + // Now that LLEventDispatcher has been extended to treat an LLSD + // scalar as a single-entry array, the error we expect in this case is + // that apply() is trying to pass that non-empty array to a nullary + // function. + call_logerr("free0_array", 17, "LL::apply"); + // similarly, apply() doesn't accept an LLSD Map + call_logerr("free0_array", LLSDMap("pi", 3.14), "unsupported"); std::string map_exc("needs a map"); call_logerr("free0_map", 17, map_exc); @@ -1178,15 +1174,21 @@ namespace tut template<> template<> void object::test<19>() { - set_test_name("call array-style functions with too-short arrays"); - // Could have two different too-short arrays, one for *na and one for - // *nb, but since they both take 5 params... + set_test_name("call array-style functions with wrong-length arrays"); + // Could have different wrong-length arrays for *na and for *nb, but + // since they both take 5 params... LLSD tooshort(llsd::array("this", "array", "too", "short")); - foreach(const LLSD& funcsab, inArray(array_funcs)) + LLSD toolong (llsd::array("this", "array", "is", "one", "too", "long")); + LLSD badargs (llsd::array(tooshort, toolong)); + for (const LLSD& toosomething: inArray(badargs)) { - foreach(const llsd::MapEntry& e, inMap(funcsab)) + for (const LLSD& funcsab: inArray(array_funcs)) { - call_logerr(e.second, tooshort, "requires more arguments"); + for (const llsd::MapEntry& e: inMap(funcsab)) + { + // apply() complains about wrong number of array entries + call_logerr(e.second, toosomething, "LL::apply"); + } } } } @@ -1206,40 +1208,25 @@ namespace tut LLDate("2011-02-03T15:07:00Z"), LLURI("http://secondlife.com"), binary))); - LLSD argsplus(args); - argsplus["a"].append("bogus"); - argsplus["b"].append("bogus"); LLSD expect; - foreach(LLSD::String a, ab) + for (LLSD::String a: ab) { expect[a] = zipmap(params[a], args[a]); } // Adjust expect["a"]["cp"] for special Vars::cp treatment. - expect["a"]["cp"] = std::string("'") + expect["a"]["cp"].asString() + "'"; + expect["a"]["cp"] = stringize("'", expect["a"]["cp"].asString(), "'"); debug("expect: ", expect); - // Use substantially the same logic for args and argsplus - LLSD argsarrays(llsd::array(args, argsplus)); - // So i==0 selects 'args', i==1 selects argsplus - for (LLSD::Integer i(0), iend(argsarrays.size()); i < iend; ++i) + for (const LLSD& funcsab: inArray(array_funcs)) { - foreach(const LLSD& funcsab, inArray(array_funcs)) + for (LLSD::String a: ab) { - foreach(LLSD::String a, ab) - { - // Reset the Vars instance before each call - Vars* vars(varsfor(funcsab[a])); - *vars = Vars(); - work(funcsab[a], argsarrays[i][a]); - ensure_llsd(STRINGIZE(funcsab[a].asString() << - ": expect[\"" << a << "\"] mismatch"), - vars->inspect(), expect[a], 7); // 7 bits ~= 2 decimal digits - - // TODO: in the i==1 or argsplus case, intercept LL_WARNS - // output? Even without that, using argsplus verifies that - // passing too many args isn't fatal; it works -- but - // would be nice to notice the warning too. - } + // Reset the Vars instance before each call + Vars* vars(varsfor(funcsab[a])); + *vars = Vars(); + work(funcsab[a], args[a]); + ensure_llsd(stringize(funcsab[a].asString(), ": expect[\"", a, "\"] mismatch"), + vars->inspect(), expect[a], 7); // 7 bits ~= 2 decimal digits } } } @@ -1267,7 +1254,7 @@ namespace tut ("a", llsd::array(false, 255, 98.6, 1024.5, "pointer")) ("b", llsd::array("object", LLUUID::generateNewID(), LLDate::now(), LLURI("http://wiki.lindenlab.com/wiki"), LLSD::Binary(boost::begin(binary), boost::end(binary))))); LLSD array_overfull(array_full); - foreach(LLSD::String a, ab) + for (LLSD::String a: ab) { array_overfull[a].append("bogus"); } @@ -1281,7 +1268,7 @@ namespace tut ensure_not_equals("UUID collision", array_full["b"][1].asUUID(), dft_array_full["b"][1].asUUID()); LLSD map_full, map_overfull; - foreach(LLSD::String a, ab) + for (LLSD::String a: ab) { map_full[a] = zipmap(params[a], array_full[a]); map_overfull[a] = map_full[a]; @@ -1324,15 +1311,15 @@ namespace tut LLSD argssets(llsd::array(array_full, array_overfull, map_full, map_overfull)); foreach(const LLSD& args, inArray(argssets)) { - foreach(LLSD::String a, ab) + for (LLSD::String a: ab) { - foreach(LLSD::String name, inArray(names[a])) + for (LLSD::String name: inArray(names[a])) { // Reset the Vars instance Vars* vars(varsfor(name)); *vars = Vars(); work(name, args[a]); - ensure_llsd(STRINGIZE(name << ": expect[\"" << a << "\"] mismatch"), + ensure_llsd(stringize(name, ": expect[\"", a, "\"] mismatch"), vars->inspect(), expect[a], 7); // 7 bits, 2 decimal digits // intercept LL_WARNS for the two overfull cases? } -- cgit v1.3 From b2205bde52acf82575757f74a642c40b7433bf6b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 6 Jan 2023 19:58:12 -0500 Subject: DRTVWR-558: Clean up LLEventDispatcher argument and result handling. Add a new LLEventDispatcher constructor accepting not only the map key to extract a requested function name, but a second map key to extract the arguments -- when required. In Doxygen comments, clarify the difference between the two constructors. Move interaction with the LLEventPump subsystem to LLDispatchListener. LLEventDispatcher is intended to be directly called. On error, instead of looking for a "reply" key in the invocation LLSD, throw DispatchError. Publish DispatchError, formerly an implementation detail, and its new subclass DispatchMissing. Make both LLEventDispatcher::operator()() overloads return LLSD, leveraging the new internal ReturnLLSD logic that returns a degenerate LLSD blob for a void target callable and, for compatible types, converts the returned value to LLSD. Notably, the public try_call() overloads still return bool; any value returned by the target callable is discarded. Clarify the operator() and try_call() argument requirements for target callables registered to accept an LLSD array, in Doxygen comments and in code. In particular, the 'event' passed to (event) overloads (vs. the (name, event) overloads) must be an LLSD map, so it must contain an "args" key (or the new arguments map key specified to the constructor) containing the LLSD args array. Since the use of the new args key depends on whether the target callable is registered to accept an array or a map, pass it into DispatchEntry::call() (and all subclass overrides), along with a bool to disambiguate whether we reached that method from an LLEventDispatcher (event) invocation method or a (name, event) invocation method. Allow streaming an LLEventDispatcher instance to std::ostream, primarily to facilitate construction of proper error messages. Revert the 'name' argument of internal try_call(key, name, event) to std::string. Ditch try_call_log(), try_call_one() and reply(). Fold try_call_one() logic into three-argument try_call(). Refactor callFail() as a template method accepting both the exception to throw and arbitrary stringize() arguments from which to construct the exception message. Non-static callFail() implicitly prepends the instance and a colon to the rest of the arguments, and calls static sCallFail(). The latter constructs the exception message, logs it and throws the specified exception. This obviates try_call_log(). Make implementation detail helper class LLSDArgsMapper a private member of LLEventDispatcher so it can access sCallFail(): we now want all error handling to go through that method. Add LLSDArgsMapper::callFail() resembling LLSDEventDispatcher::callFail(), but without having to specify the exception: only LLEventDispatcher will throw anything but generic DispatchError. Give LLEventDispatcher::ParamsDispatchEntry and its subclasses ArrayParamsDispatchEntry and MapParamsDispatchEntry a new 'name' argument to identify error messages. Store it and use it implicitly in new callFail() method, very like LLSDArgsMapper::callFail(). Make LLEventDispatcher:: addArrayParamsDispatchEntry() and addMapParamsDispatchEntry() pass a 'name' that includes the LLEventDispatcher instance name as well as the name of the specific registered callable. This way we need not intercept a low-level error and annotate it with contextual data: we can just let the exception propagate. Make ParamsDispatchEntry::call() override catch LL::apply_error thrown by an invoker_function, and pass its message to callFail(), i.e. rethrow as LLEventDispatcher::DispatchError. Introduce ArrayParamsDispatchEntry::call() override for the special logic to extract an arguments array from a passed LLSD map -- but only under the circumstances described in the Doxygen comment. Add similar logic to MapParamsDispatchEntry::call(), but with both argskey itself and a value for argskey optional in the passed LLSD map. Because LLEventDispatcher now has two constructor overloads, allow subclass constructor LLDispatchListener() to accept zero or more trailing arguments. This is different than giving LLDispatchListener's constructor a default final argument, in that the subclass doesn't need to specify its default value: that's up to the base-class constructor. But it does require that the subclass constructor move to the header file. Move private LLEventDispatcher::reply() method to LLDispatchListener. Extend LLDispatchListener::process() to handle DispatchError by attempting to reply with a map containing an "error" key, per convention. (In other words, move that logic from LLEventDispatcher to LLDispatchListener.) Also, for a map LLSD result, attempt to reply with that result; for other defined LLSD types, attempt to reply with a map containing a "data" key. This is backwards compatible with previous behavior because all previous LLDispatchListener subclass methods returned void, which now produces an undefined LLSD blob, which we don't bother trying to send in reply. In lleventdispatcher_test.cpp, rework tut::lleventdispatcher_data::call_exc() yet again to catch DispatchError instead of listening for an LLEventPump reply event. Similarly, make call_logerr() catch DispatchError. Since the exception should also be logged, we ignore it and focus on the log, as before. Add tests <23> to <27>, exercising calls to new class DispatchResult methods returning string, int, LLSD map, LLSD array and void. (cherry picked from commit 2f9c915dd3d5137b5b2b1a57f0179e1f7a090f8c) --- indra/llcommon/lleventdispatcher.cpp | 428 +++++++++++++----------- indra/llcommon/lleventdispatcher.h | 166 +++++++-- indra/llcommon/tests/lleventdispatcher_test.cpp | 126 +++++-- 3 files changed, 463 insertions(+), 257 deletions(-) (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp index e7e73125a7..7e5723c503 100644 --- a/indra/llcommon/lleventdispatcher.cpp +++ b/indra/llcommon/lleventdispatcher.cpp @@ -43,20 +43,9 @@ #include "llexception.h" #include "llsdutil.h" #include "stringize.h" +#include // std::quoted() #include // std::auto_ptr -/***************************************************************************** -* DispatchError -*****************************************************************************/ -struct DispatchError: public LLException -{ - // template constructor involving strings passes all arguments to - // stringize() to construct LLException's what() string - template - DispatchError(const std::string& arg0, ARGS&&... args): - LLException(stringize(arg0, std::forward(args)...)) {} -}; - /***************************************************************************** * LLSDArgsMapper *****************************************************************************/ @@ -109,7 +98,7 @@ struct DispatchError: public LLException * - Holes are filled with the default values. * - Any remaining holes constitute an error. */ -class LL_COMMON_API LLSDArgsMapper +class LL_COMMON_API LLEventDispatcher::LLSDArgsMapper { public: /// Accept description of function: function name, param names, param @@ -122,6 +111,8 @@ public: private: static std::string formatlist(const LLSD&); + template + void callFail(ARGS&&... args) const; // The function-name string is purely descriptive. We want error messages // to be able to indicate which function's LLSDArgsMapper has the problem. @@ -141,15 +132,16 @@ private: FilledVector _has_dft; }; -LLSDArgsMapper::LLSDArgsMapper(const std::string& function, - const LLSD& names, const LLSD& defaults): +LLEventDispatcher::LLSDArgsMapper::LLSDArgsMapper(const std::string& function, + const LLSD& names, + const LLSD& defaults): _function(function), _names(names), _has_dft(names.size()) { if (! (_names.isUndefined() || _names.isArray())) { - LLTHROW(DispatchError(function, " names must be an array, not ", names)); + callFail(" names must be an array, not ", names); } auto nparams(_names.size()); // From _names generate _indexes. @@ -172,8 +164,7 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function, // defaults is a (possibly empty) array. Right-align it with names. if (ndefaults > nparams) { - LLTHROW(DispatchError(function, " names array ", names, - " shorter than defaults array ", defaults)); + callFail(" names array ", names, " shorter than defaults array ", defaults); } // Offset by which we slide defaults array right to right-align with @@ -210,23 +201,20 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function, } if (bogus.size()) { - LLTHROW(DispatchError(function, " defaults specified for nonexistent params ", - formatlist(bogus))); + callFail(" defaults specified for nonexistent params ", formatlist(bogus)); } } else { - LLTHROW(DispatchError(function, " defaults must be a map or an array, not ", - defaults)); + callFail(" defaults must be a map or an array, not ", defaults); } } -LLSD LLSDArgsMapper::map(const LLSD& argsmap) const +LLSD LLEventDispatcher::LLSDArgsMapper::map(const LLSD& argsmap) const { if (! (argsmap.isUndefined() || argsmap.isMap() || argsmap.isArray())) { - LLTHROW(DispatchError(_function, " map() needs a map or array, not ", - argsmap)); + callFail(" 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 @@ -323,15 +311,14 @@ LLSD LLSDArgsMapper::map(const LLSD& argsmap) const // by argsmap, that's a problem. if (unfilled.size()) { - LLTHROW(DispatchError(_function, " missing required arguments ", - formatlist(unfilled), " from ", argsmap)); + callFail(" missing required arguments ", formatlist(unfilled), " from ", argsmap); } // done return args; } -std::string LLSDArgsMapper::formatlist(const LLSD& list) +std::string LLEventDispatcher::LLSDArgsMapper::formatlist(const LLSD& list) { std::ostringstream out; const char* delim = ""; @@ -344,14 +331,26 @@ std::string LLSDArgsMapper::formatlist(const LLSD& list) return out.str(); } +template +void LLEventDispatcher::LLSDArgsMapper::callFail(ARGS&&... args) const +{ + LLEventDispatcher::sCallFail + (_function, std::forward(args)...); +} + /***************************************************************************** * LLEventDispatcher *****************************************************************************/ LLEventDispatcher::LLEventDispatcher(const std::string& desc, const std::string& key): + LLEventDispatcher(desc, key, "args") +{} + +LLEventDispatcher::LLEventDispatcher(const std::string& desc, const std::string& key, + const std::string& argskey): mDesc(desc), - mKey(key) -{ -} + mKey(key), + mArgskey(argskey) +{} LLEventDispatcher::~LLEventDispatcher() { @@ -375,20 +374,21 @@ struct LLEventDispatcher::LLSDDispatchEntry: public LLEventDispatcher::DispatchE Callable mFunc; LLSD mRequired; - virtual LLSD call(const std::string& desc, const LLSD& event) const + LLSD call(const std::string& desc, const LLSD& event, bool, const std::string&) const override { // Validate the syntax of the event itself. std::string mismatch(llsd_matches(mRequired, event)); if (! mismatch.empty()) { - LLTHROW(DispatchError(desc, ": bad request: ", mismatch)); + LLEventDispatcher::sCallFail + (desc, ": bad request: ", mismatch); } // Event syntax looks good, go for it! mFunc(event); return {}; } - virtual LLSD addMetadata(LLSD meta) const + LLSD addMetadata(LLSD meta) const override { meta["required"] = mRequired; return meta; @@ -401,16 +401,35 @@ struct LLEventDispatcher::LLSDDispatchEntry: public LLEventDispatcher::DispatchE */ struct LLEventDispatcher::ParamsDispatchEntry: public LLEventDispatcher::DispatchEntry { - ParamsDispatchEntry(const std::string& desc, const invoker_function& func): + ParamsDispatchEntry(const std::string& name, const std::string& desc, + const invoker_function& func): DispatchEntry(desc), + mName(name), mInvoker(func) {} + std::string mName; invoker_function mInvoker; - virtual LLSD call(const std::string&, const LLSD& event) const + LLSD call(const std::string&, const LLSD& event, bool, const std::string&) const override + { + try + { + return mInvoker(event); + } + catch (const LL::apply_error& err) + { + // could hit runtime errors with LL::apply() + return callFail(err.what()); + } + } + + template + LLSD callFail(ARGS&&... args) const { - return mInvoker(event); + LLEventDispatcher::sCallFail(mName, ": ", std::forward(args)...); + // pacify the compiler + return {}; } }; @@ -420,15 +439,48 @@ struct LLEventDispatcher::ParamsDispatchEntry: public LLEventDispatcher::Dispatc */ struct LLEventDispatcher::ArrayParamsDispatchEntry: public LLEventDispatcher::ParamsDispatchEntry { - ArrayParamsDispatchEntry(const std::string& desc, const invoker_function& func, - LLSD::Integer arity): - ParamsDispatchEntry(desc, func), + ArrayParamsDispatchEntry(const std::string& name, const std::string& desc, + const invoker_function& func, LLSD::Integer arity): + ParamsDispatchEntry(name, desc, func), mArity(arity) {} LLSD::Integer mArity; - virtual LLSD addMetadata(LLSD meta) const + LLSD call(const std::string& desc, const LLSD& event, bool fromMap, const std::string& argskey) const override + { +// std::string context { stringize(desc, "(", event, ") with argskey ", std::quoted(argskey), ": ") }; + // Whether we try to extract arguments from 'event' depends on whether + // the LLEventDispatcher consumer called one of the (name, event) + // methods (! fromMap) or one of the (event) methods (fromMap). If we + // were called with (name, event), the passed event must itself be + // suitable to pass to the registered callable, no args extraction + // required or even attempted. Only if called with plain (event) do we + // consider extracting args from that event. Initially assume 'event' + // itself contains the arguments. + LLSD args{ event }; + if (fromMap) + { + if (mArity) + { + // We only require/retrieve argskey if the target function + // isn't nullary. For all others, since we require an LLSD + // array, we must have an argskey. + if (argskey.empty()) + { + return callFail("LLEventDispatcher has no args key"); + } + if ((! event.has(argskey))) + { + return callFail("missing required key ", std::quoted(argskey)); + } + args = event[argskey]; + } + } + return ParamsDispatchEntry::call(desc, args, fromMap, argskey); + } + + LLSD addMetadata(LLSD meta) const override { LLSD array(LLSD::emptyArray()); // Resize to number of arguments required @@ -449,7 +501,7 @@ struct LLEventDispatcher::MapParamsDispatchEntry: public LLEventDispatcher::Para MapParamsDispatchEntry(const std::string& name, const std::string& desc, const invoker_function& func, const LLSD& params, const LLSD& defaults): - ParamsDispatchEntry(desc, func), + ParamsDispatchEntry(name, desc, func), mMapper(name, params, defaults), mRequired(LLSD::emptyMap()) { @@ -493,14 +545,25 @@ struct LLEventDispatcher::MapParamsDispatchEntry: public LLEventDispatcher::Para LLSD mRequired; LLSD mOptional; - virtual LLSD call(const std::string& desc, const LLSD& event) const + LLSD call(const std::string& desc, const LLSD& event, bool fromMap, const std::string& argskey) const override { - // Just convert from LLSD::Map to LLSD::Array using mMapper, then pass - // to base-class call() method. - return ParamsDispatchEntry::call(desc, mMapper.map(event)); + // by default, pass the whole event as the arguments map + LLSD args{ event }; + // Were we called by one of the (event) methods (instead of the (name, + // event) methods), do we have an argskey, and does the incoming event + // have that key? + if (fromMap && (! argskey.empty()) && event.has(argskey)) + { + // if so, extract the value of argskey from the incoming event, + // and use that as the arguments map + args = event[argskey]; + } + // Now convert args from LLSD map to LLSD array using mMapper, then + // pass to base-class call() method. + return ParamsDispatchEntry::call(desc, mMapper.map(args), fromMap, argskey); } - virtual LLSD addMetadata(LLSD meta) const + LLSD addMetadata(LLSD meta) const override { meta["required"] = mRequired; meta["optional"] = mOptional; @@ -513,7 +576,11 @@ void LLEventDispatcher::addArrayParamsDispatchEntry(const std::string& name, const invoker_function& invoker, LLSD::Integer arity) { - mDispatch.emplace(name, new ArrayParamsDispatchEntry(desc, invoker, arity)); + // The first parameter to ArrayParamsDispatchEntry is solely for error + // messages. Identify our instance and this entry. + mDispatch.emplace( + name, + new ArrayParamsDispatchEntry(stringize(*this, '[', name, ']'), desc, invoker, arity)); } void LLEventDispatcher::addMapParamsDispatchEntry(const std::string& name, @@ -522,7 +589,11 @@ void LLEventDispatcher::addMapParamsDispatchEntry(const std::string& name, const LLSD& params, const LLSD& defaults) { - mDispatch.emplace(name, new MapParamsDispatchEntry(name, desc, invoker, params, defaults)); + // Pass instance info as well as this entry name for error messages. + mDispatch.emplace( + name, + new MapParamsDispatchEntry(stringize(*this, '[', name, ']'), + desc, invoker, params, defaults)); } /// Register a callable by name @@ -546,174 +617,101 @@ bool LLEventDispatcher::remove(const std::string& name) /// Call a registered callable with an explicitly-specified name. It is an /// error if no such callable exists. -void LLEventDispatcher::operator()(const std::string& name, const LLSD& event) const +LLSD LLEventDispatcher::operator()(const std::string& name, const LLSD& event) const { - std::string error{ try_call_log(std::string(), name, event) }; - if (! error.empty()) - { - callFail(event, error); - } + return try_call(std::string(), name, event); } -/// Extract the @a key value from the incoming @a event, and call the callable -/// whose name is specified by that map @a key. It is an error if no such -/// callable exists. -void LLEventDispatcher::operator()(const LLSD& event) const +bool LLEventDispatcher::try_call(const std::string& name, const LLSD& event) const { - std::string error{ try_call_log(mKey, event[mKey], event) }; - if (! error.empty()) + try { - callFail(event, error); + try_call(std::string(), name, event); + return true; } -} - -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)) + // Note that we don't catch the generic DispatchError, only the specific + // DispatchMissing. try_call() only promises to return false if the + // specified callable name isn't found -- not for general errors. + catch (const DispatchMissing&) { - // Oh good, the incoming event specifies a reply pump -- pass back - // our response. - sendReply(response, event, key); + return false; } } -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 +/// Extract the @a key value from the incoming @a event, and call the callable +/// whose name is specified by that map @a key. It is an error if no such +/// callable exists. +LLSD LLEventDispatcher::operator()(const LLSD& event) const { - return try_call_log(std::string(), name, event).empty(); + return try_call(mKey, event[mKey], event); } -std::string LLEventDispatcher::try_call_log(const std::string& key, const LLSD& name, - const LLSD& event) const +bool LLEventDispatcher::try_call(const LLSD& event) const { - std::string error{ try_call(key, name, event) }; - if (! error.empty()) + try { - // 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; + try_call(mKey, event[mKey], event); + return true; + } + catch (const DispatchMissing&) + { + return false; } - return error; } -// 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 LLSD& name, - const LLSD& event) const +LLSD LLEventDispatcher::try_call(const std::string& key, const std::string& name, + const LLSD& event) const { - if (name.isUndefined()) + if (name.empty()) { if (key.empty()) { - return "attempting to call with no name"; + callFail("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); + callFail("no ", key); } - reply(result, event); - return {}; } -} -std::pair -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()) { + // Here we were passed a valid name, but there's no registered + // callable with that name. This is the one case in which we throw + // DispatchMissing instead of the generic DispatchError. + // Distinguish the public method by which our caller reached here: + // key.empty() means the name was passed explicitly, non-empty means + // we extracted the name from the incoming event using that key. if (key.empty()) { - return { stringize("'", name, "' not found"), {} }; + callFail(std::quoted(name), " not found"); } else { - return { stringize("bad ", key, " value '", name, "'"), {} }; + callFail("bad ", key, " value ", std::quoted(name)); } } - try - { - // Found the name, so it's plausible to even attempt the call. - return { {}, found->second->call(stringize("calling '", name, "'"), event) }; - } - catch (const DispatchError& err) - { - // trouble preparing arguments - return { err.what(), {} }; - } - catch (const LL::apply_error& err) - { - // could also hit runtime errors with LL::apply() - return { err.what(), {} }; - } + // Found the name, so it's plausible to even attempt the call. + return found->second->call(stringize(*this, " calling ", std::quoted(name)), + event, (! key.empty()), mArgskey); +} + +template +//static +void LLEventDispatcher::sCallFail(ARGS&&... args) +{ + auto error = stringize(std::forward(args)...); + LL_WARNS("LLEventDispatcher") << error << LL_ENDL; + LLTHROW(EXCEPTION(error)); +} + +template +void LLEventDispatcher::callFail(ARGS&&... args) const +{ + // Describe this instance in addition to the error itself. + sCallFail(*this, ": ", std::forward(args)...); } LLSD LLEventDispatcher::getMetadata(const std::string& name) const @@ -729,27 +727,69 @@ LLSD LLEventDispatcher::getMetadata(const std::string& name) const return found->second->addMetadata(meta); } +std::ostream& operator<<(std::ostream& out, const LLEventDispatcher& self) +{ + // If we're a subclass of LLEventDispatcher, e.g. LLEventAPI, report that. + return out << LLError::Log::classname(self) << '(' << self.mDesc << ')'; +} + /***************************************************************************** * LLDispatchListener *****************************************************************************/ -LLDispatchListener::LLDispatchListener(const std::string& pumpname, const std::string& key): - LLEventDispatcher(pumpname, key), - // Do NOT tweak the passed pumpname. In practice, when someone - // instantiates a subclass of our LLEventAPI subclass, they intend to - // claim that LLEventPump name in the global LLEventPumps namespace. It - // would be mysterious and distressing if we allowed name tweaking, and - // someone else claimed pumpname first for a completely unrelated - // LLEventPump. Posted events would never reach our subclass listener - // because we would have silently changed its name; meanwhile listeners - // (if any) on that other LLEventPump would be confused by the events - // intended for our subclass. - LLEventStream(pumpname, false), - mBoundListener(listen("self", [this](const LLSD& event){ return process(event); })) +/*==========================================================================*| + TODO: +* When process() finds name.isArray(), construct response array from + dispatching each call -- args must also be (array of args structures) + (could also construct response map, IF array contains unique names) +* When process() finds name.isMap(), construct response map from dispatching + each call -- value of each key is its args struct -- argskey ignored -- + note, caller can't care about order +* Possible future transactional behavior: look up all names before calling any +|*==========================================================================*/ +std::string LLDispatchListener::mReplyKey{ "reply" }; + +bool LLDispatchListener::process(const LLSD& event) const { + LLSD result; + try + { + result = (*this)(event); + } + catch (const DispatchError& err) + { + // If the incoming event contains a "reply" key, we'll respond to the + // invoker with an error message (below). But if not -- silently + // ignoring an invocation request would be confusing at best. Escalate. + if (! event.has(mReplyKey)) + { + throw; + } + + // reply with a map containing an "error" key explaining the problem + reply(llsd::map("error", err.what()), event); + return false; + } + + // We seem to have gotten a valid result. But we don't know whether the + // registered callable is void or non-void. If it's void, + // LLEventDispatcher will return isUndefined(). Otherwise, try to send it + // back to our invoker. + if (result.isDefined()) + { + if (! result.isMap()) + { + // wrap the result in a map as the "data" key + result = llsd::map("data", result); + } + reply(result, event); + } + return false; } -bool LLDispatchListener::process(const LLSD& event) +void LLDispatchListener::reply(const LLSD& reply, const LLSD& request) const { - (*this)(event); - return false; + // Call sendReply() unconditionally: sendReply() itself tests whether the + // specified reply key is present in the incoming request, and does + // nothing if there's no such key. + sendReply(reply, request, mReplyKey); } diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index cebce618df..494fc6a366 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -57,7 +57,20 @@ class LLSD; class LL_COMMON_API LLEventDispatcher { public: + /** + * Pass description and the LLSD key used by try_call(const LLSD&) and + * operator()(const LLSD&) to extract the name of the registered callable + * to invoke. + */ LLEventDispatcher(const std::string& desc, const std::string& key); + /** + * Pass description, the LLSD key used by try_call(const LLSD&) and + * operator()(const LLSD&) to extract the name of the registered callable + * to invoke, and the LLSD key used by try_call(const LLSD&) and + * operator()(const LLSD&) to extract arguments LLSD. + */ + LLEventDispatcher(const std::string& desc, const std::string& key, + const std::string& argskey); virtual ~LLEventDispatcher(); /// @name Register functions accepting(const LLSD&) @@ -146,6 +159,8 @@ public: * boost::lambda::var(instance) or boost::lambda::constant(instance_ptr) * produce suitable callables. * + * TODO: variant accepting a method of the containing class, no getter. + * * When calling this name, pass an LLSD::Array. Each entry in turn will be * converted to the corresponding parameter type using LLSDParam. */ @@ -185,6 +200,8 @@ public: * boost::lambda::var(instance) or boost::lambda::constant(instance_ptr) * produce suitable callables. * + * TODO: variant accepting a method of the containing class, no getter. + * * Pass an LLSD::Array of parameter names, and optionally another * LLSD::Array of default parameter values, a la LLSDArgsMapper. * @@ -206,28 +223,82 @@ public: /// Unregister a callable bool remove(const std::string& name); - /// Call a registered callable with an explicitly-specified name. It is an - /// error if no such callable exists. It is an error if the @a event fails - /// to match the @a required prototype specified at add() time. - void operator()(const std::string& name, const LLSD& event) const; + /// Exception if an attempted call fails for any reason + struct DispatchError: public LLException + { + DispatchError(const std::string& what): LLException(what) {} + }; + + /// Specific exception for an attempt to call a nonexistent name + struct DispatchMissing: public DispatchError + { + DispatchMissing(const std::string& what): DispatchError(what) {} + }; + + /** + * Call a registered callable with an explicitly-specified name, + * converting its return value to LLSD (undefined for a void callable). + * It is an error if no such callable exists. It is an error if the @a + * event fails to match the @a required prototype specified at add() + * time. + * + * @a event must be an LLSD array for a callable registered to accept its + * arguments from such an array. It must be an LLSD map for a callable + * registered to accept its arguments from such a map. + */ + LLSD operator()(const std::string& name, const LLSD& event) const; - /// Call a registered callable with an explicitly-specified name and - /// return true. If no such callable exists, return - /// false. It is an error if the @a event fails to match the @a - /// required prototype specified at add() time. + /** + * Call a registered callable with an explicitly-specified name and + * return true. If no such callable exists, return + * false. It is an error if the @a event fails to match the @a + * required prototype specified at add() time. + * + * @a event must be an LLSD array for a callable registered to accept its + * arguments from such an array. It must be an LLSD map for a callable + * registered to accept its arguments from such a map. + */ bool try_call(const std::string& name, const LLSD& event) const; - /// Extract the @a key value from the incoming @a event, and call the - /// callable whose name is specified by that map @a key. It is an error if - /// no such callable exists. It is an error if the @a event fails to match - /// the @a required prototype specified at add() time. - void operator()(const LLSD& event) const; - - /// Extract the @a key value from the incoming @a event, call the callable - /// whose name is specified by that map @a key and return true. - /// If no such callable exists, return false. It is an error if - /// the @a event fails to match the @a required prototype specified at - /// add() time. + /** + * Extract the @a key specified to our constructor from the incoming LLSD + * map @a event, and call the callable whose name is specified by that @a + * key's value, converting its return value to LLSD (undefined for a void + * callable). It is an error if no such callable exists. It is an error if + * the @a event fails to match the @a required prototype specified at + * add() time. + * + * For a (non-nullary) callable registered to accept its arguments from an + * LLSD array, the @a event map must contain the key @a argskey specified to + * our constructor. The value of the @a argskey key must be an LLSD array + * containing the arguments to pass to the callable named by @a key. + * + * For a callable registered to accept its arguments from an LLSD map, if + * the @a event map contains the key @a argskey specified our constructor, + * extract the value of the @a argskey key and use it as the arguments map. + * If @a event contains no @a argskey key, use the whole @a event as the + * arguments map. + */ + LLSD operator()(const LLSD& event) const; + + /** + * Extract the @a key specified to our constructor from the incoming LLSD + * map @a event, call the callable whose name is specified by that @a + * key's value and return true. If no such callable exists, + * return false. It is an error if the @a event fails to match + * the @a required prototype specified at add() time. + * + * For a (non-nullary) callable registered to accept its arguments from an + * LLSD array, the @a event map must contain the key @a argskey specified to + * our constructor. The value of the @a argskey key must be an LLSD array + * containing the arguments to pass to the callable named by @a key. + * + * For a callable registered to accept its arguments from an LLSD map, if + * the @a event map contains the key @a argskey specified our constructor, + * extract the value of the @a argskey key and use it as the arguments map. + * If @a event contains no @a argskey key, use the whole @a event as the + * arguments map. + */ bool try_call(const LLSD& event) const; /// @name Iterate over defined names @@ -242,7 +313,8 @@ private: std::string mDesc; - virtual LLSD call(const std::string& desc, const LLSD& event) const = 0; + virtual LLSD call(const std::string& desc, const LLSD& event, + bool fromMap, const std::string& argskey) const = 0; virtual LLSD addMetadata(LLSD) const = 0; }; typedef std::map > DispatchMap; @@ -269,6 +341,9 @@ public: /// Retrieve the LLSD key we use for one-arg operator() method std::string getDispatchKey() const { return mKey; } + /// description of this instance's leaf class and description + friend std::ostream& operator<<(std::ostream&, const LLEventDispatcher&); + private: template void addMethod(const std::string& name, const std::string& desc, @@ -285,19 +360,16 @@ private: } } void addFail(const std::string& name, const std::string& classname) const; - std::string try_call_log(const std::string& key, const LLSD& name, - const LLSD& event) const; - std::string try_call(const std::string& key, const LLSD& name, - const LLSD& event) const; - // returns either (empty string, LLSD) or (error message, isUndefined) - std::pair - try_call_one(const std::string& key, const std::string& name, const LLSD& event) const; - // Implement "it is an error" semantics for attempted call operations: if - // the incoming event includes a "reply" key, log and send an error reply. - void callFail(const LLSD& event, const std::string& msg) const; - void reply(const LLSD& response, const LLSD& event) const; - - std::string mDesc, mKey; + LLSD try_call(const std::string& key, const std::string& name, + const LLSD& event) const; + // raise specified EXCEPTION with specified stringize(ARGS) + template + void callFail(ARGS&&... args) const; + template + static + void sCallFail(ARGS&&... args); + + std::string mDesc, mKey, mArgskey; DispatchMap mDispatch; static NameDesc makeNameDesc(const DispatchMap::value_type& item) @@ -305,6 +377,7 @@ private: return NameDesc(item.first, item.second->mDesc); } + class LLSDArgsMapper; struct LLSDDispatchEntry; struct ParamsDispatchEntry; struct ArrayParamsDispatchEntry; @@ -459,13 +532,36 @@ class LL_COMMON_API LLDispatchListener: public LLEventStream { public: - LLDispatchListener(const std::string& pumpname, const std::string& key); + template + LLDispatchListener(const std::string& pumpname, const std::string& key, + ARGS&&... args); virtual ~LLDispatchListener() {} private: - bool process(const LLSD& event); + bool process(const LLSD& event) const; + void reply(const LLSD& reply, const LLSD& request) const; LLTempBoundListener mBoundListener; + static std::string mReplyKey; }; +template +LLDispatchListener::LLDispatchListener(const std::string& pumpname, const std::string& key, + ARGS&&... args): + // pass through any additional arguments to LLEventDispatcher ctor + LLEventDispatcher(pumpname, key, std::forward(args)...), + // Do NOT tweak the passed pumpname. In practice, when someone + // instantiates a subclass of our LLEventAPI subclass, they intend to + // claim that LLEventPump name in the global LLEventPumps namespace. It + // would be mysterious and distressing if we allowed name tweaking, and + // someone else claimed pumpname first for a completely unrelated + // LLEventPump. Posted events would never reach our subclass listener + // because we would have silently changed its name; meanwhile listeners + // (if any) on that other LLEventPump would be confused by the events + // intended for our subclass. + LLEventStream(pumpname, false), + mBoundListener(listen("self", [this](const LLSD& event){ return process(event); })) +{ +} + #endif /* ! defined(LL_LLEVENTDISPATCHER_H) */ diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index f09dd63316..00bdff89e5 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -18,6 +18,7 @@ // external library headers // other Linden headers #include "../test/lltut.h" +#include "lleventfilter.h" #include "llsd.h" #include "llsdutil.h" #include "llevents.h" @@ -640,42 +641,38 @@ namespace tut std::string call_exc(const std::string& func, const LLSD& args, const std::string& exc_frag) { - // This method was written when LLEventDispatcher responded to - // name or argument errors with LL_ERRS, hence the name: we used - // to have to intercept LL_ERRS by making it throw. Now we set up - // to catch an error response instead. But -- for that we need to - // be able to sneak a "reply" key into args, which must be a Map. - if (! (args.isUndefined() or args.isMap())) - fail(stringize("can't test call_exc() with ", args)); - LLEventStream replypump("reply"); - LLSD reply; - LLTempBoundListener bound{ - replypump.listen( - "listener", - [&reply](const LLSD& event) - { - reply = event; - return false; - }) }; - LLSD modargs{ args }; - modargs["reply"] = replypump.getName(); - if (func.empty()) + std::string what; + try { - work(modargs); + if (func.empty()) + { + work(args); + } + else + { + work(func, args); + } } - else + catch (const LLEventDispatcher::DispatchError& err) { - work(func, modargs); + what = err.what(); } - ensure("no error response", reply.has("error")); - ensure_has(reply["error"], exc_frag); - return reply["error"]; + ensure_has(what, exc_frag); + return what; } void call_logerr(const std::string& func, const LLSD& args, const std::string& frag) { CaptureLog capture; - work(func, args); + try + { + work(func, args); + } + catch (const LLEventDispatcher::DispatchError& err) + { + // the error should also have been logged; we just need to + // stop the exception propagating + } capture.messageWith(frag); } @@ -1017,6 +1014,10 @@ namespace tut (llsd::array("freenb_map_mdft", "smethodnb_map_mdft", "methodnb_map_mdft"), llsd::array(LLSD::emptyMap(), dft_map_full["b"])))); // required, optional + llsd::array // group + (llsd::array("freenb_map_mdft", "smethodnb_map_mdft", "methodnb_map_mdft"), + llsd::array(LLSD::emptyMap(), dft_map_full["b"])))); // required, optional + for (LLSD grp: inArray(groups)) { // Internal structure of each group in 'groups': @@ -1196,7 +1197,7 @@ namespace tut template<> template<> void object::test<20>() { - set_test_name("call array-style functions with (just right | too long) arrays"); + set_test_name("call array-style functions with right-size arrays"); std::vector binary; for (size_t h(0x01), i(0); i < 5; h+= 0x22, ++i) { @@ -1326,4 +1327,73 @@ namespace tut } } } + + struct DispatchResult: public LLEventDispatcher + { + using DR = DispatchResult; + + DispatchResult(): LLEventDispatcher("expect result", "op") + { + // As of 2022-12-22, LLEventDispatcher's shorthand add() methods + // for pointer-to-method of same instance only support methods + // with signature void(const LLSD&). The generic add(pointer-to- + // method) requires an instance getter. + add("strfunc", "return string", &DR::strfunc, [this](){ return this; }); + add("voidfunc", "void function", &DR::voidfunc, [this](){ return this; }); + add("intfunc", "return Integer LLSD", &DR::intfunc, [this](){ return this; }); + add("mapfunc", "return map LLSD", &DR::mapfunc, [this](){ return this; }); + add("arrayfunc", "return array LLSD", &DR::arrayfunc, [this](){ return this; }); + } + + std::string strfunc(const LLSD&) const { return "a string"; } + void voidfunc() const {} + int intfunc(const LLSD&) const { return 17; } + LLSD mapfunc(const LLSD&) const { return llsd::map("key", "value"); } + LLSD arrayfunc(const LLSD&) const { return llsd::array("a", "b", "c"); } + }; + + template<> template<> + void object::test<23>() + { + set_test_name("string result"); + DispatchResult service; + LLSD result{ service("strfunc", "ignored") }; + ensure_equals("strfunc() mismatch", result.asString(), "a string"); + } + + template<> template<> + void object::test<24>() + { + set_test_name("void result"); + DispatchResult service; + LLSD result{ service("voidfunc", LLSD()) }; + ensure("voidfunc() returned defined", result.isUndefined()); + } + + template<> template<> + void object::test<25>() + { + set_test_name("Integer result"); + DispatchResult service; + LLSD result{ service("intfunc", "ignored") }; + ensure_equals("intfunc() mismatch", result.asInteger(), 17); + } + + template<> template<> + void object::test<26>() + { + set_test_name("map LLSD result"); + DispatchResult service; + LLSD result{ service("mapfunc", "ignored") }; + ensure_equals("mapfunc() mismatch", result, llsd::map("key", "value")); + } + + template<> template<> + void object::test<27>() + { + set_test_name("array LLSD result"); + DispatchResult service; + LLSD result{ service("arrayfunc", "ignored") }; + ensure_equals("arrayfunc() mismatch", result, llsd::array("a", "b", "c")); + } } // namespace tut -- cgit v1.3 From 9553965ad661b2753d13fa9b414f529ad440000f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 10 Jan 2023 17:38:01 -0500 Subject: DRTVWR-558: Add tests for LLDispatchListener functionality. Refine the special case of calling a nullary target function from an (event) method, notably via LLDispatchListener. (cherry picked from commit edcc52a9f60b1ec9b8f53603d6e2676558d41294) --- indra/llcommon/lleventdispatcher.cpp | 8 +- indra/llcommon/tests/lleventdispatcher_test.cpp | 152 ++++++++++++++++++------ 2 files changed, 123 insertions(+), 37 deletions(-) (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp index 7e5723c503..7abdc8f57a 100644 --- a/indra/llcommon/lleventdispatcher.cpp +++ b/indra/llcommon/lleventdispatcher.cpp @@ -461,7 +461,13 @@ struct LLEventDispatcher::ArrayParamsDispatchEntry: public LLEventDispatcher::Pa LLSD args{ event }; if (fromMap) { - if (mArity) + if (! mArity) + { + // When the target function is nullary, and we're called from + // an (event) method, just ignore the rest of the map entries. + args.clear(); + } + else { // We only require/retrieve argskey if the target function // isn't nullary. For all others, since we require an LLSD diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 00bdff89e5..179fab9fad 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -23,6 +23,7 @@ #include "llsdutil.h" #include "llevents.h" #include "stringize.h" +#include "StringVec.h" #include "tests/wrapllerrs.h" #include "../test/catch_and_store_what_in.h" #include "../test/debug.h" @@ -315,6 +316,31 @@ void freenb(NPARAMSb) *****************************************************************************/ namespace tut { + void ensure_has(const std::string& outer, const std::string& inner) + { + ensure(stringize("'", outer, "' does not contain '", inner, "'"), + outer.find(inner) != std::string::npos); + } + + template + std::string call_exc(CALLABLE&& func, const std::string& exc_frag) + { + std::string what = + catch_what(std::forward(func)); + ensure_has(what, exc_frag); + return what; + } + + template + void call_logerr(CALLABLE&& func, const std::string& frag) + { + CaptureLog capture; + // the error should be logged; we just need to stop the exception + // propagating + catch_what(std::forward(func)); + capture.messageWith(frag); + } + struct lleventdispatcher_data { Debug debug{"test"}; @@ -633,47 +659,26 @@ namespace tut return found->second; } - void ensure_has(const std::string& outer, const std::string& inner) - { - ensure(stringize("'", outer, "' does not contain '", inner, "'").c_str(), - outer.find(inner) != std::string::npos); - } - std::string call_exc(const std::string& func, const LLSD& args, const std::string& exc_frag) { - std::string what; - try - { - if (func.empty()) + return tut::call_exc( + [this, func, args]() { - work(args); - } - else - { - work(func, args); - } - } - catch (const LLEventDispatcher::DispatchError& err) - { - what = err.what(); - } - ensure_has(what, exc_frag); - return what; + if (func.empty()) + { + work(args); + } + else + { + work(func, args); + } + }, + exc_frag); } void call_logerr(const std::string& func, const LLSD& args, const std::string& frag) { - CaptureLog capture; - try - { - work(func, args); - } - catch (const LLEventDispatcher::DispatchError& err) - { - // the error should also have been logged; we just need to - // stop the exception propagating - } - capture.messageWith(frag); + tut::call_logerr([this, func, args](){ work(func, args); }, frag); } LLSD getMetadata(const std::string& name) @@ -1328,11 +1333,11 @@ namespace tut } } - struct DispatchResult: public LLEventDispatcher + struct DispatchResult: public LLDispatchListener { using DR = DispatchResult; - DispatchResult(): LLEventDispatcher("expect result", "op") + DispatchResult(): LLDispatchListener("results", "op") { // As of 2022-12-22, LLEventDispatcher's shorthand add() methods // for pointer-to-method of same instance only support methods @@ -1340,6 +1345,7 @@ namespace tut // method) requires an instance getter. add("strfunc", "return string", &DR::strfunc, [this](){ return this; }); add("voidfunc", "void function", &DR::voidfunc, [this](){ return this; }); + add("emptyfunc", "return empty LLSD", &DR::emptyfunc, [this](){ return this; }); add("intfunc", "return Integer LLSD", &DR::intfunc, [this](){ return this; }); add("mapfunc", "return map LLSD", &DR::mapfunc, [this](){ return this; }); add("arrayfunc", "return array LLSD", &DR::arrayfunc, [this](){ return this; }); @@ -1347,6 +1353,7 @@ namespace tut std::string strfunc(const LLSD&) const { return "a string"; } void voidfunc() const {} + LLSD emptyfunc() const { return {}; } int intfunc(const LLSD&) const { return 17; } LLSD mapfunc(const LLSD&) const { return llsd::map("key", "value"); } LLSD arrayfunc(const LLSD&) const { return llsd::array("a", "b", "c"); } @@ -1396,4 +1403,77 @@ namespace tut LLSD result{ service("arrayfunc", "ignored") }; ensure_equals("arrayfunc() mismatch", result, llsd::array("a", "b", "c")); } + + template<> template<> + void object::test<28>() + { + set_test_name("listener error, no reply"); + DispatchResult service; + tut::call_exc( + [&service]() + { service.post(llsd::map("op", "nosuchfunc", "reqid", 17)); }, + "nosuchfunc"); + } + + template<> template<> + void object::test<29>() + { + set_test_name("listener error with reply"); + DispatchResult service; + LLCaptureListener result; + service.post(llsd::map("op", "nosuchfunc", "reqid", 17, "reply", result.getName())); + LLSD reply{ result.get() }; + ensure("no reply", reply.isDefined()); + ensure_equals("reqid not echoed", reply["reqid"].asInteger(), 17); + ensure_has(reply["error"].asString(), "nosuchfunc"); + } + + template<> template<> + void object::test<30>() + { + set_test_name("listener call to void function"); + DispatchResult service; + LLCaptureListener result; + result.set("non-empty"); + for (const auto& func: StringVec{ "voidfunc", "emptyfunc" }) + { + service.post(llsd::map( + "op", func, + "reqid", 17, + "reply", result.getName())); + ensure_equals("reply from " + func, result.get().asString(), "non-empty"); + } + } + + template<> template<> + void object::test<31>() + { + set_test_name("listener call to string function"); + DispatchResult service; + LLCaptureListener result; + service.post(llsd::map( + "op", "strfunc", + "args", llsd::array(LLSD()), + "reqid", 17, + "reply", result.getName())); + LLSD reply{ result.get() }; + ensure_equals("reqid not echoed", reply["reqid"].asInteger(), 17); + ensure_equals("bad reply from strfunc", reply["data"].asString(), "a string"); + } + + template<> template<> + void object::test<32>() + { + set_test_name("listener call to map function"); + DispatchResult service; + LLCaptureListener result; + service.post(llsd::map( + "op", "mapfunc", + "args", llsd::array(LLSD()), + "reqid", 17, + "reply", result.getName())); + LLSD reply{ result.get() }; + ensure_equals("reqid not echoed", reply["reqid"].asInteger(), 17); + ensure_equals("bad reply from mapfunc", reply["key"], "value"); + } } // namespace tut -- cgit v1.3 From 98793b8d44b32604033bd1b280f37023c86055bb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 13 Jan 2023 21:53:18 -0500 Subject: DRTVWR-558: Make DispatchResult methods use their arguments. Fix lleventdispatcher_test.cpp's test class DispatchResult::strfunc(), intfunc(), mapfunc() and arrayfunc() to return values derived from (not identical to) their arguments, so we can reuse these functions for further testing of passing arguments to a named callable. Adjust existing tests accordingly. (cherry picked from commit 07e09a8daea008d28b97399920db60a147cf75c0) --- indra/llcommon/tests/lleventdispatcher_test.cpp | 37 +++++++++++++++---------- 1 file changed, 22 insertions(+), 15 deletions(-) (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 179fab9fad..0254d23a0b 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -1351,12 +1351,18 @@ namespace tut add("arrayfunc", "return array LLSD", &DR::arrayfunc, [this](){ return this; }); } - std::string strfunc(const LLSD&) const { return "a string"; } + std::string strfunc(const std::string& str) const { return "got " + str; } void voidfunc() const {} LLSD emptyfunc() const { return {}; } - int intfunc(const LLSD&) const { return 17; } - LLSD mapfunc(const LLSD&) const { return llsd::map("key", "value"); } - LLSD arrayfunc(const LLSD&) const { return llsd::array("a", "b", "c"); } + int intfunc(int i) const { return -i; } + LLSD mapfunc(int i, const std::string& str) const + { + return llsd::map("i", intfunc(i), "str", strfunc(str)); + } + LLSD arrayfunc(int i, const std::string& str) const + { + return llsd::array(intfunc(i), strfunc(str)); + } }; template<> template<> @@ -1364,8 +1370,8 @@ namespace tut { set_test_name("string result"); DispatchResult service; - LLSD result{ service("strfunc", "ignored") }; - ensure_equals("strfunc() mismatch", result.asString(), "a string"); + LLSD result{ service("strfunc", "a string") }; + ensure_equals("strfunc() mismatch", result.asString(), "got a string"); } template<> template<> @@ -1382,7 +1388,7 @@ namespace tut { set_test_name("Integer result"); DispatchResult service; - LLSD result{ service("intfunc", "ignored") }; + LLSD result{ service("intfunc", -17) }; ensure_equals("intfunc() mismatch", result.asInteger(), 17); } @@ -1391,8 +1397,8 @@ namespace tut { set_test_name("map LLSD result"); DispatchResult service; - LLSD result{ service("mapfunc", "ignored") }; - ensure_equals("mapfunc() mismatch", result, llsd::map("key", "value")); + LLSD result{ service("mapfunc", llsd::array(-12, "value")) }; + ensure_equals("mapfunc() mismatch", result, llsd::map("i", 12, "str", "got value")); } template<> template<> @@ -1400,8 +1406,8 @@ namespace tut { set_test_name("array LLSD result"); DispatchResult service; - LLSD result{ service("arrayfunc", "ignored") }; - ensure_equals("arrayfunc() mismatch", result, llsd::array("a", "b", "c")); + LLSD result{ service("arrayfunc", llsd::array(-8, "word")) }; + ensure_equals("arrayfunc() mismatch", result, llsd::array(8, "got word")); } template<> template<> @@ -1453,12 +1459,12 @@ namespace tut LLCaptureListener result; service.post(llsd::map( "op", "strfunc", - "args", llsd::array(LLSD()), + "args", llsd::array("a string"), "reqid", 17, "reply", result.getName())); LLSD reply{ result.get() }; ensure_equals("reqid not echoed", reply["reqid"].asInteger(), 17); - ensure_equals("bad reply from strfunc", reply["data"].asString(), "a string"); + ensure_equals("bad reply from strfunc", reply["data"].asString(), "got a string"); } template<> template<> @@ -1469,11 +1475,12 @@ namespace tut LLCaptureListener result; service.post(llsd::map( "op", "mapfunc", - "args", llsd::array(LLSD()), + "args", llsd::array(-7, "value"), "reqid", 17, "reply", result.getName())); LLSD reply{ result.get() }; ensure_equals("reqid not echoed", reply["reqid"].asInteger(), 17); - ensure_equals("bad reply from mapfunc", reply["key"], "value"); + ensure_equals("bad i from mapfunc", reply["i"].asInteger(), 7); + ensure_equals("bad str from mapfunc", reply["str"], "got value"); } } // namespace tut -- cgit v1.3 From b36deb79c2e99bfa600b5895c277ffb78c61957f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 13 Jan 2023 23:07:24 -0500 Subject: DRTVWR-558: Add tests for batched LLDispatchListener operations. Specifically, add tests for: - successful map batch - map batch with some errors and a reply pump - map batch with some errors and no reply - successful array batch - array batch with some errors and a reply pump - array batch with some errors and no reply (cherry picked from commit 078f0f5c9fb5075a8ad01cac417e1d7ee2b6a919) --- indra/llcommon/tests/lleventdispatcher_test.cpp | 175 ++++++++++++++++++++++++ 1 file changed, 175 insertions(+) (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 0254d23a0b..8e84a9e038 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -1483,4 +1483,179 @@ namespace tut ensure_equals("bad i from mapfunc", reply["i"].asInteger(), 7); ensure_equals("bad str from mapfunc", reply["str"], "got value"); } + + template<> template<> + void object::test<33>() + { + set_test_name("batched map success"); + DispatchResult service; + LLCaptureListener result; + service.post(llsd::map( + "op", llsd::map( + "strfunc", "some string", + "intfunc", 2, + "voidfunc", LLSD(), + "arrayfunc", llsd::array(-5, "other string")), + "reqid", 17, + "reply", result.getName())); + LLSD reply{ result.get() }; + ensure_equals("reqid not echoed", reply["reqid"].asInteger(), 17); + reply.erase("reqid"); + ensure_equals( + "bad map batch", + reply, + llsd::map( + "strfunc", "got some string", + "intfunc", -2, + "voidfunc", LLSD(), + "arrayfunc", llsd::array(5, "got other string"))); + } + + template<> template<> + void object::test<34>() + { + set_test_name("batched map error"); + DispatchResult service; + LLCaptureListener result; + service.post(llsd::map( + "op", llsd::map( + "badfunc", 34, // ! + "strfunc", "some string", + "intfunc", 2, + "missing", LLSD(), // ! + "voidfunc", LLSD(), + "arrayfunc", llsd::array(-5, "other string")), + "reqid", 17, + "reply", result.getName())); + LLSD reply{ result.get() }; + ensure_equals("reqid not echoed", reply["reqid"].asInteger(), 17); + reply.erase("reqid"); + auto error{ reply["error"].asString() }; + reply.erase("error"); + ensure_has(error, "badfunc"); + ensure_has(error, "missing"); + ensure_equals( + "bad partial batch", + reply, + llsd::map( + "strfunc", "got some string", + "intfunc", -2, + "voidfunc", LLSD(), + "arrayfunc", llsd::array(5, "got other string"))); + } + + template<> template<> + void object::test<35>() + { + set_test_name("batched map exception"); + DispatchResult service; + auto error = tut::call_exc( + [&service]() + { + service.post(llsd::map( + "op", llsd::map( + "badfunc", 34, // ! + "strfunc", "some string", + "intfunc", 2, + "missing", LLSD(), // ! + "voidfunc", LLSD(), + "arrayfunc", llsd::array(-5, "other string")), + "reqid", 17)); + // no "reply" + }, + "badfunc"); + ensure_has(error, "missing"); + } + + template<> template<> + void object::test<36>() + { + set_test_name("batched array success"); + DispatchResult service; + LLCaptureListener result; + service.post(llsd::map( + "op", llsd::array( + llsd::array("strfunc", "some string"), + llsd::array("intfunc", 2), + "arrayfunc", + "voidfunc"), + "args", llsd::array( + LLSD(), + LLSD(), + llsd::array(-5, "other string")), + // args array deliberately short, since the default + // [3] is undefined, which should work for voidfunc + "reqid", 17, + "reply", result.getName())); + LLSD reply{ result.get() }; + ensure_equals("reqid not echoed", reply["reqid"].asInteger(), 17); + reply.erase("reqid"); + ensure_equals( + "bad array batch", + reply, + llsd::map( + "data", llsd::array( + "got some string", + -2, + llsd::array(5, "got other string"), + LLSD()))); + } + + template<> template<> + void object::test<37>() + { + set_test_name("batched array error"); + DispatchResult service; + LLCaptureListener result; + service.post(llsd::map( + "op", llsd::array( + llsd::array("strfunc", "some string"), + llsd::array("intfunc", 2, "whoops"), // bad form + "arrayfunc", + "voidfunc"), + "args", llsd::array( + LLSD(), + LLSD(), + llsd::array(-5, "other string")), + // args array deliberately short, since the default + // [3] is undefined, which should work for voidfunc + "reqid", 17, + "reply", result.getName())); + LLSD reply{ result.get() }; + ensure_equals("reqid not echoed", reply["reqid"].asInteger(), 17); + reply.erase("reqid"); + auto error{ reply["error"] }; + reply.erase("error"); + ensure_has(error, "[1]"); + ensure_has(error, "unsupported"); + ensure_equals("bad array batch", reply, + llsd::map("data", llsd::array("got some string"))); + } + + template<> template<> + void object::test<38>() + { + set_test_name("batched array exception"); + DispatchResult service; + auto error = tut::call_exc( + [&service]() + { + service.post(llsd::map( + "op", llsd::array( + llsd::array("strfunc", "some string"), + llsd::array("intfunc", 2, "whoops"), // bad form + "arrayfunc", + "voidfunc"), + "args", llsd::array( + LLSD(), + LLSD(), + llsd::array(-5, "other string")), + // args array deliberately short, since the default + // [3] is undefined, which should work for voidfunc + "reqid", 17)); + // no "reply" + }, + "[1]"); + ensure_has(error, "unsupported"); + } } // namespace tut -- cgit v1.3 From 2c894ecb25de044f5cb9c408c5264e5234b73983 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 20 Jan 2023 22:34:31 -0500 Subject: DRTVWR-558: Extend LLEventDispatcher::add() overloads. Add LL::always_return(), which takes a callable and variadic arguments. It calls the callable with those arguments and, if the returned type is convertible to T, converts it and returns it. Otherwise it returns T(). always_return() is generalized from, and supersedes, LLEventDispatcher::ReturnLLSD. Add LL::function_arity, which extends boost::function_types::function_arity by reporting results for both std::function and boost::function. Use for LL::apply(function, LLSD array) as well as for LLEventDispatcher. Make LLEventDispatcher::add() overloads uniformly distinguish between a callable (whether non-static member function or otherwise) that accepts a single LLSD parameter, versus any other signature. Accepting exactly one LLSD parameter signals that the callable will accept the composite arguments LLSD blob, instead of asking LLEventDispatcher to unpack the arguments blob into individual arguments. Support add(subclass method) overloads for arbitrary-parameters methods as well as for (const LLSD&) methods. Update tests accordingly: we need no longer pass the boilerplate lambda instance getter that binds and returns 'this'. Extract to the two LLEventDispatcher::make_invoker() overloads the LL::apply() logic formerly found in ReturnLLSD. Change lleventdispatcher_test.cpp tests from boost::bind(), which accepts variadic arguments (even though it only passes a fixed set to the target callable), to fixed-signature lambdas. This is because the revamped add() overloads care about signature. Add a test for a non-static method that accepts (const LLSD&), in other words the composite arguments LLSD blob, and likewise returns LLSD. (cherry picked from commit 95b787f7d7226ee9de79dfc9816f33c8bf199aad) --- indra/llcommon/CMakeLists.txt | 2 + indra/llcommon/always_return.h | 124 +++++++++ indra/llcommon/function_types.h | 49 ++++ indra/llcommon/lleventdispatcher.cpp | 7 +- indra/llcommon/lleventdispatcher.h | 326 ++++++++++++++++++------ indra/llcommon/llsdutil.h | 4 +- indra/llcommon/tests/lleventdispatcher_test.cpp | 62 +++-- 7 files changed, 460 insertions(+), 114 deletions(-) create mode 100644 indra/llcommon/always_return.h create mode 100644 indra/llcommon/function_types.h (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 33e8301e12..64751926d0 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -117,11 +117,13 @@ set(llcommon_SOURCE_FILES set(llcommon_HEADER_FILES CMakeLists.txt + always_return.h apply.h chrono.h classic_callback.h ctype_workaround.h fix_macros.h + function_types.h indra_constants.h lazyeventapi.h linden_common.h diff --git a/indra/llcommon/always_return.h b/indra/llcommon/always_return.h new file mode 100644 index 0000000000..6b9f1fdeaf --- /dev/null +++ b/indra/llcommon/always_return.h @@ -0,0 +1,124 @@ +/** + * @file always_return.h + * @author Nat Goodspeed + * @date 2023-01-20 + * @brief Call specified callable with arbitrary arguments, but always return + * specified type. + * + * $LicenseInfo:firstyear=2023&license=viewerlgpl$ + * Copyright (c) 2023, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_ALWAYS_RETURN_H) +#define LL_ALWAYS_RETURN_H + +#include // std::enable_if, std::is_convertible + +namespace LL +{ + +#if __cpp_lib_is_invocable >= 201703L // C++17 + template + using invoke_result = std::invoke_result; +#else // C++14 + template + using invoke_result = std::result_of; +#endif // C++14 + + /** + * AlwaysReturn()(some_function, some_args...) calls + * some_function(some_args...). It is guaranteed to return a value of type + * T, regardless of the return type of some_function(). If some_function() + * returns a type convertible to T, it will convert and return that value. + * Otherwise (notably if some_function() is void), AlwaysReturn returns + * T(). + * + * When some_function() returns a type not convertible to T, if + * you want AlwaysReturn to return some T value other than + * default-constructed T(), pass that value to AlwaysReturn's constructor. + */ + template + class AlwaysReturn + { + public: + /// pass explicit default value if other than default-constructed type + AlwaysReturn(const DESIRED& dft=DESIRED()): mDefault(dft) {} + + // callable returns a type not convertible to DESIRED, return default + template ::type, + DESIRED + >::value, + bool + >::type=true> + DESIRED operator()(CALLABLE&& callable, ARGS&&... args) + { + // discard whatever callable(args) returns + std::forward(callable)(std::forward(args)...); + return mDefault; + } + + // callable returns a type convertible to DESIRED + template ::type, + DESIRED + >::value, + bool + >::type=true> + DESIRED operator()(CALLABLE&& callable, ARGS&&... args) + { + return { std::forward(callable)(std::forward(args)...) }; + } + + private: + DESIRED mDefault; + }; + + /** + * always_return(some_function, some_args...) calls + * some_function(some_args...). It is guaranteed to return a value of type + * T, regardless of the return type of some_function(). If some_function() + * returns a type convertible to T, it will convert and return that value. + * Otherwise (notably if some_function() is void), always_return() returns + * T(). + */ + template + DESIRED always_return(CALLABLE&& callable, ARGS&&... args) + { + return AlwaysReturn()(std::forward(callable), + std::forward(args)...); + } + + /** + * make_always_return(some_function) returns a callable which, when + * called with appropriate some_function() arguments, always returns a + * value of type T, regardless of the return type of some_function(). If + * some_function() returns a type convertible to T, the returned callable + * will convert and return that value. Otherwise (notably if + * some_function() is void), the returned callable returns T(). + * + * When some_function() returns a type not convertible to T, if + * you want the returned callable to return some T value other than + * default-constructed T(), pass that value to make_always_return() as its + * optional second argument. + */ + template + auto make_always_return(CALLABLE&& callable, const DESIRED& dft=DESIRED()) + { + return + [dft, callable = std::forward(callable)] + (auto&&... args) + { + return AlwaysReturn(dft)(callable, + std::forward(args)...); + }; + } + +} // namespace LL + +#endif /* ! defined(LL_ALWAYS_RETURN_H) */ diff --git a/indra/llcommon/function_types.h b/indra/llcommon/function_types.h new file mode 100644 index 0000000000..3f42f6d640 --- /dev/null +++ b/indra/llcommon/function_types.h @@ -0,0 +1,49 @@ +/** + * @file function_types.h + * @author Nat Goodspeed + * @date 2023-01-20 + * @brief Extend boost::function_types to examine boost::function and + * std::function + * + * $LicenseInfo:firstyear=2023&license=viewerlgpl$ + * Copyright (c) 2023, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_FUNCTION_TYPES_H) +#define LL_FUNCTION_TYPES_H + +#include +#include +#include + +namespace LL +{ + + template + struct function_arity_impl + { + static constexpr auto value = boost::function_types::function_arity::value; + }; + + template + struct function_arity_impl> + { + static constexpr auto value = function_arity_impl::value; + }; + + template + struct function_arity_impl> + { + static constexpr auto value = function_arity_impl::value; + }; + + template + struct function_arity + { + static constexpr auto value = function_arity_impl::type>::value; + }; + +} // namespace LL + +#endif /* ! defined(LL_FUNCTION_TYPES_H) */ diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp index d10cf16b88..caff854753 100644 --- a/indra/llcommon/lleventdispatcher.cpp +++ b/indra/llcommon/lleventdispatcher.cpp @@ -384,8 +384,7 @@ struct LLEventDispatcher::LLSDDispatchEntry: public LLEventDispatcher::DispatchE (desc, ": bad request: ", mismatch); } // Event syntax looks good, go for it! - mFunc(event); - return {}; + return mFunc(event); } LLSD addMetadata(LLSD meta) const override @@ -603,8 +602,8 @@ void LLEventDispatcher::addMapParamsDispatchEntry(const std::string& name, } /// Register a callable by name -void LLEventDispatcher::add(const std::string& name, const std::string& desc, - const Callable& callable, const LLSD& required) +void LLEventDispatcher::addLLSD(const std::string& name, const std::string& desc, + const Callable& callable, const LLSD& required) { mDispatch.emplace(name, new LLSDDispatchEntry(desc, callable, required)); } diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index 5ab860b6dd..789a59459c 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -32,17 +32,18 @@ #if ! defined(LL_LLEVENTDISPATCHER_H) #define LL_LLEVENTDISPATCHER_H -#include #include #include -#include -#include +#include // until C++17, when we get std::is_invocable +#include #include // std::function #include // std::unique_ptr #include #include #include #include // std::pair +#include "always_return.h" +#include "function_types.h" // LL::function_arity #include "llevents.h" #include "llptrto.h" #include "llsdutil.h" @@ -78,7 +79,7 @@ public: //@{ /// Accept any C++ callable with the right signature - typedef std::function Callable; + typedef std::function Callable; /** * Register a @a callable by @a name. The passed @a callable accepts a @@ -90,19 +91,25 @@ public: void add(const std::string& name, const std::string& desc, const Callable& callable, - const LLSD& required=LLSD()); + const LLSD& required=LLSD()) + { + addLLSD(name, desc, callable, required); + } - /** - * The case of a free function (or static method) accepting(const LLSD&) - * could also be intercepted by the arbitrary-args overload below. Ensure - * that it's directed to the Callable overload above instead. - */ + template ::value + >::type> void add(const std::string& name, const std::string& desc, - void (*f)(const LLSD&), + CALLABLE&& callable, const LLSD& required=LLSD()) { - add(name, desc, Callable(f), required); + addLLSD( + name, + desc, + Callable(LL::make_always_return(std::forward(callable))), + required); } /** @@ -111,6 +118,27 @@ public: * specifying a std::bind() expression. The passed @a method * accepts a single LLSD value, presumably containing other parameters. */ + template + void add(const std::string& name, + const std::string& desc, + R (CLASS::*method)(const LLSD&), + const LLSD& required=LLSD()) + { + addMethod(name, desc, method, required); + } + + /// Overload for both const and non-const methods. The passed @a method + /// accepts a single LLSD value, presumably containing other parameters. + template + void add(const std::string& name, + const std::string& desc, + R (CLASS::*method)(const LLSD&) const, + const LLSD& required=LLSD()) + { + addMethod(name, desc, method, required); + } + + // because the compiler can't match a method returning void to the above template void add(const std::string& name, const std::string& desc, @@ -131,6 +159,128 @@ public: addMethod(name, desc, method, required); } + // non-const nullary method + template + void add(const std::string& name, + const std::string& desc, + R (CLASS::*method)()) + { + addVMethod(name, desc, method); + } + + // const nullary method + template + void add(const std::string& name, + const std::string& desc, + R (CLASS::*method)() const) + { + addVMethod(name, desc, method); + } + + // non-const nullary method returning void + template + void add(const std::string& name, + const std::string& desc, + void (CLASS::*method)()) + { + addVMethod(name, desc, method); + } + + // const nullary method returning void + template + void add(const std::string& name, + const std::string& desc, + void (CLASS::*method)() const) + { + addVMethod(name, desc, method); + } + + // non-const unary method (but method accepting LLSD should use the other add()) + // enable_if usage per https://stackoverflow.com/a/39913395/5533635 + template ::type, LLSD>::value + >::type> + void add(const std::string& name, + const std::string& desc, + R (CLASS::*method)(ARG)) + { + addVMethod(name, desc, method); + } + + // const unary method (but method accepting LLSD should use the other add()) + template ::type, LLSD>::value + >::type> + void add(const std::string& name, + const std::string& desc, + R (CLASS::*method)(ARG) const) + { + addVMethod(name, desc, method); + } + + // non-const unary method returning void + // enable_if usage per https://stackoverflow.com/a/39913395/5533635 + template ::type, LLSD>::value + >::type> + void add(const std::string& name, + const std::string& desc, + void (CLASS::*method)(ARG)) + { + addVMethod(name, desc, method); + } + + // const unary method returning void + template ::type, LLSD>::value + >::type> + void add(const std::string& name, + const std::string& desc, + void (CLASS::*method)(ARG) const) + { + addVMethod(name, desc, method); + } + + // non-const binary (or more) method + template + void add(const std::string& name, + const std::string& desc, + R (CLASS::*method)(ARG0, ARG1, ARGS...)) + { + addVMethod(name, desc, method); + } + + // const binary (or more) method + template + void add(const std::string& name, + const std::string& desc, + R (CLASS::*method)(ARG0, ARG1, ARGS...) const) + { + addVMethod(name, desc, method); + } + + // non-const binary (or more) method returning void + template + void add(const std::string& name, + const std::string& desc, + void (CLASS::*method)(ARG0, ARG1, ARGS...)) + { + addVMethod(name, desc, method); + } + + // const binary (or more) method returning void + template + void add(const std::string& name, + const std::string& desc, + void (CLASS::*method)(ARG0, ARG1, ARGS...) const) + { + addVMethod(name, desc, method); + } + //@} /// @name Register functions with arbitrary param lists @@ -143,12 +293,16 @@ public: * When calling this name, pass an LLSD::Array. Each entry in turn will be * converted to the corresponding parameter type using LLSDParam. */ - // enable_if usage per https://stackoverflow.com/a/39913395/5533635 - template::value + template () >::type> - void add(const std::string& name, const std::string& desc, Function f); + void add(const std::string& name, + const std::string& desc, + CALLABLE&& f) + { + addV(name, desc, f); + } /** * Register a nonstatic class method with arbitrary parameters. @@ -156,11 +310,7 @@ public: * To cover cases such as a method on an LLSingleton we don't yet want to * instantiate, instead of directly storing an instance pointer, accept a * nullary callable returning a pointer/reference to the desired class - * instance. If you already have an instance in hand, - * boost::lambda::var(instance) or boost::lambda::constant(instance_ptr) - * produce suitable callables. - * - * TODO: variant accepting a method of the containing class, no getter. + * instance. * * When calling this name, pass an LLSD::Array. Each entry in turn will be * converted to the corresponding parameter type using LLSDParam. @@ -186,7 +336,8 @@ public: */ template::value + boost::function_types::is_nonmember_callable_builtin::value && + ! boost::hof::is_invocable::value >::type> void add(const std::string& name, const std::string& desc, Function f, const LLSD& params, const LLSD& defaults=LLSD()); @@ -348,6 +499,11 @@ public: friend std::ostream& operator<<(std::ostream&, const LLEventDispatcher&); private: + void addLLSD(const std::string& name, + const std::string& desc, + const Callable& callable, + const LLSD& required); + template void addMethod(const std::string& name, const std::string& desc, const METHOD& method, const LLSD& required) @@ -359,9 +515,40 @@ private: } else { - add(name, desc, std::bind(method, downcast, std::placeholders::_1), required); + add(name, + desc, + Callable(LL::make_always_return( + [downcast, method] + (const LLSD& args) + { + return (downcast->*method)(args); + })), + required); } } + + template + void addVMethod(const std::string& name, const std::string& desc, + const METHOD& method) + { + CLASS* downcast = dynamic_cast(this); + if (! downcast) + { + addFail(name, typeid(CLASS).name()); + } + else + { + // add() arbitrary method plus InstanceGetter, where the + // InstanceGetter in this case returns 'this'. We don't need to + // worry about binding 'this' because, once this LLEventDispatcher + // is destroyed, the DispatchEntry goes away too. + add(name, desc, method, [downcast](){ return downcast; }); + } + } + + template + void addV(const std::string& name, const std::string& desc, Function f); + void addFail(const std::string& name, const std::string& classname) const; LLSD try_call(const std::string& key, const std::string& name, const LLSD& event) const; @@ -405,21 +592,19 @@ private: const invoker_function& invoker, const LLSD& params, const LLSD& defaults); - template - struct ReturnLLSD; }; /***************************************************************************** * LLEventDispatcher template implementation details *****************************************************************************/ -template -void LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f) +template +void LLEventDispatcher::addV(const std::string& name, const std::string& desc, Function f) { // Construct an invoker_function, a callable accepting const LLSD&. // Add to DispatchMap an ArrayParamsDispatchEntry that will handle the // caller's LLSD::Array. addArrayParamsDispatchEntry(name, desc, make_invoker(f), - boost::function_types::function_arity::value); + LL::function_arity::value); } template @@ -429,7 +614,7 @@ void LLEventDispatcher::add(const std::string& name, const std::string& desc, Me // Subtract 1 from the compile-time arity because the getter takes care of // the first parameter. We only need (arity - 1) additional arguments. addArrayParamsDispatchEntry(name, desc, make_invoker(f, getter), - boost::function_types::function_arity::value - 1); + LL::function_arity::value - 1); } template @@ -448,64 +633,23 @@ void LLEventDispatcher::add(const std::string& name, const std::string& desc, Me addMapParamsDispatchEntry(name, desc, make_invoker(f, getter), params, defaults); } -// general case, when f() has a non-void return type -template -struct LLEventDispatcher::ReturnLLSD -{ - template - LLSD operator()(Function f, const LLSD& args) - { - return { LL::apply(f, args) }; - } - - template - LLSD operator()(Method f, const InstanceGetter& getter, const LLSD& args) - { - constexpr auto arity = boost::function_types::function_arity< - typename std::remove_reference::type>::value - 1; - - // Use bind_front() to bind the method to (a pointer to) the object - // returned by getter(). It's okay to capture and bind a pointer - // because this bind_front() object will last only as long as this - // operator() call. - return { LL::apply_n(LL::bind_front(f, LL::get_ptr(getter())), args) }; - } -}; - -// specialize for void return type -template <> -struct LLEventDispatcher::ReturnLLSD -{ - template - LLSD operator()(Function f, const LLSD& args) - { - LL::apply(f, args); - return {}; - } - - template - LLSD operator()(Method f, const InstanceGetter& getter, const LLSD& args) - { - constexpr auto arity = boost::function_types::function_arity< - typename std::remove_reference::type>::value - 1; - - // Use bind_front() to bind the method to (a pointer to) the object - // returned by getter(). It's okay to capture and bind a pointer - // because this bind_front() object will last only as long as this - // operator() call. - LL::apply_n(LL::bind_front(f, LL::get_ptr(getter())), args); - return {}; - } -}; - template LLEventDispatcher::invoker_function LLEventDispatcher::make_invoker(Function f) { + // Return an invoker_function that accepts (const LLSD& args). return [f](const LLSD& args) { - return ReturnLLSD::type>() - (f, args); + // When called, call always_return, directing it to call + // f(expanded args). always_return guarantees we'll get an LLSD + // value back, even if it's undefined because 'f' doesn't return a + // type convertible to LLSD. + return LL::always_return( + [f, args] + () + { + return LL::apply(f, args); + }); }; } @@ -513,10 +657,24 @@ template LLEventDispatcher::invoker_function LLEventDispatcher::make_invoker(Method f, const InstanceGetter& getter) { + // function_arity includes its implicit 'this' pointer + constexpr auto arity = LL::function_arity< + typename std::remove_reference::type>::value - 1; + return [f, getter](const LLSD& args) { - return ReturnLLSD::type>() - (f, getter, args); + // always_return() immediately calls the lambda we pass, and + // returns LLSD whether our passed lambda returns void or non-void. + return LL::always_return( + [f, getter, args] + () + { + // Use bind_front() to bind the method to (a pointer to) the object + // returned by getter(). It's okay to capture and bind a pointer + // because this bind_front() object will last only as long as this + // lambda call. + return LL::apply_n(LL::bind_front(f, LL::get_ptr(getter())), args); + }); }; } diff --git a/indra/llcommon/llsdutil.h b/indra/llcommon/llsdutil.h index baf4400768..a6fd2fdac2 100644 --- a/indra/llcommon/llsdutil.h +++ b/indra/llcommon/llsdutil.h @@ -30,9 +30,9 @@ #define LL_LLSDUTIL_H #include "apply.h" // LL::invoke() +#include "function_types.h" // LL::function_arity #include "llsd.h" #include -#include #include #include @@ -628,7 +628,7 @@ template auto apply(CALLABLE&& func, const LLSD& args) { // infer arity from the definition of func - constexpr auto arity = boost::function_types::function_arity< + constexpr auto arity = function_arity< typename std::remove_reference::type>::value; // now that we have a compile-time arity, apply_n() works return apply_n(std::forward(func), args); diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 8e84a9e038..2a3644e2e1 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -423,9 +423,9 @@ namespace tut work.add(name, desc, &Dispatcher::cmethod1, required); // Non-subclass method with/out required params addf("method1", "method1", &v); - work.add(name, desc, boost::bind(&Vars::method1, boost::ref(v), _1)); + work.add(name, desc, [this](const LLSD& args){ return v.method1(args); }); addf("method1_req", "method1", &v); - work.add(name, desc, boost::bind(&Vars::method1, boost::ref(v), _1), required); + work.add(name, desc, [this](const LLSD& args){ return v.method1(args); }, required); /*--------------- Arbitrary params, array style ----------------*/ @@ -609,6 +609,7 @@ namespace tut void addf(const std::string& n, const std::string& d, Vars* v) { + debug("addf('", n, "', '", d, "')"); // This method is to capture in our own DescMap the name and // description of every registered function, for metadata query // testing. @@ -1339,22 +1340,25 @@ namespace tut DispatchResult(): LLDispatchListener("results", "op") { - // As of 2022-12-22, LLEventDispatcher's shorthand add() methods - // for pointer-to-method of same instance only support methods - // with signature void(const LLSD&). The generic add(pointer-to- - // method) requires an instance getter. - add("strfunc", "return string", &DR::strfunc, [this](){ return this; }); - add("voidfunc", "void function", &DR::voidfunc, [this](){ return this; }); - add("emptyfunc", "return empty LLSD", &DR::emptyfunc, [this](){ return this; }); - add("intfunc", "return Integer LLSD", &DR::intfunc, [this](){ return this; }); - add("mapfunc", "return map LLSD", &DR::mapfunc, [this](){ return this; }); - add("arrayfunc", "return array LLSD", &DR::arrayfunc, [this](){ return this; }); + add("strfunc", "return string", &DR::strfunc); + add("voidfunc", "void function", &DR::voidfunc); + add("emptyfunc", "return empty LLSD", &DR::emptyfunc); + add("intfunc", "return Integer LLSD", &DR::intfunc); + add("llsdfunc", "return passed LLSD", &DR::llsdfunc); + add("mapfunc", "return map LLSD", &DR::mapfunc); + add("arrayfunc", "return array LLSD", &DR::arrayfunc); } std::string strfunc(const std::string& str) const { return "got " + str; } void voidfunc() const {} LLSD emptyfunc() const { return {}; } int intfunc(int i) const { return -i; } + LLSD llsdfunc(const LLSD& event) const + { + LLSD result{ event }; + result["with"] = "string"; + return result; + } LLSD mapfunc(int i, const std::string& str) const { return llsd::map("i", intfunc(i), "str", strfunc(str)); @@ -1394,6 +1398,16 @@ namespace tut template<> template<> void object::test<26>() + { + set_test_name("LLSD echo"); + DispatchResult service; + LLSD result{ service("llsdfunc", llsd::map("op", "llsdfunc", "reqid", 17)) }; + ensure_equals("llsdfunc() mismatch", result, + llsd::map("op", "llsdfunc", "reqid", 17, "with", "string")); + } + + template<> template<> + void object::test<27>() { set_test_name("map LLSD result"); DispatchResult service; @@ -1402,7 +1416,7 @@ namespace tut } template<> template<> - void object::test<27>() + void object::test<28>() { set_test_name("array LLSD result"); DispatchResult service; @@ -1411,7 +1425,7 @@ namespace tut } template<> template<> - void object::test<28>() + void object::test<29>() { set_test_name("listener error, no reply"); DispatchResult service; @@ -1422,7 +1436,7 @@ namespace tut } template<> template<> - void object::test<29>() + void object::test<30>() { set_test_name("listener error with reply"); DispatchResult service; @@ -1435,7 +1449,7 @@ namespace tut } template<> template<> - void object::test<30>() + void object::test<31>() { set_test_name("listener call to void function"); DispatchResult service; @@ -1452,7 +1466,7 @@ namespace tut } template<> template<> - void object::test<31>() + void object::test<32>() { set_test_name("listener call to string function"); DispatchResult service; @@ -1468,7 +1482,7 @@ namespace tut } template<> template<> - void object::test<32>() + void object::test<33>() { set_test_name("listener call to map function"); DispatchResult service; @@ -1485,7 +1499,7 @@ namespace tut } template<> template<> - void object::test<33>() + void object::test<34>() { set_test_name("batched map success"); DispatchResult service; @@ -1512,7 +1526,7 @@ namespace tut } template<> template<> - void object::test<34>() + void object::test<35>() { set_test_name("batched map error"); DispatchResult service; @@ -1545,7 +1559,7 @@ namespace tut } template<> template<> - void object::test<35>() + void object::test<36>() { set_test_name("batched map exception"); DispatchResult service; @@ -1568,7 +1582,7 @@ namespace tut } template<> template<> - void object::test<36>() + void object::test<37>() { set_test_name("batched array success"); DispatchResult service; @@ -1602,7 +1616,7 @@ namespace tut } template<> template<> - void object::test<37>() + void object::test<38>() { set_test_name("batched array error"); DispatchResult service; @@ -1633,7 +1647,7 @@ namespace tut } template<> template<> - void object::test<38>() + void object::test<39>() { set_test_name("batched array exception"); DispatchResult service; -- cgit v1.3 From 7d1a3d70373f2988510eccf7dd8f9bd2f6c2694a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 13 Jul 2023 15:58:37 -0400 Subject: DRTVWR-558: Fix a few lleventdispatcher_test merge glitches. --- indra/llcommon/tests/lleventdispatcher_test.cpp | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 2a3644e2e1..0f27211d9e 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -754,7 +754,7 @@ namespace tut set_test_name("map-style registration with non-array params"); // Pass "param names" as scalar or as map LLSD attempts(llsd::array(17, LLSDMap("pi", 3.14)("two", 2))); - foreach(LLSD ae, inArray(attempts)) + for (LLSD ae: inArray(attempts)) { std::string threw = catch_what([this, &ae](){ work.add("freena_err", "freena", freena, ae); @@ -829,7 +829,7 @@ namespace tut { set_test_name("query Callables with/out required params"); LLSD names(llsd::array("free1", "Dmethod1", "Dcmethod1", "method1")); - foreach(LLSD nm, inArray(names)) + for (LLSD nm: inArray(names)) { LLSD metadata(getMetadata(nm)); ensure_equals("name mismatch", metadata["name"], nm); @@ -858,7 +858,7 @@ namespace tut (5, llsd::array("freena_array", "smethodna_array", "methodna_array")), llsd::array (5, llsd::array("freenb_array", "smethodnb_array", "methodnb_array")))); - foreach(LLSD ae, inArray(expected)) + for (LLSD ae: inArray(expected)) { LLSD::Integer arity(ae[0].asInteger()); LLSD names(ae[1]); @@ -884,7 +884,7 @@ namespace tut // - (Free function | non-static method), map style, no params (ergo // no defaults) LLSD names(llsd::array("free0_map", "smethod0_map", "method0_map")); - foreach(LLSD nm, inArray(names)) + for (LLSD nm: inArray(names)) { LLSD metadata(getMetadata(nm)); ensure_equals("name mismatch", metadata["name"], nm); @@ -914,7 +914,7 @@ namespace tut llsd::array("smethodnb_map_adft", "smethodnb_map_mdft"), llsd::array("methodna_map_adft", "methodna_map_mdft"), llsd::array("methodnb_map_adft", "methodnb_map_mdft"))); - foreach(LLSD eq, inArray(equivalences)) + for (LLSD eq: inArray(equivalences)) { LLSD adft(eq[0]); LLSD mdft(eq[1]); @@ -1020,10 +1020,6 @@ namespace tut (llsd::array("freenb_map_mdft", "smethodnb_map_mdft", "methodnb_map_mdft"), llsd::array(LLSD::emptyMap(), dft_map_full["b"])))); // required, optional - llsd::array // group - (llsd::array("freenb_map_mdft", "smethodnb_map_mdft", "methodnb_map_mdft"), - llsd::array(LLSD::emptyMap(), dft_map_full["b"])))); // required, optional - for (LLSD grp: inArray(groups)) { // Internal structure of each group in 'groups': @@ -1156,7 +1152,7 @@ namespace tut ("free0_array", "free0_map", "smethod0_array", "smethod0_map", "method0_array", "method0_map")); - foreach(LLSD name, inArray(names)) + for (LLSD name: inArray(names)) { // Look up the Vars instance for this function. Vars* vars(varsfor(name)); @@ -1316,7 +1312,7 @@ namespace tut "freenb_map_mdft", "smethodnb_map_mdft", "methodnb_map_mdft"))); // Treat (full | overfull) (array | map) the same. LLSD argssets(llsd::array(array_full, array_overfull, map_full, map_overfull)); - foreach(const LLSD& args, inArray(argssets)) + for (const LLSD& args: inArray(argssets)) { for (LLSD::String a: ab) { -- cgit v1.3 From 6a77d333d3eb876ccd64324c09cf63c0989164ca Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 27 Jul 2023 21:09:50 -0400 Subject: DRTVWR-587: Skip some tests that only fail with older Visual Studio --- indra/llcommon/tests/lleventdispatcher_test.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 0f27211d9e..58469313e9 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -1200,6 +1200,9 @@ namespace tut void object::test<20>() { set_test_name("call array-style functions with right-size arrays"); +#if defined(_MSC_VER) && _MSC_VER <= 1933 + skip("This test fails on VS older than VS2022 ver 17.4"); +#endif std::vector binary; for (size_t h(0x01), i(0); i < 5; h+= 0x22, ++i) { @@ -1238,6 +1241,9 @@ namespace tut void object::test<21>() { set_test_name("verify that passing LLSD() to const char* sends NULL"); +#if defined(_MSC_VER) && _MSC_VER <= 1933 + skip("This test fails on VS older than VS2022 ver 17.4"); +#endif ensure_equals("Vars::cp init", v.cp, ""); work("methodna_map_mdft", LLSDMap("cp", LLSD())); @@ -1251,6 +1257,9 @@ namespace tut template<> template<> void object::test<22>() { +#if defined(_MSC_VER) && _MSC_VER <= 1933 + skip("This test fails on VS older than VS2022 ver 17.4"); +#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 -- cgit v1.3 From 2667653d41d3b4799bf319783a884cbac7f826da Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 27 Oct 2023 15:40:20 -0400 Subject: DRTVWR-587: Skip Visual Studio LLSDParam tests for now. They do work fine on clang... unblocking the rest of the team during diagnosis. --- indra/llcommon/tests/lleventdispatcher_test.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 58469313e9..40643172ee 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -1200,8 +1200,8 @@ namespace tut void object::test<20>() { set_test_name("call array-style functions with right-size arrays"); -#if defined(_MSC_VER) && _MSC_VER <= 1933 - skip("This test fails on VS older than VS2022 ver 17.4"); +#if defined(_MSC_VER) + skip("This test fails on VS"); #endif std::vector binary; for (size_t h(0x01), i(0); i < 5; h+= 0x22, ++i) @@ -1241,8 +1241,8 @@ namespace tut void object::test<21>() { set_test_name("verify that passing LLSD() to const char* sends NULL"); -#if defined(_MSC_VER) && _MSC_VER <= 1933 - skip("This test fails on VS older than VS2022 ver 17.4"); +#if defined(_MSC_VER) + skip("This test fails on VS"); #endif ensure_equals("Vars::cp init", v.cp, ""); @@ -1257,8 +1257,8 @@ namespace tut template<> template<> void object::test<22>() { -#if defined(_MSC_VER) && _MSC_VER <= 1933 - skip("This test fails on VS older than VS2022 ver 17.4"); +#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"; -- cgit v1.3 From f7d2d40b3057f5bc249c88784b35443aad8de7aa Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sun, 29 Oct 2023 11:56:17 -0400 Subject: DRTVWR-587: Fix LL::apply(function, LLSD array). We define a specialization of LLSDParam 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(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, used when we don't have the parameter type available to select the LLSDParam specialization. LLSDParam defines a templated conversion operator T() that constructs an LLSDParam to provide the actual parameter value. So far, so good. The trouble was with the implementation of LLSDParam: it constructed a _temporary_ LLSDParam, implicitly called its operator T() and immediately destroyed it. Destroying LLSDParam destroyed its stored string, thus invalidating the c_str() pointer before the target function was entered. Instead, make LLSDParam::operator T() capture each LLSDParam it constructs, extending its lifespan to the lifespan of the LLSDParam 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. --- indra/llcommon/CMakeLists.txt | 8 +-- indra/llcommon/llsdutil.h | 75 ++++++++++++++++++------- indra/llcommon/tests/lleventdispatcher_test.cpp | 15 ++--- 3 files changed, 60 insertions(+), 38 deletions(-) (limited to 'indra/llcommon/tests/lleventdispatcher_test.cpp') 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 #include +#include // std::shared_ptr #include +#include // 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 -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 +class LLSDParam: public LLSDParamBase { private: LLSD value_; + // LLSDParam::operator T() works by instantiating an LLSDParam on + // demand. Returning that engages LLSDParam::operator T(), producing + // the desired result. But LLSDParam owns a std::string whose + // c_str() is returned by its operator const char*(). If we return a temp + // LLSDParam, 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 is presumably guaranteed to survive until the + // subject function has returned, so we must ensure that any constructed + // LLSDParam lives just as long as this LLSDParam does. Putting + // each LLSDParam 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 + // will be asked for at most ONE conversion. We could store a scalar + // std::unique_ptr and, when constructing an new LLSDParam, 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> converters_; public: LLSDParam(const LLSD& value): value_(value) {} @@ -358,7 +385,15 @@ public: /// otherwise, instantiate a more specific LLSDParam to convert; that /// preserves the existing customization mechanism template - operator T() const { return LLSDParam>(value_); } + operator T() const + { + // capture 'ptr' with the specific subclass type because converters_ + // only stores LLSDParamBase pointers + auto ptr{ std::make_shared>>(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 \ +class LLSDParam: 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(yourLLSD). */ template <> -class LLSDParam +class LLSDParam: 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(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 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 -- cgit v1.3