From 112f364f0109059f10735766dfdac911ad5b7ab5 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Mon, 9 Mar 2020 13:51:45 -0400 Subject: [PATCH] Unify ReadFieldNew / ReadFieldUpdate Replace with ReadField wrapper and CustomReadField hook to be consistent with BuildField / CustomBuildField. This - Makes it more straightforward pass custom types with symmetric CustomReadField / CustomBuildField overloads - Simplifies debugging and overload resolution, removing hacks that allowed ReadFieldNew calls to forward to ReadFieldUpdate calls and vice versa - Enables more flexibility and efficiency within CustomReadField implementations allowing emplacing or updating in the same function based on which is simpler or more efficient. Removes a lot of std::enable_if overload hacks - Should allow clientInvoke to use RVO and construct return values in place instead of copying or moving. This will allow more types to be for supported as IPC return values even if they don't support default construction, copying, or moving. - Improves separation of concerns, avoids need for ReadField callers to know whether callee will emplace or update, avoids need for emplace ReadDest object to have hardcoded support for map, vector, optional, shared_ptr, reference_wrapper types, so they now are implemented the same as any other custom type. --- include/mp/proxy-types.h | 486 +++++++++++++++++++-------------------- include/mp/util.h | 6 +- 2 files changed, 238 insertions(+), 254 deletions(-) diff --git a/include/mp/proxy-types.h b/include/mp/proxy-types.h index 255b702d..e29753e0 100644 --- a/include/mp/proxy-types.h +++ b/include/mp/proxy-types.h @@ -175,257 +175,292 @@ auto PassField(TypeList<>, ServerContext& server_context, const Fn& fn, const Ar kj::mv(future.promise)); } -template -class Emplace +template +struct ReadDestEmplace { - Value& m_value; + ReadDestEmplace(TypeList, EmplaceFn&& emplace_fn) : m_emplace_fn(emplace_fn) {} - template - static T& call(std::optional& value, Params&&... params) + template + auto& construct(Args&&... args) { - value.emplace(std::forward(params)...); - return *value; + return m_emplace_fn(std::forward(args)...); } - template - static T& call(std::vector& value, Params&&... params) + template + void update(UpdateFn&& update_fn) { - value.emplace_back(std::forward(params)...); - return value.back(); + update(std::forward(update_fn), Priority<1>()); } - template - static const T& call(std::set& value, Params&&... params) + template >>, UpdateFn>> + void update(UpdateFn&& update_fn, Priority<1>) { - return *value.emplace(std::forward(params)...).first; + update_fn(construct()); } - template - static std::pair& call(std::map& value, Params&&... params) + template + void update(UpdateFn&& update_fn, Priority<0>) { - return *value.emplace(std::forward(params)...).first; + std::remove_cv_t temp; + update_fn(temp); + construct(std::move(temp)); } - template - static T& call(std::shared_ptr& value, Params&&... params) + EmplaceFn& m_emplace_fn; +}; + +template +struct ReadDestValue +{ + ReadDestValue(Value& value) : m_value(value) {} + + template + void update(UpdateFn&& update_fn) { - value = std::make_shared(std::forward(params)...); - return *value; + update_fn(m_value); } - template - static T& call(std::reference_wrapper& value, Params&&... params) + template + Value& construct(Args&&... args) { - value.get().~T(); - new (&value.get()) T(std::forward(params)...); - return value.get(); + m_value.~Value(); + new (&m_value) Value(std::forward(args)...); + return m_value; } -public: - Emplace(Value& value) : m_value(value) {} - - // Needs to be declared after m_value for compiler to understand declaration. - template - auto operator()(Params&&... params) -> AUTO_RETURN(Emplace::call(this->m_value, std::forward(params)...)) + Value& m_value; }; -template -void ReadFieldUpdate(TypeList>, +template +decltype(auto) CustomReadField(TypeList>, InvokeContext& invoke_context, Input&& input, - DestValue&& value) -{ - if (!input.has()) { - value.reset(); - return; - } - if (value) { - ReadFieldUpdate(TypeList(), invoke_context, input, *value); - } else { - ReadField(TypeList(), invoke_context, input, Emplace(value)); - } + ReadDest&& read_dest) +{ + return read_dest.update([&](auto& value) { + if (!input.has()) { + value.reset(); + } else if (value) { + ReadField(TypeList(), invoke_context, input, ReadDestValue(*value)); + } else { + ReadField(TypeList(), invoke_context, input, + ReadDestEmplace(TypeList(), [&](auto&&... args) -> auto& { + value.emplace(std::forward(args)...); + return *value; + })); + } + }); } -template -void ReadFieldUpdate(TypeList>, +template +decltype(auto) CustomReadField(TypeList>, InvokeContext& invoke_context, Input&& input, - DestValue&& value) -{ - if (!input.has()) { - value.reset(); - return; - } - if (value) { - ReadFieldUpdate(TypeList(), invoke_context, input, *value); - } else { - ReadField(TypeList(), invoke_context, input, Emplace(value)); - } + ReadDest&& read_dest) +{ + return read_dest.update([&](auto& value) { + if (!input.has()) { + value.reset(); + } else if (value) { + ReadField(TypeList(), invoke_context, input, ReadDestValue(*value)); + } else { + ReadField(TypeList(), invoke_context, input, + ReadDestEmplace(TypeList(), [&](auto&&... args) -> auto& { + value = std::make_shared(std::forward(args)...); + return *value; + })); + } + }); } -template -void ReadFieldUpdate(TypeList, InvokeContext& invoke_context, Input&& input, DestValue&& value) +template +decltype(auto) CustomReadField(TypeList, + InvokeContext& invoke_context, + Input&& input, + ReadDest&& read_dest) { - if (value) { - ReadFieldUpdate(TypeList(), invoke_context, std::forward(input), *value); - } + return read_dest.update([&](auto& value) { + if (value) { + ReadField(TypeList(), invoke_context, std::forward(input), ReadDestValue(*value)); + } + }); } -template -void ReadFieldUpdate(TypeList>, +template +decltype(auto) CustomReadField(TypeList>, InvokeContext& invoke_context, Input&& input, - DestValue&& value) + ReadDest&& read_dest) { - if (!input.has()) { - value.reset(); - return; - } - ReadField(TypeList(), invoke_context, std::forward(input), Emplace(value)); + return read_dest.update([&](auto& value) { + if (!input.has()) { + value.reset(); + return; + } + ReadField(TypeList(), invoke_context, std::forward(input), + ReadDestEmplace(TypeList(), [&](auto&&... args) -> auto& { + value = std::make_shared(std::forward(args)...); + return *value; + })); + }); } -template -void ReadFieldUpdate(TypeList>, InvokeContext& invoke_context, Input&& input, DestValue&& value) -{ - auto data = input.get(); - value.clear(); - value.reserve(data.size()); - for (auto item : data) { - ReadField(TypeList(), invoke_context, Make(item), Emplace(value)); - } +template +decltype(auto) CustomReadField(TypeList>, + InvokeContext& invoke_context, + Input&& input, + ReadDest&& read_dest) +{ + return read_dest.update([&](auto& value) { + auto data = input.get(); + value.clear(); + value.reserve(data.size()); + for (auto item : data) { + ReadField(TypeList(), invoke_context, Make(item), + ReadDestEmplace(TypeList(), [&](auto&&... args) -> auto& { + value.emplace_back(std::forward(args)...); + return value.back(); + })); + } + }); } -template -void ReadFieldUpdate(TypeList>, InvokeContext& invoke_context, Input&& input, DestValue&& value) -{ - auto data = input.get(); - value.clear(); - for (auto item : data) { - ReadField(TypeList(), invoke_context, Make(item), Emplace(value)); - } +template +decltype(auto) CustomReadField(TypeList>, + InvokeContext& invoke_context, + Input&& input, + ReadDest&& read_dest) +{ + return read_dest.update([&](auto& value) { + auto data = input.get(); + value.clear(); + for (auto item : data) { + ReadField(TypeList(), invoke_context, Make(item), + ReadDestEmplace(TypeList(), [&](auto&&... args) -> auto& { + return *value.emplace(std::forward(args)...).first; + })); + } + }); } -template -void ReadFieldUpdate(TypeList>, +template +decltype(auto) CustomReadField(TypeList>, InvokeContext& invoke_context, Input&& input, - DestValue&& value) -{ - auto data = input.get(); - value.clear(); - for (auto item : data) { - ReadField(TypeList>(), invoke_context, Make(item), - Emplace(value)); - } + ReadDest&& read_dest) +{ + return read_dest.update([&](auto& value) { + auto data = input.get(); + value.clear(); + for (auto item : data) { + ReadField(TypeList>(), invoke_context, + Make(item), + ReadDestEmplace( + TypeList>(), [&](auto&&... args) -> auto& { + return *value.emplace(std::forward(args)...).first; + })); + } + }); } -// Emplace function that when called with tuple of key constructor arguments -// reads value from pair and calls piecewise construct. -template -struct PairValueEmplace -{ - InvokeContext& m_context; - Input& m_input; - Emplace& m_emplace; - template - - // FIXME Should really return reference to emplaced key object. - void operator()(KeyTuple&& key_tuple) - { - const auto& pair = m_input.get(); - using ValueAccessor = typename ProxyStruct::Reads>::ValueAccessor; - ReadField(TypeList(), m_context, Make(pair), - BindTuple(Make(GetFn<1>(), Bind(m_emplace, std::piecewise_construct, key_tuple)))); - } -}; - -template -void ReadFieldNew(TypeList>, +template +decltype(auto) CustomReadField(TypeList>, InvokeContext& invoke_context, Input&& input, - Emplace&& emplace) + ReadDest&& read_dest) { - /* This could be simplified a lot with c++14 generic lambdas. All it is doing is: - ReadField(TypeList(), invoke_context, Make(input.get().getKey()), [&](auto&&... key_args) - { ReadField(TypeList(), invoke_context, Make(input.get().getValue()), [&](auto&&... - value_args) - { - emplace(std::piecewise_construct, std::forward_as_tuple(key_args...), - std::forward_as_tuple(value_args...)); - }) - }); - */ const auto& pair = input.get(); using KeyAccessor = typename ProxyStruct::Reads>::KeyAccessor; - ReadField(TypeList(), invoke_context, Make(pair), - BindTuple(PairValueEmplace{invoke_context, input, emplace})); -} + using ValueAccessor = typename ProxyStruct::Reads>::ValueAccessor; -template -void ReadFieldUpdate(TypeList>, + ReadField(TypeList(), invoke_context, Make(pair), + ReadDestEmplace(TypeList(), [&](auto&&... key_args) -> auto& { + KeyLocalType* key = nullptr; + ReadField(TypeList(), invoke_context, Make(pair), + ReadDestEmplace(TypeList(), [&](auto&&... value_args) -> auto& { + auto& ret = read_dest.construct(std::piecewise_construct, std::forward_as_tuple(key_args...), + std::forward_as_tuple(value_args...)); + key = &ret.first; + return ret.second; + })); + return *key; + })); +} + +template +decltype(auto) CustomReadField(TypeList>, InvokeContext& invoke_context, Input&& input, - Tuple&& tuple) -{ - const auto& pair = input.get(); - using Struct = ProxyStruct::Reads>; - ReadFieldUpdate(TypeList(), invoke_context, Make(pair), - std::get<0>(tuple)); - ReadFieldUpdate(TypeList(), invoke_context, - Make(pair), std::get<1>(tuple)); + ReadDest&& read_dest) +{ + return read_dest.update([&](auto& value) { + const auto& pair = input.get(); + using Struct = ProxyStruct::Reads>; + ReadField(TypeList(), invoke_context, Make(pair), + ReadDestValue(std::get<0>(value))); + ReadField(TypeList(), invoke_context, Make(pair), + ReadDestValue(std::get<1>(value))); + }); } -template -void ReadFieldNew(TypeList, +template +decltype(auto) CustomReadField(TypeList, InvokeContext& invoke_context, Input&& input, - Emplace&& emplace, + ReadDest&& read_dest, typename std::enable_if::value>::type* enable = 0) { - emplace(static_cast(input.get())); + return read_dest.construct(static_cast(input.get())); } -template -void ReadFieldNew(TypeList, +template +decltype(auto) CustomReadField(TypeList, InvokeContext& invoke_context, Input&& input, - Emplace&& emplace, + ReadDest&& read_dest, typename std::enable_if::value>::type* enable = 0) { auto value = input.get(); if (value < std::numeric_limits::min() || value > std::numeric_limits::max()) { throw std::range_error("out of bound int received"); } - emplace(static_cast(value)); + return read_dest.construct(static_cast(value)); } -template -void ReadFieldNew(TypeList, +template +decltype(auto) CustomReadField(TypeList, InvokeContext& invoke_context, Input&& input, - Emplace&& emplace, + ReadDest&& read_dest, typename std::enable_if::value>::type* enable = 0) { auto value = input.get(); static_assert(std::is_same::value, "floating point type mismatch"); - emplace(value); + return read_dest.construct(value); } -template -void ReadFieldNew(TypeList, InvokeContext& invoke_context, Input&& input, Emplace&& emplace) +template +decltype(auto) CustomReadField(TypeList, + InvokeContext& invoke_context, + Input&& input, + ReadDest&& read_dest) { auto data = input.get(); - emplace(CharCast(data.begin()), data.size()); + return read_dest.construct(CharCast(data.begin()), data.size()); } -template -void ReadFieldUpdate(TypeList, +template +decltype(auto) CustomReadField(TypeList, InvokeContext& invoke_context, Input&& input, - unsigned char (&value)[size]) + ReadDest&& read_dest) { - auto data = input.get(); - memcpy(value, data.begin(), size); + return read_dest.update([&](auto& value) { + auto data = input.get(); + memcpy(value, data.begin(), size); + }); } template @@ -441,17 +476,19 @@ std::unique_ptr CustomMakeProxyClient(InvokeContext& context, typename Int return MakeProxyClient(context, kj::mv(client)); } -template -void ReadFieldNew(TypeList>, +template +decltype(auto) CustomReadField(TypeList>, InvokeContext& invoke_context, Input&& input, - Emplace&& emplace, + ReadDest&& read_dest, typename Decay::Calls* enable = nullptr) { using Interface = typename Decay::Calls; if (input.has()) { - emplace(CustomMakeProxyClient(invoke_context, std::move(input.get()))); + return read_dest.construct( + CustomMakeProxyClient(invoke_context, std::move(input.get()))); } + return read_dest.construct(); } // ProxyCallFn class is needed because c++11 doesn't support auto lambda parameters. @@ -466,34 +503,21 @@ struct ProxyCallFn auto operator()(CallParams&&... params) -> AUTO_RETURN(this->m_proxy->call(std::forward(params)...)) }; -template -void ReadFieldNew(TypeList>, +template +decltype(auto) CustomReadField(TypeList>, InvokeContext& invoke_context, Input&& input, - Emplace&& emplace) + ReadDest&& read_dest) { if (input.has()) { using Interface = typename Decay::Calls; auto client = std::make_shared>( input.get(), &invoke_context.connection, /* destroy_connection= */ false); - emplace(ProxyCallFn{std::move(client)}); + return read_dest.construct(ProxyCallFn{std::move(client)}); } + return read_dest.construct(); }; -template -void ReadFieldUpdate(TypeList, - InvokeContext& invoke_context, - Input&& input, - Value&& value, - decltype(ReadFieldNew(TypeList>(), - invoke_context, - std::forward(input), - std::declval>()))* enable = nullptr) -{ - auto ref = std::ref(value); - ReadFieldNew(TypeList>(), invoke_context, input, Emplace(ref)); -} - template void ReadOne(TypeList param, InvokeContext& invoke_context, @@ -506,8 +530,8 @@ void ReadOne(TypeList param, using Accessor = typename std::tuple_element::Accessors>::type; const auto& struc = input.get(); auto&& field_value = value.*ProxyType::get(Index()); - ReadFieldUpdate( - TypeList>(), invoke_context, Make(struc), field_value); + ReadField(TypeList>(), invoke_context, Make(struc), + ReadDestValue(field_value)); ReadOne(param, invoke_context, input, value); } @@ -520,66 +544,27 @@ void ReadOne(TypeList param, { } -template -void ReadFieldUpdate(TypeList param, +template +decltype(auto) CustomReadField(TypeList param, InvokeContext& invoke_context, Input&& input, - Value&& value, + ReadDest&& read_dest, typename ProxyType::Struct* enable = nullptr) { - ReadOne<0>(param, invoke_context, input, value); + return read_dest.update([&](auto& value) { ReadOne<0>(param, invoke_context, input, value); }); } -// ReadField calling Emplace when ReadFieldNew is available. -template -void ReadFieldImpl(TypeList, - Priority<2>, - InvokeContext& invoke_context, - Input&& input, - Emplace&& emplace, - decltype( - ReadFieldNew(TypeList>(), invoke_context, input, std::forward(emplace)))* enable = - nullptr) -{ - ReadFieldNew(TypeList>(), invoke_context, input, std::forward(emplace)); -} - -// ReadField calling Emplace when ReadFieldNew is not available, and emplace creates non-const object. -// Call emplace first to create empty value, then ReadFieldUpdate into the new object. -template -void ReadFieldImpl(TypeList, - Priority<1>, - InvokeContext& invoke_context, - Input&& input, - Emplace&& emplace, - typename std::enable_if::value && - !std::is_const::type>::value>::type* - enable = nullptr) +template +void ReadField(TypeList, Args&&... args) { - auto&& ref = emplace(); - ReadFieldUpdate(TypeList>(), invoke_context, input, ref); -} - -// ReadField calling Emplace when ReadFieldNew is not available, and emplace creates const object. -// Initialize temporary with ReadFieldUpdate then std::move into emplace. -template -void ReadFieldImpl(TypeList, Priority<0>, InvokeContext& invoke_context, Input&& input, Emplace&& emplace) -{ - Decay temp; - ReadFieldUpdate(TypeList>(), invoke_context, input, temp); - emplace(std::move(temp)); -} - -template -void ReadField(LocalTypes, InvokeContext& invoke_context, Input&& input, Values&&... values) -{ - ReadFieldImpl(LocalTypes(), Priority<2>(), invoke_context, input, std::forward(values)...); + CustomReadField(TypeList...>(), std::forward(args)...); } template void ThrowField(TypeList, InvokeContext& invoke_context, Input&& input) { - ReadField(TypeList(), invoke_context, input, ThrowFn()); + ReadField( + TypeList(), invoke_context, input, ReadDestEmplace(TypeList(), ThrowFn())); } //! Special case for generic std::exception. It's an abstract type so it can't @@ -970,7 +955,7 @@ void PassField(TypeList, ServerContext& server_context, const Fn& fn Decay param; MaybeReadField(std::integral_constant(), TypeList(), invoke_context, input, - Emplace(param)); + ReadDestValue(param)); fn.invoke(server_context, std::forward(args)..., ¶m); @@ -1014,15 +999,6 @@ template void MaybeReadField(std::false_type, Args&&...) { } -template -void MaybeReadFieldUpdate(std::true_type, Args&&... args) -{ - ReadFieldUpdate(std::forward(args)...); -} -template -void MaybeReadFieldUpdate(std::false_type, Args&&...) -{ -} template void MaybeSetWant(TypeList, Priority<1>, Value&& value, Output&& output) @@ -1041,10 +1017,14 @@ template , ServerContext& server_context, Fn&& fn, Args&&... args) { InvokeContext& invoke_context = server_context; - std::optional> param; + using ArgType = RemoveCvRef; + std::optional param; const auto& params = server_context.call_context.getParams(); - MaybeReadField(std::integral_constant(), TypeList(), invoke_context, - Make(params), Emplace(param)); + MaybeReadField(std::integral_constant(), TypeList(), invoke_context, + Make(params), ReadDestEmplace(TypeList(), [&](auto&&... args) -> auto& { + param.emplace(std::forward(args)...); + return *param; + })); if (!param) param.emplace(); fn.invoke(server_context, std::forward(args)..., static_cast(*param)); auto&& results = server_context.call_context.getResults(); @@ -1072,7 +1052,7 @@ void CustomBuildField(TypeList<>, } template -void ReadFieldUpdate(TypeList<>, +decltype(auto) CustomReadField(TypeList<>, InvokeContext& invoke_context, Input&& input, typename std::enable_if::value>::type* enable = nullptr) @@ -1086,7 +1066,7 @@ auto PassField(TypeList<>, ServerContext& server_context, const Fn& fn, Args&&.. { const auto& params = server_context.call_context.getParams(); const auto& input = Make(params); - ReadFieldUpdate(TypeList<>(), server_context, input); + ReadField(TypeList<>(), server_context, input); fn.invoke(server_context, std::forward(args)...); auto&& results = server_context.call_context.getResults(); BuildField(TypeList<>(), server_context, Make(results)); @@ -1204,8 +1184,8 @@ struct ClientParam auto callRead(ClientInvokeContext& invoke_context, Results& results, TypeList, Values&&... values) -> typename std::enable_if::type { - MaybeReadFieldUpdate(std::integral_constant(), TypeList...>(), - invoke_context, Make(results), std::forward(values)...); + MaybeReadField(std::integral_constant(), TypeList...>(), invoke_context, + Make(results), ReadDestValue(values)...); } ReadResults(ClientParam* client_param) : m_client_param(client_param) {} diff --git a/include/mp/util.h b/include/mp/util.h index 2f781c49..174cd50a 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -203,7 +203,7 @@ template struct ThrowFn { template - void operator()(Params&&... params) + Exception& operator()(Params&&... params) { throw Exception(std::forward(params)...); } @@ -238,6 +238,10 @@ struct Split, TypeList<_First...>, false> template using ResultOf = decltype(std::declval()()); +//! Substitutue for std::remove_cvref_t +template +using RemoveCvRef = std::remove_cv_t>; + //! Type helper abbreviating std::decay. template using Decay = typename std::decay::type;