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') 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 af4fbc1f8a99a3c5370cb6db45435e67f9ce92d2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 18 Jun 2022 11:57:10 -0400 Subject: DRTVWR-564: WIP: Add LazyEventAPI and tests. Tests don't yet pass. LazyEventAPI is a registrar that implicitly instantiates some particular LLEventAPI subclass on demand: that is, when LLEventPumps::obtain() tries to find an LLEventPump by the registered name. This leverages the new LLEventPumps::registerPumpFactory() machinery. Fix registerPumpFactory() to adapt the passed PumpFactory to accept TypeFactory parameters (two of which it ignores). Supplement it with unregisterPumpFactory() to support LazyEventAPI instances with lifespans shorter than the process -- which may be mostly test programs, but still a hole worth closing. Similarly, add unregisterTypeFactory(). A LazyEventAPI subclass takes over responsibility for specifying the LLEventAPI's name, desc, field, plus whatever add() calls will be needed to register the LLEventAPI's operations. This is so we can (later) enhance LLLeapListener to consult LazyEventAPI instances for not-yet-instantiated LLEventAPI metadata, as well as enumerating existing LLEventAPI instances. The trickiest part of this is capturing calls to the various LLEventDispatcher::add() overloads in such a way that, when the LLEventAPI subclass is eventually instantiated, we can replay them in the new instance. LLEventAPI acquires a new protected constructor specifically for use by a subclass registered by a companion LazyEventAPI. It accepts a const reference to LazyEventAPIParams, intended to be opaque to the LLEventAPI subclass; the subclass must declare a constructor that accepts and forwards the parameter block to the new LLEventAPI constructor. The implementation delegates to the existing LLEventAPI constructor, plus it runs deferred add() calls. LLDispatchListener now derives from LLEventStream instead of containing it as a data member. The reason is that if LLEventPumps::obtain() implicitly instantiates it, LLEventPumps's destructor will try to destroy it by deleting the LLEventPump*. If the LLEventPump returned by the factory function is a data member of an outer class, that won't work so well. But if LLDispatchListener (and by implication, LLEventAPI and any subclass) is derived from LLEventPump, then the virtual destructor will Do The Right Thing. Change LLDispatchListener to *not* allow tweaking the LLEventPump name. Since the overwhelming use case for LLDispatchListener is LLEventAPI, accepting but silently renaming an LLEventAPI subclass would ensure nobody could reach it. Change LLEventDispatcher's use of std::enable_if to control the set of add() overloads available for the intended use cases. Apparently this formulation is just as functional at the method declaration point, while avoiding the need to restate the whole enable_if expression at the method definition point. Add lazyeventapi_test.cpp to exercise. --- indra/llcommon/CMakeLists.txt | 4 + indra/llcommon/lazyeventapi.cpp | 53 ++++++++ indra/llcommon/lazyeventapi.h | 204 +++++++++++++++++++++++++++++ indra/llcommon/lleventapi.cpp | 8 ++ indra/llcommon/lleventapi.h | 23 +++- indra/llcommon/lleventdispatcher.cpp | 13 +- indra/llcommon/lleventdispatcher.h | 105 +++++++-------- indra/llcommon/llevents.cpp | 29 +++- indra/llcommon/llevents.h | 6 +- indra/llcommon/tests/lazyeventapi_test.cpp | 89 +++++++++++++ 10 files changed, 466 insertions(+), 68 deletions(-) create mode 100644 indra/llcommon/lazyeventapi.cpp create mode 100644 indra/llcommon/lazyeventapi.h create mode 100644 indra/llcommon/tests/lazyeventapi_test.cpp (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index ca8b5e946f..36b2e09dc5 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -30,6 +30,7 @@ include_directories( set(llcommon_SOURCE_FILES indra_constants.cpp + lazyeventapi.cpp llallocator.cpp llallocator_heap_profile.cpp llapp.cpp @@ -128,10 +129,12 @@ set(llcommon_SOURCE_FILES set(llcommon_HEADER_FILES CMakeLists.txt + apply.h chrono.h ctype_workaround.h fix_macros.h indra_constants.h + lazyeventapi.h linden_common.h llalignedarray.h llallocator.h @@ -338,6 +341,7 @@ if (LL_TESTS) ${BOOST_SYSTEM_LIBRARY}) LL_ADD_INTEGRATION_TEST(commonmisc "" "${test_libs}") LL_ADD_INTEGRATION_TEST(bitpack "" "${test_libs}") + 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/lazyeventapi.cpp b/indra/llcommon/lazyeventapi.cpp new file mode 100644 index 0000000000..aefc2db6da --- /dev/null +++ b/indra/llcommon/lazyeventapi.cpp @@ -0,0 +1,53 @@ +/** + * @file lazyeventapi.cpp + * @author Nat Goodspeed + * @date 2022-06-17 + * @brief Implementation for lazyeventapi. + * + * $LicenseInfo:firstyear=2022&license=viewerlgpl$ + * Copyright (c) 2022, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "lazyeventapi.h" +// STL headers +// std headers +// external library headers +// other Linden headers +#include "llevents.h" + +LL::LazyEventAPIBase::LazyEventAPIBase( + const std::string& name, const std::string& desc, const std::string& field) +{ + // populate embedded LazyEventAPIParams instance + mParams.name = name; + mParams.desc = desc; + mParams.field = field; + // mParams.init and mOperations are populated by subsequent add() calls. + + // Our raison d'etre: register as an LLEventPumps::PumpFactory + // so obtain() will notice any request for this name and call us. + // Of course, our subclass constructor must finish running (making add() + // calls) before mParams will be fully populated, but we expect that to + // happen well before the first LLEventPumps::obtain(name) call. + mRegistered = LLEventPumps::instance().registerPumpFactory( + name, + [this](const std::string& name){ return construct(name); }); +} + +LL::LazyEventAPIBase::~LazyEventAPIBase() +{ + // If our constructor's registerPumpFactory() call was unsuccessful, that + // probably means somebody else claimed the name first. If that's the + // case, do NOT unregister their name out from under them! + // If this is a static instance being destroyed at process shutdown, + // LLEventPumps will probably have been cleaned up already. + if (mRegistered && ! LLEventPumps::wasDeleted()) + { + // unregister the callback to this doomed instance + LLEventPumps::instance().unregisterPumpFactory(mParams.name); + } +} diff --git a/indra/llcommon/lazyeventapi.h b/indra/llcommon/lazyeventapi.h new file mode 100644 index 0000000000..2e947899dc --- /dev/null +++ b/indra/llcommon/lazyeventapi.h @@ -0,0 +1,204 @@ +/** + * @file lazyeventapi.h + * @author Nat Goodspeed + * @date 2022-06-16 + * @brief Declaring a static module-scope LazyEventAPI registers a specific + * LLEventAPI for future on-demand instantiation. + * + * $LicenseInfo:firstyear=2022&license=viewerlgpl$ + * Copyright (c) 2022, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LAZYEVENTAPI_H) +#define LL_LAZYEVENTAPI_H + +#include "apply.h" +#include "lleventapi.h" +#include "llinstancetracker.h" +#include +#include +#include +#include // std::pair +#include + +namespace LL +{ + /** + * Bundle params we want to pass to LLEventAPI's protected constructor. We + * package them this way so a subclass constructor can simply forward an + * opaque reference to the LLEventAPI constructor. + */ + // This is a class instead of a plain struct mostly so when we forward- + // declare it we don't have to remember the distinction. + class LazyEventAPIParams + { + public: + // package the parameters used by the normal LLEventAPI constructor + std::string name, desc, field; + // bundle LLEventAPI::add() calls collected by LazyEventAPI::add(), so + // the special LLEventAPI constructor we engage can "play back" those + // add() calls + boost::signals2::signal init; + }; + + // The tricky part is: can we capture a sequence of add() calls in the + // LazyEventAPI subclass constructor and then, in effect, replay those + // add() calls on instantiation of the registered LLEventAPI subclass? so + // we don't have to duplicate the add() calls in both constructors? + + // Derive a subclass from LazyEventAPI. Its constructor must pass + // LazyEventAPI's constructor the name, desc, field params. Moreover the + // constructor body must call add(name, desc, *args) for any of the + // LLEventDispatcher add() methods, referencing the LLEventAPI subclass + // methods. + + // LazyEventAPI will store the name, desc, field params for the overall + // LLEventAPI. It will support a single generic add() call accepting name, + // desc, parameter pack. + + // It will hold a std::vector> for each operation. + // It will make all these strings available to LLLeapListener. + + // Maybe what we want is to store a vector of callables (a + // boost::signals2!) and populate it with lambdas, each of which accepts + // LLEventAPI* and calls the relevant add() method by forwarding exactly + // the name, desc and parameter pack. Then, on constructing the target + // LLEventAPI, we just fire the signal, passing the new instance pointer. + + /** + * LazyEventAPIBase implements most of the functionality of LazyEventAPI + * (q.v.), but we need the LazyEventAPI template subclass so we can accept + * the specific LLEventAPI subclass type. + */ + // No LLInstanceTracker key: we don't need to find a specific instance, + // LLLeapListener just needs to be able to enumerate all instances. + class LazyEventAPIBase: public LLInstanceTracker + { + public: + LazyEventAPIBase(const std::string& name, const std::string& desc, + const std::string& field); + virtual ~LazyEventAPIBase(); + + // Do not copy or move: once constructed, LazyEventAPIBase must stay + // put: we bind its instance pointer into a callback. + LazyEventAPIBase(const LazyEventAPIBase&) = delete; + LazyEventAPIBase(LazyEventAPIBase&&) = delete; + LazyEventAPIBase& operator=(const LazyEventAPIBase&) = delete; + LazyEventAPIBase& operator=(LazyEventAPIBase&&) = delete; + + // actually instantiate the companion LLEventAPI subclass + virtual LLEventPump* construct(const std::string& name) = 0; + + // capture add() calls we want to play back on LLEventAPI construction + template + void add(const std::string& name, const std::string& desc, ARGS&&... rest) + { + // capture the metadata separately + mOperations.push_back(std::make_pair(name, desc)); + // Use connect_extended() so the lambda is passed its own + // connection. + // We can't bind an unexpanded parameter pack into a lambda -- + // shame really. Instead, capture it as a std::tuple and then, in + // the lambda, use apply() to convert back to function args. + mParams.init.connect_extended( + [name, desc, rest = std::make_tuple(std::forward(rest)...)] + (const boost::signals2::connection& conn, LLEventAPI* instance) + { + // we only need this connection once + conn.disconnect(); + // Our add() method distinguishes name and desc because we + // capture them separately. But now, because apply() + // expects a tuple specifying ALL the arguments, expand to + // a tuple including add_trampoline() arguments: instance, + // name, desc, rest. + // apply() can't accept a template per se; it needs a + // particular specialization. + apply(&LazyEventAPIBase::add_trampoline, + std::tuple_cat(std::make_tuple(instance, name, desc), + rest)); + }); + } + + // metadata that might be queried by LLLeapListener + std::vector> mOperations; + // Params with which to instantiate the companion LLEventAPI subclass + LazyEventAPIParams mParams; + + private: + // Passing an overloaded function to any function that accepts an + // arbitrary callable is a PITB because you have to specify the + // correct overload. What we want is for the compiler to select the + // correct overload, based on the carefully-wrought enable_ifs in + // LLEventDispatcher. This (one and only) add_trampoline() method + // exists solely to pass to LL::apply(). Once add_trampoline() is + // called with the expanded arguments, we hope the compiler will Do + // The Right Thing in selecting the correct LLEventAPI::add() + // overload. + template + static + void add_trampoline(LLEventAPI* instance, ARGS&&... args) + { + instance->add(std::forward(args)...); + } + + bool mRegistered; + }; + + /** + * LazyEventAPI provides a way to register a particular LLEventAPI to be + * instantiated on demand, that is, when its name is passed to + * LLEventPumps::obtain(). + * + * Derive your listener from LLEventAPI as usual, with its various + * operation methods, but code your constructor to accept + * (const LL::LazyEventAPIParams& params) + * and forward that reference to (the protected) + * LLEventAPI(const LL::LazyEventAPIParams&) constructor. + * + * Then derive your listener registrar from + * LazyEventAPI. The constructor should + * look very like a traditional LLEventAPI constructor: + * + * * pass (name, desc [, field]) to LazyEventAPI's constructor + * * in the body, make a series of add() calls referencing your LLEventAPI + * subclass methods. + * + * You may use any LLEventAPI::add() methods, that is, any + * LLEventDispatcher::add() methods. But the target methods you pass to + * add() must belong to your LLEventAPI subclass, not the LazyEventAPI + * subclass. + * + * Declare a static instance of your LazyEventAPI listener registrar + * class. When it's constructed at static initialization time, it will + * register your LLEventAPI subclass with LLEventPumps. It will also + * collect metadata for the LLEventAPI and its operations to provide to + * LLLeapListener's introspection queries. + * + * When someone later calls LLEventPumps::obtain() to post an event to + * your LLEventAPI subclass, obtain() will instantiate it using + * LazyEventAPI's name, desc, field and add() calls. + */ + template + class LazyEventAPI: public LazyEventAPIBase + { + public: + // for subclass constructor to reference handler methods + using listener = EVENTAPI; + + LazyEventAPI(const std::string& name, const std::string& desc, + const std::string& field="op"): + // Forward ctor params to LazyEventAPIBase + LazyEventAPIBase(name, desc, field) + {} + + LLEventPump* construct(const std::string& /*name*/) override + { + // base class has carefully assembled LazyEventAPIParams embedded + // in this instance, just pass to LLEventAPI subclass constructor + return new EVENTAPI(mParams); + } + }; +} // namespace LL + +#endif /* ! defined(LL_LAZYEVENTAPI_H) */ diff --git a/indra/llcommon/lleventapi.cpp b/indra/llcommon/lleventapi.cpp index ff5459c1eb..3d46ef1034 100644 --- a/indra/llcommon/lleventapi.cpp +++ b/indra/llcommon/lleventapi.cpp @@ -35,6 +35,7 @@ // external library headers // other Linden headers #include "llerror.h" +#include "lazyeventapi.h" LLEventAPI::LLEventAPI(const std::string& name, const std::string& desc, const std::string& field): lbase(name, field), @@ -43,6 +44,13 @@ LLEventAPI::LLEventAPI(const std::string& name, const std::string& desc, const s { } +LLEventAPI::LLEventAPI(const LL::LazyEventAPIParams& params): + LLEventAPI(params.name, params.desc, params.field) +{ + // call initialization functions with our brand-new instance pointer + params.init(this); +} + LLEventAPI::~LLEventAPI() { } diff --git a/indra/llcommon/lleventapi.h b/indra/llcommon/lleventapi.h index ed62fa064a..a019458553 100644 --- a/indra/llcommon/lleventapi.h +++ b/indra/llcommon/lleventapi.h @@ -35,6 +35,13 @@ #include "llinstancetracker.h" #include +namespace LL +{ + template + class LazyEventAPI; + class LazyEventAPIParams; +} + /** * LLEventAPI not only provides operation dispatch functionality, inherited * from LLDispatchListener -- it also gives us event API introspection. @@ -45,6 +52,8 @@ class LL_COMMON_API LLEventAPI: public LLDispatchListener, { typedef LLDispatchListener lbase; typedef LLInstanceTracker ibase; + template + friend class LL::LazyEventAPI; public: @@ -137,16 +146,20 @@ public: * @endcode */ LLSD& operator[](const LLSD::String& key) { return mResp[key]; } - - /** - * set the response to the given data - */ - void setResponse(LLSD const & response){ mResp = response; } + + /** + * set the response to the given data + */ + void setResponse(LLSD const & response){ mResp = response; } LLSD mResp, mReq; LLSD::String mKey; }; +protected: + // constructor used only by subclasses registered by LazyEventAPI + LLEventAPI(const LL::LazyEventAPIParams&); + private: std::string mDesc; }; diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp index 742d6cf51f..bc53ec3da0 100644 --- a/indra/llcommon/lleventdispatcher.cpp +++ b/indra/llcommon/lleventdispatcher.cpp @@ -706,8 +706,17 @@ LLSD LLEventDispatcher::getMetadata(const std::string& name) const LLDispatchListener::LLDispatchListener(const std::string& pumpname, const std::string& key): LLEventDispatcher(pumpname, key), - mPump(pumpname, true), // allow tweaking for uniqueness - mBoundListener(mPump.listen("self", boost::bind(&LLDispatchListener::process, this, _1))) + // 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); })) { } diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index 1b3e834aeb..ce9d3775cc 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -165,12 +165,12 @@ public: * When calling this name, pass an LLSD::Array. Each entry in turn will be * converted to the corresponding parameter type using LLSDParam. */ - template - typename std::enable_if< - boost::function_types::is_nonmember_callable_builtin::value - >::type add(const std::string& name, - const std::string& desc, - Function f); + // enable_if usage per https://stackoverflow.com/a/39913395/5533635 + template::value + >::type> + void add(const std::string& name, const std::string& desc, Function f); /** * Register a nonstatic class method with arbitrary parameters. @@ -189,14 +189,13 @@ public: * When calling this name, pass an LLSD::Array. Each entry in turn will be * converted to the corresponding parameter type using LLSDParam. */ - template - typename std::enable_if< - boost::function_types::is_member_function_pointer::value && - ! std::is_convertible::value - >::type add(const std::string& name, - const std::string& desc, - Method f, - const InstanceGetter& getter); + template::value && + ! std::is_convertible::value + >::type> + void add(const std::string& name, const std::string& desc, Method f, + const InstanceGetter& getter); /** * Register a free function with arbitrary parameters. (This also works @@ -213,14 +212,12 @@ public: * an LLSD::Array using LLSDArgsMapper and then convert each entry in turn * to the corresponding parameter type using LLSDParam. */ - template - typename std::enable_if< - boost::function_types::is_nonmember_callable_builtin::value - >::type add(const std::string& name, - const std::string& desc, - Function f, - const LLSD& params, - const LLSD& defaults=LLSD()); + template::value + >::type> + void add(const std::string& name, const std::string& desc, Function f, + const LLSD& params, const LLSD& defaults=LLSD()); /** * Register a nonstatic class method with arbitrary parameters. @@ -243,16 +240,14 @@ public: * an LLSD::Array using LLSDArgsMapper and then convert each entry in turn * to the corresponding parameter type using LLSDParam. */ - template - typename std::enable_if< - boost::function_types::is_member_function_pointer::value && - ! std::is_convertible::value - >::type add(const std::string& name, - const std::string& desc, - Method f, - const InstanceGetter& getter, - const LLSD& params, - const LLSD& defaults=LLSD()); + template::value && + ! std::is_convertible::value + >::type> + void add(const std::string& name, const std::string& desc, Method f, + const InstanceGetter& getter, const LLSD& params, + const LLSD& defaults=LLSD()); //@} @@ -476,9 +471,8 @@ struct LLEventDispatcher::invoker } }; -template -typename std::enable_if< boost::function_types::is_nonmember_callable_builtin::value >::type -LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f) +template +void LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f) { // Construct an invoker_function, a callable accepting const args_source&. // Add to DispatchMap an ArrayParamsDispatchEntry that will handle the @@ -487,13 +481,9 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Functio boost::function_types::function_arity::value); } -template -typename std::enable_if< - boost::function_types::is_member_function_pointer::value && - ! std::is_convertible::value ->::type -LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, - const InstanceGetter& getter) +template +void LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, + const InstanceGetter& getter) { // Subtract 1 from the compile-time arity because the getter takes care of // the first parameter. We only need (arity - 1) additional arguments. @@ -501,23 +491,18 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Method boost::function_types::function_arity::value - 1); } -template -typename std::enable_if< boost::function_types::is_nonmember_callable_builtin::value >::type -LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f, - const LLSD& params, const LLSD& defaults) +template +void LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f, + const LLSD& params, const LLSD& defaults) { // See comments for previous is_nonmember_callable_builtin add(). addMapParamsDispatchEntry(name, desc, make_invoker(f), params, defaults); } -template -typename std::enable_if< - boost::function_types::is_member_function_pointer::value && - ! std::is_convertible::value ->::type -LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, - const InstanceGetter& getter, - const LLSD& params, const LLSD& defaults) +template +void LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, + const InstanceGetter& getter, + const LLSD& params, const LLSD& defaults) { addMapParamsDispatchEntry(name, desc, make_invoker(f, getter), params, defaults); } @@ -560,17 +545,21 @@ LLEventDispatcher::make_invoker(Method f, const InstanceGetter& getter) * LLEventPump name and dispatch key, and add() its methods. Incoming events * will automatically be dispatched. */ -class LL_COMMON_API LLDispatchListener: public LLEventDispatcher +// Instead of containing an LLEventStream, LLDispatchListener derives from it. +// This allows an LLEventPumps::PumpFactory to return a pointer to an +// LLDispatchListener (subclass) instance, and still have ~LLEventPumps() +// properly clean it up. +class LL_COMMON_API LLDispatchListener: + public LLEventDispatcher, + public LLEventStream { public: LLDispatchListener(const std::string& pumpname, const std::string& key); - - std::string getPumpName() const { return mPump.getName(); } + virtual ~LLDispatchListener() {} private: bool process(const LLSD& event); - LLEventStream mPump; LLTempBoundListener mBoundListener; }; diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index 5725dad9cc..1a305ec3dc 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -90,6 +90,13 @@ bool LLEventPumps::registerTypeFactory(const std::string& type, const TypeFactor return true; } +void LLEventPumps::unregisterTypeFactory(const std::string& type) +{ + auto found = mFactories.find(type); + if (found != mFactories.end()) + mFactories.erase(found); +} + bool LLEventPumps::registerPumpFactory(const std::string& name, const PumpFactory& factory) { // Do we already have a pump by this name? @@ -109,10 +116,30 @@ bool LLEventPumps::registerPumpFactory(const std::string& name, const PumpFactor static std::string nul(1, '\0'); std::string type_name{ nul + name }; mTypes[name] = type_name; - mFactories[type_name] = factory; + // TypeFactory is called with (name, tweak, type), whereas PumpFactory + // accepts only name. We could adapt with std::bind(), but this lambda + // does the trick. + mFactories[type_name] = + [factory] + (const std::string& name, bool /*tweak*/, const std::string& /*type*/) + { return factory(name); }; return true; } +void LLEventPumps::unregisterPumpFactory(const std::string& name) +{ + auto tfound = mTypes.find(name); + if (tfound != mTypes.end()) + { + auto ffound = mFactories.find(tfound->second); + if (ffound != mFactories.end()) + { + mFactories.erase(ffound); + } + mTypes.erase(tfound); + } +} + LLEventPump& LLEventPumps::obtain(const std::string& name) { PumpMap::iterator found = mPumpMap.find(name); diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index 38adc31121..c1dbf4392f 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -280,6 +280,7 @@ public: * a TypeFactory for the specified @a type name. */ bool registerTypeFactory(const std::string& type, const TypeFactory& factory); + void unregisterTypeFactory(const std::string& type); /// function passed to registerPumpFactory() typedef std::function PumpFactory; @@ -304,6 +305,7 @@ public: * instantiated an LLEventPump(name), so obtain(name) returned that. */ bool registerPumpFactory(const std::string& name, const PumpFactory& factory); + void unregisterPumpFactory(const std::string& name); /** * Find the named LLEventPump instance. If it exists post the message to it. @@ -362,13 +364,13 @@ testable: typedef std::set PumpSet; PumpSet mOurPumps; // for make(), map string type name to LLEventPump subclass factory function - typedef std::map PumpFactories; + typedef std::map TypeFactories; // Data used by make(). // One might think mFactories and mTypes could reasonably be static. So // they could -- if not for the fact that make() or obtain() might be // called before this module's static variables have been initialized. // This is why we use singletons in the first place. - PumpFactories mFactories; + TypeFactories mFactories; // for obtain(), map desired string instance name to string type when // obtain() must create the instance diff --git a/indra/llcommon/tests/lazyeventapi_test.cpp b/indra/llcommon/tests/lazyeventapi_test.cpp new file mode 100644 index 0000000000..6639c5e540 --- /dev/null +++ b/indra/llcommon/tests/lazyeventapi_test.cpp @@ -0,0 +1,89 @@ +/** + * @file lazyeventapi_test.cpp + * @author Nat Goodspeed + * @date 2022-06-18 + * @brief Test for lazyeventapi. + * + * $LicenseInfo:firstyear=2022&license=viewerlgpl$ + * Copyright (c) 2022, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "lazyeventapi.h" +// STL headers +// std headers +// external library headers +// other Linden headers +#include "../test/lltut.h" +#include "llevents.h" + +// LLEventAPI listener subclass +class MyListener: public LLEventAPI +{ +public: + MyListener(const LL::LazyEventAPIParams& params): + LLEventAPI(params) + {} + + void get(const LLSD& event) + { + std::cout << "MyListener::get() got " << event << std::endl; + } +}; + +// LazyEventAPI registrar subclass +class MyRegistrar: public LL::LazyEventAPI +{ + using super = LL::LazyEventAPI; + using super::listener; +public: + MyRegistrar(): + super("Test", "This is a test LLEventAPI") + { + add("get", "This is a get operation", &listener::get); + } +}; +// Normally we'd declare a static instance of MyRegistrar -- but because we +// may want to test with and without, defer declaration to individual test +// methods. + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct lazyeventapi_data + { + ~lazyeventapi_data() + { + // after every test, reset LLEventPumps + LLEventPumps::deleteSingleton(); + } + }; + typedef test_group lazyeventapi_group; + typedef lazyeventapi_group::object object; + lazyeventapi_group lazyeventapigrp("lazyeventapi"); + + template<> template<> + void object::test<1>() + { + set_test_name("LazyEventAPI"); + // this is where the magic (should) happen + // 'register' still a keyword until C++17 + MyRegistrar regster; + LLEventPumps::instance().obtain("Test").post("hey"); + } + + template<> template<> + void object::test<2>() + { + set_test_name("No LazyEventAPI"); + // Because the MyRegistrar declaration in test<1>() is local, because + // it has been destroyed, we fully expect NOT to reach a MyListener + // instance with this post. + LLEventPumps::instance().obtain("Test").post("moot"); + } +} // namespace tut -- cgit v1.3 From fdc0257acbde5a2d5bb201efcc8bb723df09daf8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 21 Jun 2022 15:23:29 -0400 Subject: DRTVWR-564: Fix LLEventDispatcher::addMethod() for LazyEventAPI. A classic LLEventAPI subclass calls LLEventDispatcher::add() methods in its own constructor. At that point, addMethod() can reliably dynamic_cast its 'this' pointer to the new subclass. But because of the way LazyEventAPI queues up add() calls, they're invoked in the (new) LLEventAPI constructor itself. The subclass constructor body hasn't even started running, and LLEventDispatcher::addMethod()'s dynamic_cast to the LLEventAPI subclass returns nullptr. addMethod() claims the new subclass isn't derived from LLEventDispatcher, which is confusing since it is. It works to change addMethod()'s dynamic_cast to static_cast. Flesh out lazyeventapi_test.cpp. post() maps with "op" keys to actually try to engage the registered operation. Give the operation an observable side effect; use ensure_mumble() to verify. Also verify that LazyEventAPI has captured the subject LLEventAPI's metadata in a way we can retrieve. --- indra/llcommon/lleventdispatcher.h | 2 +- indra/llcommon/tests/lazyeventapi_test.cpp | 47 ++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 7 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index ce9d3775cc..2e140329f3 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -329,7 +329,7 @@ private: void addMethod(const std::string& name, const std::string& desc, const METHOD& method, const LLSD& required) { - CLASS* downcast = dynamic_cast(this); + CLASS* downcast = static_cast(this); if (! downcast) { addFail(name, typeid(CLASS).name()); diff --git a/indra/llcommon/tests/lazyeventapi_test.cpp b/indra/llcommon/tests/lazyeventapi_test.cpp index 6639c5e540..4c78fd7105 100644 --- a/indra/llcommon/tests/lazyeventapi_test.cpp +++ b/indra/llcommon/tests/lazyeventapi_test.cpp @@ -19,18 +19,25 @@ // other Linden headers #include "../test/lltut.h" #include "llevents.h" +#include "llsdutil.h" + +// observable side effect, solely for testing +static LLSD data; // LLEventAPI listener subclass class MyListener: public LLEventAPI { public: + // need this trivial forwarding constructor + // (of course do any other initialization your subclass requires) MyListener(const LL::LazyEventAPIParams& params): LLEventAPI(params) {} - void get(const LLSD& event) + // example operation, registered by LazyEventAPI subclass below + void set_data(const LLSD& event) { - std::cout << "MyListener::get() got " << event << std::endl; + data = event["data"]; } }; @@ -40,14 +47,17 @@ class MyRegistrar: public LL::LazyEventAPI using super = LL::LazyEventAPI; using super::listener; public: + // LazyEventAPI subclass initializes like a classic LLEventAPI subclass + // constructor, with API name and desc plus add() calls for the defined + // operations MyRegistrar(): super("Test", "This is a test LLEventAPI") { - add("get", "This is a get operation", &listener::get); + add("set", "This is a set operation", &listener::set_data); } }; // Normally we'd declare a static instance of MyRegistrar -- but because we -// may want to test with and without, defer declaration to individual test +// want to test both with and without, defer declaration to individual test // methods. /***************************************************************************** @@ -57,6 +67,11 @@ namespace tut { struct lazyeventapi_data { + lazyeventapi_data() + { + // before every test, reset 'data' + data.clear(); + } ~lazyeventapi_data() { // after every test, reset LLEventPumps @@ -74,7 +89,8 @@ namespace tut // this is where the magic (should) happen // 'register' still a keyword until C++17 MyRegistrar regster; - LLEventPumps::instance().obtain("Test").post("hey"); + LLEventPumps::instance().obtain("Test").post(llsd::map("op", "set", "data", "hey")); + ensure_equals("failed to set data", data.asString(), "hey"); } template<> template<> @@ -84,6 +100,25 @@ namespace tut // Because the MyRegistrar declaration in test<1>() is local, because // it has been destroyed, we fully expect NOT to reach a MyListener // instance with this post. - LLEventPumps::instance().obtain("Test").post("moot"); + LLEventPumps::instance().obtain("Test").post(llsd::map("op", "set", "data", "moot")); + ensure("accidentally set data", ! data.isDefined()); + } + + template<> template<> + void object::test<3>() + { + set_test_name("LazyEventAPI metadata"); + MyRegistrar regster; + const MyRegistrar* found = nullptr; + for (const auto& registrar : LL::LazyEventAPIBase::instance_snapshot()) + if ((found = dynamic_cast(®istrar))) + break; + ensure("Failed to find MyRegistrar via LLInstanceTracker", found); + ensure_equals("wrong API name", found->mParams.name, "Test"); + ensure_contains("wrong API desc", found->mParams.desc, "test LLEventAPI"); + ensure_equals("wrong API field", found->mParams.field, "op"); + ensure_equals("failed to find operations", found->mOperations.size(), 1); + ensure_equals("wrong operation name", found->mOperations[0].first, "set"); + ensure_contains("wrong operation desc", found->mOperations[0].second, "set operation"); } } // namespace tut -- cgit v1.3 From 6b53036f7499a4e42813378009050eaf02c0b69d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 22 Jun 2022 10:51:11 -0400 Subject: DRTVWR-564: Allow LLLeapListener to report LazyEventAPIs too. One important factor in the design of LazyEventAPI was the desire to allow LLLeapListener to query metadata for an LLEventAPI even if it hasn't yet been instantiated by LazyEventAPI. That's why LazyEventAPI requires the same metadata required by a classic LLEventAPI. Instead of just publicly exposing its data members, give LazyEventAPI a query API mimicking LLEventAPI / LLEventDispatcher. Protect data members and private methods. Adapt lazyeventapi_test.cpp accordingly. Extend LLLeapListener::getAPIs() and getAPI() to look through LazyEventAPIBase instances after first checking existing LLEventAPI instances. Because the query API for LazyEventAPIBase mimics LLEventAPI's, extract getAPI()'s actual metadata reporting to a new internal template function reportAPI(). While we're touching LLLeapListener, we no longer need BOOST_FOREACH(). --- indra/llcommon/lazyeventapi.cpp | 19 ++++++++ indra/llcommon/lazyeventapi.h | 36 ++++++++++++--- indra/llcommon/llleaplistener.cpp | 70 ++++++++++++++++++++++-------- indra/llcommon/tests/lazyeventapi_test.cpp | 24 +++++++--- 4 files changed, 120 insertions(+), 29 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/lazyeventapi.cpp b/indra/llcommon/lazyeventapi.cpp index aefc2db6da..028af9f33f 100644 --- a/indra/llcommon/lazyeventapi.cpp +++ b/indra/llcommon/lazyeventapi.cpp @@ -15,9 +15,11 @@ #include "lazyeventapi.h" // STL headers // std headers +#include // std::find_if // external library headers // other Linden headers #include "llevents.h" +#include "llsdutil.h" LL::LazyEventAPIBase::LazyEventAPIBase( const std::string& name, const std::string& desc, const std::string& field) @@ -51,3 +53,20 @@ LL::LazyEventAPIBase::~LazyEventAPIBase() LLEventPumps::instance().unregisterPumpFactory(mParams.name); } } + +LLSD LL::LazyEventAPIBase::getMetadata(const std::string& name) const +{ + // Since mOperations is a vector rather than a map, just search. + auto found = std::find_if(mOperations.begin(), mOperations.end(), + [&name](const auto& namedesc) + { return (namedesc.first == name); }); + if (found == mOperations.end()) + return {}; + + // LLEventDispatcher() supplements the returned metadata in different + // ways, depending on metadata provided to the specific add() method. + // Don't try to emulate all that. At some point we might consider more + // closely unifying LLEventDispatcher machinery with LazyEventAPI, but for + // now this will have to do. + return llsd::map("name", found->first, "desc", found->second); +} diff --git a/indra/llcommon/lazyeventapi.h b/indra/llcommon/lazyeventapi.h index 7267a3e4ec..a815b119f0 100644 --- a/indra/llcommon/lazyeventapi.h +++ b/indra/llcommon/lazyeventapi.h @@ -63,9 +63,6 @@ namespace LL LazyEventAPIBase& operator=(const LazyEventAPIBase&) = delete; LazyEventAPIBase& operator=(LazyEventAPIBase&&) = delete; - // actually instantiate the companion LLEventAPI subclass - virtual LLEventPump* construct(const std::string& name) = 0; - // capture add() calls we want to play back on LLEventAPI construction template void add(const std::string& name, const std::string& desc, ARGS&&... rest) @@ -96,12 +93,40 @@ namespace LL }); } + // The following queries mimic the LLEventAPI / LLEventDispatcher + // query API. + + // Get the string name of the subject LLEventAPI + std::string getName() const { return mParams.name; } + // Get the documentation string + std::string getDesc() const { return mParams.desc; } + // Retrieve the LLSD key we use for dispatching + std::string getDispatchKey() const { return mParams.field; } + + // operations + using NameDesc = std::pair; + + private: // metadata that might be queried by LLLeapListener - std::vector> mOperations; + std::vector mOperations; + + public: + using const_iterator = decltype(mOperations)::const_iterator; + const_iterator begin() const { return mOperations.begin(); } + const_iterator end() const { return mOperations.end(); } + LLSD getMetadata(const std::string& name) const; + + protected: // Params with which to instantiate the companion LLEventAPI subclass LazyEventAPIParams mParams; private: + // true if we successfully registered our LLEventAPI on construction + bool mRegistered; + + // actually instantiate the companion LLEventAPI subclass + virtual LLEventPump* construct(const std::string& name) = 0; + // Passing an overloaded function to any function that accepts an // arbitrary callable is a PITB because you have to specify the // correct overload. What we want is for the compiler to select the @@ -117,8 +142,6 @@ namespace LL { instance->add(std::forward(args)...); } - - bool mRegistered; }; /** @@ -168,6 +191,7 @@ namespace LL LazyEventAPIBase(name, desc, field) {} + private: LLEventPump* construct(const std::string& /*name*/) override { // base class has carefully assembled LazyEventAPIParams embedded diff --git a/indra/llcommon/llleaplistener.cpp b/indra/llcommon/llleaplistener.cpp index 11bfec1b31..471f52e91c 100644 --- a/indra/llcommon/llleaplistener.cpp +++ b/indra/llcommon/llleaplistener.cpp @@ -14,14 +14,16 @@ // associated header #include "llleaplistener.h" // STL headers -#include +#include // std::find_if #include +#include +#include // std headers // external library headers -#include // other Linden headers -#include "lluuid.h" +#include "lazyeventapi.h" #include "llsdutil.h" +#include "lluuid.h" #include "stringize.h" /***************************************************************************** @@ -110,7 +112,7 @@ LLLeapListener::~LLLeapListener() // value_type, and Bad Things would happen if you copied an // LLTempBoundListener. (Destruction of the original would disconnect the // listener, invalidating every stored connection.) - BOOST_FOREACH(ListenersMap::value_type& pair, mListeners) + for (ListenersMap::value_type& pair : mListeners) { pair.second.disconnect(); } @@ -208,31 +210,65 @@ void LLLeapListener::getAPIs(const LLSD& request) const { Response reply(LLSD(), request); + // first, traverse existing LLEventAPI instances + std::set instances; for (auto& ea : LLEventAPI::instance_snapshot()) { - LLSD info; - info["desc"] = ea.getDesc(); - reply[ea.getName()] = info; + // remember which APIs are actually instantiated + instances.insert(ea.getName()); + reply[ea.getName()] = llsd::map("desc", ea.getDesc()); + } + // supplement that with *potential* instances: that is, instances of + // LazyEventAPI that can each instantiate an LLEventAPI on demand + for (const auto& lea : LL::LazyEventAPIBase::instance_snapshot()) + { + // skip any LazyEventAPI that's already instantiated its LLEventAPI + if (instances.find(lea.getName()) == instances.end()) + { + reply[lea.getName()] = llsd::map("desc", lea.getDesc()); + } } } +// Because LazyEventAPI deliberately mimics LLEventAPI's query API, this +// function can be passed either -- even though they're unrelated types. +template +void reportAPI(LLEventAPI::Response& reply, const API& api) +{ + reply["name"] = api.getName(); + reply["desc"] = api.getDesc(); + reply["key"] = api.getDispatchKey(); + LLSD ops; + for (const auto& namedesc : api) + { + ops.append(api.getMetadata(namedesc.first)); + } + reply["ops"] = ops; +} + void LLLeapListener::getAPI(const LLSD& request) const { Response reply(LLSD(), request); - auto found = LLEventAPI::getInstance(request["api"]); - if (found) + // check first among existing LLEventAPI instances + auto foundea = LLEventAPI::getInstance(request["api"]); + if (foundea) + { + reportAPI(reply, *foundea); + } + else { - reply["name"] = found->getName(); - reply["desc"] = found->getDesc(); - reply["key"] = found->getDispatchKey(); - LLSD ops; - for (LLEventAPI::const_iterator oi(found->begin()), oend(found->end()); - oi != oend; ++oi) + // Here the requested LLEventAPI doesn't yet exist, but do we have a + // registered LazyEventAPI for it? + LL::LazyEventAPIBase::instance_snapshot snap; + auto foundlea = std::find_if(snap.begin(), snap.end(), + [api = request["api"].asString()] + (const auto& lea) + { return (lea.getName() == api); }); + if (foundlea != snap.end()) { - ops.append(found->getMetadata(oi->first)); + reportAPI(reply, *foundlea); } - reply["ops"] = ops; } } diff --git a/indra/llcommon/tests/lazyeventapi_test.cpp b/indra/llcommon/tests/lazyeventapi_test.cpp index 4c78fd7105..31b2d6d17f 100644 --- a/indra/llcommon/tests/lazyeventapi_test.cpp +++ b/indra/llcommon/tests/lazyeventapi_test.cpp @@ -109,16 +109,28 @@ namespace tut { set_test_name("LazyEventAPI metadata"); MyRegistrar regster; + // Of course we have 'regster' in hand; we don't need to search for + // it. But this next test verifies that we can find (all) LazyEventAPI + // instances using LazyEventAPIBase::instance_snapshot. Normally we + // wouldn't search; normally we'd just look at each instance in the + // loop body. const MyRegistrar* found = nullptr; for (const auto& registrar : LL::LazyEventAPIBase::instance_snapshot()) if ((found = dynamic_cast(®istrar))) break; ensure("Failed to find MyRegistrar via LLInstanceTracker", found); - ensure_equals("wrong API name", found->mParams.name, "Test"); - ensure_contains("wrong API desc", found->mParams.desc, "test LLEventAPI"); - ensure_equals("wrong API field", found->mParams.field, "op"); - ensure_equals("failed to find operations", found->mOperations.size(), 1); - ensure_equals("wrong operation name", found->mOperations[0].first, "set"); - ensure_contains("wrong operation desc", found->mOperations[0].second, "set operation"); + + ensure_equals("wrong API name", found->getName(), "Test"); + ensure_contains("wrong API desc", found->getDesc(), "test LLEventAPI"); + ensure_equals("wrong API field", found->getDispatchKey(), "op"); + // Normally we'd just iterate over *found. But for test purposes, + // actually capture the range of NameDesc pairs in a vector. + std::vector ops{ found->begin(), found->end() }; + ensure_equals("failed to find operations", ops.size(), 1); + ensure_equals("wrong operation name", ops[0].first, "set"); + ensure_contains("wrong operation desc", ops[0].second, "set operation"); + LLSD metadata{ found->getMetadata(ops[0].first) }; + ensure_equals("bad metadata name", metadata["name"].asString(), ops[0].first); + ensure_equals("bad metadata desc", metadata["desc"].asString(), ops[0].second); } } // namespace tut -- cgit v1.3 From 324f0d9b8abad3a74a7c19a6e28f8c77c76b3b83 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 22 Aug 2022 21:00:42 -0400 Subject: DRTVWR-558: Fix builds on macOS 12.5 Monterey. Always search for python3[.exe] instead of plain 'python'. macOS Monterey no longer bundles Python 2 at all. Explicitly make PYTHON_EXECUTABLE a cached value so if the user edits it in CMakeCache.txt, it won't be overwritten by indra/cmake/Python.cmake. Do NOT set DYLD_LIBRARY_PATH for test executables! That has Bad Effects, as discussed in https://stackoverflow.com/q/73418423/5533635. Instead, create symlinks from build-mumble/sharedlibs/Resources -> Release/Resources and from build-mumble/test/Resources -> ../sharedlibs/Release/Resources. For test executables in sharedlibs/RelWithDebInfo and test/RelWithDebInfo, this supports our dylibs' baked-in load path @executable_path/../Resources. That load path assumes running in a standard app bundle (which the viewer in fact does), but we've been avoiding creating an app bundle for every test program. These symlinks allow us to continue doing that while avoiding DYLD_LIBRARY_PATH. Add indra/llcommon/apply.h. The LL::apply() function and its wrapper macro VAPPLY were very useful in diagnosing the problem. Tweak llleap_test.cpp. This source was modified extensively for diagnostic purposes; these are the small improvements that remain. (cherry picked from commit 15d37713b9113a6f70dde48c764df02c76e18cbc) (cherry picked from commit a1adcf1905d1fbc5fe07ff5a627295ccfe461ac4) --- indra/llcommon/tests/llleap_test.cpp | 1 + indra/test/CMakeLists.txt | 7 +++++++ 2 files changed, 8 insertions(+) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index 7ee36a9ea6..168825a8cd 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -17,6 +17,7 @@ // std headers #include // external library headers +//#include #include #include // other Linden headers diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt index b7a39a7450..4a0a8716c4 100644 --- a/indra/test/CMakeLists.txt +++ b/indra/test/CMakeLists.txt @@ -74,6 +74,13 @@ if (WINDOWS) LINK_FLAGS "/NODEFAULTLIB:LIBCMT" LINK_FLAGS_DEBUG "/NODEFAULTLIB:\"LIBCMT;LIBCMTD;MSVCRT\"" ) +elseif (DARWIN) + # Support our "@executable_path/../Resources" load path for our test + # executable. This SHOULD properly be "$/Resources", + # but the CMake $ generator expression isn't evaluated by + # CREATE_LINK, so fudge it. + file(CREATE_LINK "../sharedlibs/Release/Resources" "${CMAKE_BINARY_DIR}/test/Resources" + SYMBOLIC) endif (WINDOWS) set(TEST_EXE $) -- cgit v1.3 From c682603417e1ef8290aacf274ff49821bd204c0b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 19 Dec 2022 16:29:06 -0500 Subject: DRTVWR-558: Extend LL::apply() to LLSD array arguments. Make apply(function, std::array) and apply(function, std::vector) available even when we borrow the C++17 implementation of apply(function, std::tuple). Add apply(function, LLSD) with interpretations: * isUndefined() is treated as an empty array, for calling a nullary function * scalar LLSD is treated as a single-entry array, for calling a unary function * isArray() converts function parameters using LLSDParam * isMap() is an error. Add unit tests for all flavors of LL::apply(). (cherry picked from commit 3006c24251c6259d00df9e0f4f66b8a617e6026d) --- indra/llcommon/CMakeLists.txt | 2 + indra/llcommon/apply.h | 23 +++-- indra/llcommon/llsdutil.h | 81 ++++++++++++++++- indra/llcommon/tests/apply_test.cpp | 171 ++++++++++++++++++++++++++++++++++++ 4 files changed, 269 insertions(+), 8 deletions(-) create mode 100644 indra/llcommon/tests/apply_test.cpp (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 91ae2440fa..941d2d7baf 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -293,6 +293,8 @@ if (LL_TESTS) #set(TEST_DEBUG on) set(test_libs llcommon) + LL_ADD_INTEGRATION_TEST(apply "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(bitpack "" "${test_libs}") LL_ADD_INTEGRATION_TEST(classic_callback "" "${test_libs}") LL_ADD_INTEGRATION_TEST(commonmisc "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lazyeventapi "" "${test_libs}") diff --git a/indra/llcommon/apply.h b/indra/llcommon/apply.h index 7c58d63bc0..26753f5017 100644 --- a/indra/llcommon/apply.h +++ b/indra/llcommon/apply.h @@ -13,6 +13,7 @@ #define LL_APPLY_H #include +#include #include namespace LL @@ -54,6 +55,9 @@ namespace LL }, \ (ARGS)) +/***************************************************************************** +* apply(function, tuple) +*****************************************************************************/ #if __cplusplus >= 201703L // C++17 implementation @@ -63,8 +67,8 @@ using std::apply; // Derived from https://stackoverflow.com/a/20441189 // and https://en.cppreference.com/w/cpp/utility/apply -template -auto apply_impl(CALLABLE&& func, TUPLE&& args, std::index_sequence) +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))...); @@ -81,6 +85,11 @@ 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) @@ -88,13 +97,15 @@ auto apply(CALLABLE&& func, const std::array& args) return apply(std::forward(func), std::tuple_cat(args)); } +/***************************************************************************** +* apply(function, std::vector) +*****************************************************************************/ // per https://stackoverflow.com/a/28411055/5533635 template auto apply_impl(CALLABLE&& func, const std::vector& args, std::index_sequence) { - return apply_impl(std::forward(func), - std::make_tuple(std::forward(args[I])...), - I...); + return apply(std::forward(func), + std::make_tuple(args[I]...)); } // this goes beyond C++17 std::apply() @@ -108,8 +119,6 @@ auto apply(CALLABLE&& func, const std::vector& args) std::make_index_sequence()); } -#endif // C++14 - } // namespace LL #endif /* ! defined(LL_APPLY_H) */ diff --git a/indra/llcommon/llsdutil.h b/indra/llcommon/llsdutil.h index 372278c51a..eaf8825791 100644 --- a/indra/llcommon/llsdutil.h +++ b/indra/llcommon/llsdutil.h @@ -31,6 +31,7 @@ #include "llsd.h" #include +#include // U32 LL_COMMON_API LLSD ll_sd_from_U32(const U32); @@ -332,6 +333,31 @@ private: T _value; }; +/** + * LLSDParam is for when you don't already have the target parameter + * type in hand. Instantiate LLSDParam(your LLSD object), and the + * templated conversion operator will try to select a more specific LLSDParam + * specialization. + */ +template <> +class LLSDParam +{ +private: + LLSD value_; + +public: + LLSDParam(const LLSD& value): value_(value) {} + + /// if we're literally being asked for an LLSD parameter, avoid infinite + /// recursion + operator LLSD() const { return value_; } + + /// otherwise, instantiate a more specific LLSDParam to convert; that + /// preserves the existing customization mechanism + template + operator T() const { return LLSDParam(value_); } +}; + /** * Turns out that several target types could accept an LLSD param using any of * a few different conversions, e.g. LLUUID's constructor can accept LLUUID or @@ -350,7 +376,7 @@ class LLSDParam \ { \ public: \ LLSDParam(const LLSD& value): \ - _value((T)value.AS()) \ + _value((T)value.AS()) \ {} \ \ operator T() const { return _value; } \ @@ -555,4 +581,57 @@ struct hash } }; } + +namespace LL +{ + +/***************************************************************************** +* apply(function, LLSD array) +*****************************************************************************/ +// 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 +// tuple to the function -- how would make_tuple() deduce the correct +// parameter type for each entry? We must go directly to the target function. +template +auto apply_impl(CALLABLE&& func, const LLSD& array, std::index_sequence) +{ + // call func(unpacked args), using generic LLSDParam to convert each + // entry in 'array' to the target parameter type + return std::forward(func)(LLSDParam(array[I])...); +} + +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()); +} + +} // namespace LL + #endif // LL_LLSDUTIL_H diff --git a/indra/llcommon/tests/apply_test.cpp b/indra/llcommon/tests/apply_test.cpp new file mode 100644 index 0000000000..1f1085a702 --- /dev/null +++ b/indra/llcommon/tests/apply_test.cpp @@ -0,0 +1,171 @@ +/** + * @file apply_test.cpp + * @author Nat Goodspeed + * @date 2022-12-19 + * @brief Test 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 "../test/lltut.h" +#include "llsd.h" +#include "llsdutil.h" + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + namespace statics + { + /*------------------------------ data ------------------------------*/ + // Although we're using types from the LLSD namespace, we're not + // constructing LLSD values, but rather instances of the C++ types + // supported by LLSD. + static LLSD::Boolean b{true}; + static LLSD::Integer i{17}; + static LLSD::Real f{3.14}; + static LLSD::String s{ "hello" }; + static LLSD::UUID uu{ "baadf00d-dead-beef-baad-feedb0ef" }; + static LLSD::Date dt{ "2022-12-19" }; + static LLSD::URI uri{ "http://secondlife.com" }; + static LLSD::Binary bin{ 0x01, 0x02, 0x03, 0x04, 0x05 }; + + static std::vector quick + { + "The", "quick", "brown", "fox", "etc." + }; + + static std::array fibs + { + 0, 1, 1, 2, 3 + }; + + // ensure that apply() actually reaches the target method -- + // lack of ensure_equals() failure could be due to no-op apply() + bool called{ false }; + + /*------------------------- test functions -------------------------*/ + void various(LLSD::Boolean b, LLSD::Integer i, LLSD::Real f, const LLSD::String& s, + const LLSD::UUID& uu, const LLSD::Date& dt, + const LLSD::URI& uri, const LLSD::Binary& bin) + { + called = true; + ensure_equals( "b mismatch", b, statics::b); + ensure_equals( "i mismatch", i, statics::i); + ensure_equals( "f mismatch", f, statics::f); + ensure_equals( "s mismatch", s, statics::s); + ensure_equals( "uu mismatch", uu, statics::uu); + ensure_equals( "dt mismatch", dt, statics::dt); + ensure_equals("uri mismatch", uri, statics::uri); + ensure_equals("bin mismatch", bin, statics::bin); + } + + void strings(std::string s0, std::string s1, std::string s2, std::string s3, std::string s4) + { + called = true; + ensure_equals("s0 mismatch", s0, statics::quick[0]); + ensure_equals("s1 mismatch", s1, statics::quick[1]); + ensure_equals("s2 mismatch", s2, statics::quick[2]); + ensure_equals("s3 mismatch", s3, statics::quick[3]); + ensure_equals("s4 mismatch", s4, statics::quick[4]); + } + + void ints(int i0, int i1, int i2, int i3, int i4) + { + called = true; + ensure_equals("i0 mismatch", i0, statics::fibs[0]); + ensure_equals("i1 mismatch", i1, statics::fibs[1]); + ensure_equals("i2 mismatch", i2, statics::fibs[2]); + ensure_equals("i3 mismatch", i3, statics::fibs[3]); + ensure_equals("i4 mismatch", i4, statics::fibs[4]); + } + + void intfunc(int i) + { + called = true; + ensure_equals("i mismatch", i, statics::i); + } + + void voidfunc() + { + called = true; + } + } // namespace statics + + struct apply_data + { + apply_data() + { + // reset called before each test + statics::called = false; + } + }; + typedef test_group apply_group; + typedef apply_group::object object; + apply_group applygrp("apply"); + + template<> template<> + void object::test<1>() + { + set_test_name("apply(tuple)"); + LL::apply(statics::various, + std::make_tuple(statics::b, statics::i, statics::f, statics::s, + statics::uu, statics::dt, statics::uri, statics::bin)); + ensure("apply(tuple) failed", statics::called); + } + + template<> template<> + void object::test<2>() + { + set_test_name("apply(array)"); + LL::apply(statics::ints, statics::fibs); + ensure("apply(array) failed", statics::called); + } + + template<> template<> + void object::test<3>() + { + set_test_name("apply(vector)"); + LL::apply(statics::strings, statics::quick); + ensure("apply(vector) failed", statics::called); + } + + // The various apply(LLSD) tests exercise only the success cases because + // the failure cases trigger assert() fail, which is hard to catch. + template<> template<> + void object::test<4>() + { + set_test_name("apply(LLSD())"); + LL::apply(statics::voidfunc, LLSD()); + ensure("apply(LLSD()) failed", statics::called); + } + + template<> template<> + void object::test<5>() + { + set_test_name("apply(LLSD scalar)"); + LL::apply(statics::intfunc, LLSD(statics::i)); + ensure("apply(LLSD scalar) failed", statics::called); + } + + template<> template<> + void object::test<6>() + { + set_test_name("apply(LLSD array)"); + LL::apply(statics::various, + llsd::array(statics::b, statics::i, statics::f, statics::s, + statics::uu, statics::dt, statics::uri, statics::bin)); + ensure("apply(LLSD array) failed", statics::called); + } +} // namespace tut -- cgit v1.3 From 196e49c1f8bd58ab9ce81843c10d452284ca7569 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 19 Dec 2022 17:27:36 -0500 Subject: DRTVWR-558: Add unit test for VAPPLY(). Add to apply_test.cpp a collect() function that incrementally accumulates an arbitrary number of arguments into a std::vector. Construct a std::array to pass it, using VAPPLY(). Clarify in header comments that LL::apply() can't call a variadic function with arguments of dynamic size: std::vector or LLSD. The compiler can deduce how many arguments to pass to a function with a fixed argument list; it can deduce how many arguments to pass to a variadic function with a fixed number of arguments. But it can't compile a call to a variadic function with an arguments data structure whose size can vary at runtime. (cherry picked from commit ceed33396266b123896f7cfb9b90abdf240e1eec) --- indra/llcommon/apply.h | 6 ++++- indra/llcommon/llsdutil.h | 6 +++++ indra/llcommon/tests/apply_test.cpp | 52 ++++++++++++++++++++++++++++++++++++- 3 files changed, 62 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/apply.h b/indra/llcommon/apply.h index 26753f5017..9f4c268895 100644 --- a/indra/llcommon/apply.h +++ b/indra/llcommon/apply.h @@ -108,7 +108,11 @@ auto apply_impl(CALLABLE&& func, const std::vector& args, std::index_sequence std::make_tuple(args[I]...)); } -// this goes beyond C++17 std::apply() +/** + * 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 + * time how many arguments to pass. This isn't Python. + */ template auto apply(CALLABLE&& func, const std::vector& args) { diff --git a/indra/llcommon/llsdutil.h b/indra/llcommon/llsdutil.h index eaf8825791..eddaa64bd2 100644 --- a/indra/llcommon/llsdutil.h +++ b/indra/llcommon/llsdutil.h @@ -31,6 +31,7 @@ #include "llsd.h" #include +#include #include // U32 @@ -601,6 +602,11 @@ auto apply_impl(CALLABLE&& func, const LLSD& array, std::index_sequence) return std::forward(func)(LLSDParam(array[I])...); } +/** + * apply(function, LLSD) goes beyond C++17 std::apply(). For this case + * @a function @emph cannot be variadic: the compiler must know at compile + * time how many arguments to pass. This isn't Python. + */ template auto apply(CALLABLE&& func, const LLSD& args) { diff --git a/indra/llcommon/tests/apply_test.cpp b/indra/llcommon/tests/apply_test.cpp index 1f1085a702..28ee3f9c81 100644 --- a/indra/llcommon/tests/apply_test.cpp +++ b/indra/llcommon/tests/apply_test.cpp @@ -15,12 +15,27 @@ #include "apply.h" // STL headers // std headers +#include // external library headers // other Linden headers -#include "../test/lltut.h" #include "llsd.h" #include "llsdutil.h" +// for ensure_equals +std::ostream& operator<<(std::ostream& out, const std::vector& stringvec) +{ + const char* delim = "["; + for (const auto& str : stringvec) + { + out << delim << std::quoted(str); + delim = ", "; + } + return out << ']'; +} + +// the above must be declared BEFORE ensure_equals(std::vector) +#include "../test/lltut.h" + /***************************************************************************** * TUT *****************************************************************************/ @@ -54,6 +69,8 @@ namespace tut // ensure that apply() actually reaches the target method -- // lack of ensure_equals() failure could be due to no-op apply() bool called{ false }; + // capture calls from collect() + std::vector collected; /*------------------------- test functions -------------------------*/ void various(LLSD::Boolean b, LLSD::Integer i, LLSD::Real f, const LLSD::String& s, @@ -101,6 +118,20 @@ namespace tut { called = true; } + + // recursion tail + void collect() + { + called = true; + } + + // collect(arbitrary) + template + void collect(const std::string& first, ARGS&&... rest) + { + statics::collected.push_back(first); + collect(std::forward(rest)...); + } } // namespace statics struct apply_data @@ -109,6 +140,7 @@ namespace tut { // reset called before each test statics::called = false; + statics::collected.clear(); } }; typedef test_group apply_group; @@ -168,4 +200,22 @@ namespace tut statics::uu, statics::dt, statics::uri, statics::bin)); ensure("apply(LLSD array) failed", statics::called); } + + template<> template<> + void object::test<7>() + { + set_test_name("VAPPLY()"); + // Make a std::array from statics::quick. We can't call a + // variadic function with a data structure of dynamic length. + std::array strray; + for (size_t i = 0; i < strray.size(); ++i) + strray[i] = statics::quick[i]; + // This doesn't work: the compiler doesn't know which overload of + // collect() to pass to LL::apply(). + // LL::apply(statics::collect, strray); + // That's what VAPPLY() is for. + VAPPLY(statics::collect, strray); + ensure("VAPPLY() failed", statics::called); + ensure_equals("collected mismatch", statics::collected, statics::quick); + } } // namespace tut -- cgit v1.3 From 8855a82f512286bce6bd131d87dcafd303f2a5f6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 20 Dec 2022 09:48:36 -0500 Subject: DRTVWR-558: Add LL::apply() test for function(const LLSD&). (cherry picked from commit 7d33e00d925614911a7602da1bd79916cc849ad7) --- indra/llcommon/tests/apply_test.cpp | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/apply_test.cpp b/indra/llcommon/tests/apply_test.cpp index 28ee3f9c81..9a17afc18c 100644 --- a/indra/llcommon/tests/apply_test.cpp +++ b/indra/llcommon/tests/apply_test.cpp @@ -108,6 +108,12 @@ namespace tut ensure_equals("i4 mismatch", i4, statics::fibs[4]); } + void sdfunc(const LLSD& sd) + { + called = true; + ensure_equals("sd mismatch", sd.asInteger(), statics::i); + } + void intfunc(int i) { called = true; @@ -186,13 +192,23 @@ namespace tut template<> template<> void object::test<5>() { - set_test_name("apply(LLSD scalar)"); + set_test_name("apply(fn(int), LLSD scalar)"); LL::apply(statics::intfunc, LLSD(statics::i)); - ensure("apply(LLSD scalar) failed", statics::called); + ensure("apply(fn(int), LLSD scalar) failed", statics::called); } template<> template<> void object::test<6>() + { + set_test_name("apply(fn(LLSD), LLSD scalar)"); + // This test verifies that LLSDParam doesn't send the compiler + // into infinite recursion when the target is itself LLSD. + LL::apply(statics::sdfunc, LLSD(statics::i)); + ensure("apply(fn(LLSD), LLSD scalar) failed", statics::called); + } + + template<> template<> + void object::test<7>() { set_test_name("apply(LLSD array)"); LL::apply(statics::various, @@ -202,7 +218,7 @@ namespace tut } template<> template<> - void object::test<7>() + void object::test<8>() { set_test_name("VAPPLY()"); // Make a std::array from statics::quick. We can't call a -- 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') 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 8e61a8f7c0ebcc72a1c348e790eeb73b9388ccde Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 6 Jan 2023 17:06:39 -0500 Subject: DRTVWR-558: Allow directly streaming test helper class CaptureLog. (cherry picked from commit 374eb409b98795158b36e232f670d1302f31b9ff) --- indra/llcommon/tests/wrapllerrs.h | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index 3779fb41bc..d657b329bb 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -226,6 +226,11 @@ public: return boost::dynamic_pointer_cast(mRecorder)->streamto(out); } + friend inline std::ostream& operator<<(std::ostream& out, const CaptureLog& self) + { + return self.streamto(out); + } + private: LLError::FatalFunction mFatalFunction; LLError::SettingsStoragePtr mOldSettings; -- 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') 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') 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') 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') 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') 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') 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') 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 34b83e4047b3170e744538580dc8f7ddee387ba7 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Wed, 25 Oct 2023 22:57:57 +0300 Subject: Post merge build fix --- indra/llcommon/tests/apply_test.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'indra/llcommon/tests') diff --git a/indra/llcommon/tests/apply_test.cpp b/indra/llcommon/tests/apply_test.cpp index 9a17afc18c..56b497e0c8 100644 --- a/indra/llcommon/tests/apply_test.cpp +++ b/indra/llcommon/tests/apply_test.cpp @@ -20,6 +20,9 @@ // other Linden headers #include "llsd.h" #include "llsdutil.h" +#include +#include +#include // for ensure_equals std::ostream& operator<<(std::ostream& out, const std::vector& stringvec) -- 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') 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') 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