From d4ac2d78e2050360eea59b987ac6aa66ba833507 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Fri, 11 Aug 2017 23:52:13 +0200 Subject: [PATCH] C++11: improve moving and forwarding of iterators this becomes more relevant now, since the actual MutationMessage iterators are implemented in terms of a shared_ptr to IterSource. Thus, when building processing pipelines, we most definitively want to move that smart-ptr into the destination, since this avoids touching the shared count and thus avoids generating unnecessary memory barriers. --- doc/technical/code/c++11.txt | 46 ++++++-- src/lib/diff/diff-message.hpp | 9 +- src/lib/diff/gen-node.hpp | 4 +- src/lib/format-util.hpp | 9 +- src/lib/iter-source.hpp | 86 ++++++++------- src/lib/itertools.hpp | 106 +++++++++++++++---- src/proc/mobject/session/placement-index.cpp | 2 +- 7 files changed, 184 insertions(+), 78 deletions(-) diff --git a/doc/technical/code/c++11.txt b/doc/technical/code/c++11.txt index 6b443e195..e504b51c4 100644 --- a/doc/technical/code/c++11.txt +++ b/doc/technical/code/c++11.txt @@ -12,7 +12,7 @@ language and compiler support wasn't ready for what we consider _state of the cr amended deficiencies by rolling our own helper facilities, with a little help from Boost. Thus there was no urge for us to adopt the new language standard; we could simply wait for the compiler support to mature. In spring 2014, finally, we were able to switch our codebase -to C++11 with minimal effort.footnote[since 8/2015 -- after the switch to Debian/Jessie +to C++11 with minimal effort.footnote:[since 8/2015 -- after the switch to Debian/Jessie as a »reference platform«, we even compile with `-std=gnu++14`] Following this switch, we're now able to reap the benefits of this approach; we may now gradually replace our sometimes clunky helpers and workarounds @@ -112,7 +112,9 @@ were thinking since ages; it is more like an official affirmation of that style ******************************************************************************************** The core idea is that at times you need to 'move' a value due to a change of ownership. Now, the explicit support for 'move semantics' allows to decouple this 'conceptual move' from actually -moving memory contents on the raw implementation level. The whole idea behind C++ seems to be +moving memory contents on the raw implementation level. If we use a _rvalue reference on a signature,_ +we express that an entiy is or can be moved on a conceptual level; typically the actual implementation +of this moving is _delegated_ and done by ``someone else''. The whole idea behind C++ seems to be allowing people to think on a conceptual level, while 'retaining' awareness of the gory details below the hood. Such is achieved by 'removing' the need to worry about details, confident that there is a way to deal with those concerns in an orthogonal fashion. @@ -137,6 +139,40 @@ CAUTION: as soon as there is an explicitly defined copy operation, or even just compromise decision of the C++ committee -- instead of either breaking no code at all or boldly breaking code, they settled upon ``somewhat'' breaking existing code... +Perfect forwarding +^^^^^^^^^^^^^^^^^^ +The ``perfect forwarding'' technique is how we actually pass on and delegate move semantics. +In conjunction with variadic templates, this technique promises to obsolete a lot of template trickery +when it comes to implementing custom containers, allocators or similar kinds of managing wrappers. + +.a typical example +[source,c] +-------------------------------------------------------------------------- + template + TY& + create (ARGS&& ...args) + { + return *new(&buf_) TY {std::forward (args)...}; + } +-------------------------------------------------------------------------- +The result is a `create()` function with the ability to _forward_ an arbitrary number of arguments +of arbitrary type to the constructor of type `TY`. Note the following details + +- the template parameter `ARGS&&` leads to deducing the actual parameters _sans_ rvalue reference +- we pass each argument through `std::forward`. This is necessary to overcome the basic limitation + that a rvalue reference can only bind to _something unnamed_. This is a safety feature; moving destroys + the contents at the source location. Thus if we really want to move something known by name (like e.g. + the function arguments in this example), we need to indicate so by an explicit call. +- `std::forward` needs an explicit type parameter to be able to deduce the right target type in any case +- note how we're allowed to ``wrap'' a function call around the unpacking of the _argument pack_ (`args`): + the three dots `...` are _outside_ the parenthesis, and thus the `std::forward` is applied to each of + the arguments individually + +NOTE: forwarding calls can be chained, but at some point you get to acutally _consuming_ the value passed through. + To support the maximum flexibility at this point, you typically need to write two flavours of the receiving + function or constructor: one version taking a rvalue reference, and one version taking `const&`. Moving is + _destructive_, while the `const&` variant deals with all those cases where we copy without affecting the + source object. @@ -159,13 +195,11 @@ September 2014:: requirement level_ soon. August 2015:: our »reference system« (platform) is Debian/Jessie from now on. - We have switched to **C++14** and use (even require) GCC-4.9 or CLang 3.5 -- we can expect solid support - for all C++11 features and most C++14 features. + We have switched to **C\+\+14** and use (even require) GCC-4.9 or CLang 3.5 -- we can expect solid support + for all C\+\+11 features and most C++14 features. Perfect forwarding ~~~~~~~~~~~~~~~~~~ -The ``perfect forwarding'' technique in conjunction with variadic templates promises to obsolete a lot of -template trickery when it comes to implementing custom containers, allocators or similar kinds of managing wrappers. Unfortunately, we ran into nasty problems with both GCC-4.7 and CLang 3.0 here, when chaining several forwarding calls. - the new _reference collapsing rules_ seem to be unreliably still. Note that even the standard library uses an diff --git a/src/lib/diff/diff-message.hpp b/src/lib/diff/diff-message.hpp index 4a2305f6d..640ed154e 100644 --- a/src/lib/diff/diff-message.hpp +++ b/src/lib/diff/diff-message.hpp @@ -97,6 +97,7 @@ namespace diff{ DiffMessage() = default; + // default copy operations acceptable /** * DiffMessage builder: @@ -132,8 +133,8 @@ namespace diff{ * @note source iterator is copied into a heap allocated IterSource */ template - DiffMessage(IT ii, enable_if< can_IterForEach, void*> =nullptr) - : _FrontEnd{iter_source::wrapIter(ii)} + DiffMessage(IT&& ii, enable_if< can_IterForEach, void*> =nullptr) + : _FrontEnd{iter_source::wrapIter (std::forward(ii))} { } /** @@ -142,8 +143,8 @@ namespace diff{ * @warning like with any classical iterators, the container must stay alive and accessible */ template - DiffMessage(CON& container, enable_if< __and_< can_STL_ForEach - ,__not_>>, void*> =nullptr) + DiffMessage(CON& container, enable_if< __and_< can_STL_ForEach + ,__not_>>, void*> =nullptr) : _FrontEnd{iter_source::eachEntry(container)} { } }; diff --git a/src/lib/diff/gen-node.hpp b/src/lib/diff/gen-node.hpp index 19e3abcd5..6cc1f5cb7 100644 --- a/src/lib/diff/gen-node.hpp +++ b/src/lib/diff/gen-node.hpp @@ -381,9 +381,9 @@ namespace diff{ } friend ChildDataIter - childData (Rec::scopeIter const& scopeIter) + childData (Rec::scopeIter&& scopeIter) { - return ChildDataIter{ scopeIter + return ChildDataIter{ std::forward(scopeIter) , [](GenNode const& child) ->DataCap const& { return child.data; diff --git a/src/lib/format-util.hpp b/src/lib/format-util.hpp index 80b17dd15..f9d8ac90f 100644 --- a/src/lib/format-util.hpp +++ b/src/lib/format-util.hpp @@ -55,6 +55,7 @@ namespace util { using lib::meta::can_IterForEach; using std::string; + using std::forward; using std::move; @@ -127,12 +128,12 @@ namespace util { * @see FormatHelper_test::checkStringify() */ template - inline lib::TransformIter - stringify (IT const& src) + inline auto + stringify (IT&& src) { - using Val = typename IT::value_type; + using Val = typename std::remove_reference::type::value_type; - return lib::transformIterator(src, util::toString); + return lib::transformIterator(forward(src), util::toString); } diff --git a/src/lib/iter-source.hpp b/src/lib/iter-source.hpp index 49178ac52..bacc0835e 100644 --- a/src/lib/iter-source.hpp +++ b/src/lib/iter-source.hpp @@ -52,8 +52,9 @@ #include "lib/iter-adapter.hpp" #include "lib/itertools.hpp" -#include #include +#include +#include #include #include @@ -64,6 +65,7 @@ namespace lib { using std::string; using std::shared_ptr; + using std::forward; @@ -255,6 +257,10 @@ namespace lib { WrappedLumieraIter (IT const& orig) : src_(orig) { } + + WrappedLumieraIter (IT&& orig) + : src_(forward(orig)) + { } }; @@ -268,55 +274,58 @@ namespace lib { template struct _SeqT { - typedef typename CON::iterator::value_type Val; - typedef typename IterSource::iterator Iter; + using Val = typename CON::iterator::value_type; + using Iter = typename IterSource::iterator; }; template struct _RangeT { - typedef typename IT::value_type Val; - typedef typename IterSource::iterator Iter; + using Val = typename IT::value_type; + using Iter = typename IterSource::iterator; }; template struct _MapT { - typedef typename MAP::key_type Key; - typedef typename MAP::value_type::second_type Val; - typedef typename IterSource::iterator KeyIter; - typedef typename IterSource::iterator ValIter; + using Key = typename MAP::key_type; + using Val = typename MAP::value_type::second_type; + using KeyIter = typename IterSource::iterator; + using ValIter = typename IterSource::iterator; }; template struct _IterT { - typedef typename IT::value_type Val; - typedef typename IterSource::iterator Iter; + using Src = typename std::remove_reference::type; + using Val = typename Src::value_type; + using Iter = typename IterSource::iterator; }; template struct _TransformIterT { - typedef typename lib::meta::_Fun::Ret ResVal; - typedef TransformIter TransIter; - typedef typename IterSource::iterator Iter; + using Src = typename std::remove_reference::type; + using ResVal = typename lib::meta::_Fun::Ret; + using TransIter = TransformIter; + using Iter = typename IterSource::iterator; }; template struct _PairIterT { - typedef typename IT::value_type PairType; - typedef typename PairType::second_type ValType; - typedef typename PairType::first_type ConstKeyType; + using Src = typename std::remove_reference::type; + using PairType = typename Src::value_type; + using ValType = typename PairType::second_type; + using ConstKeyType = typename PairType::first_type; // since we're returning the keys always by value, // we can strip the const added by the STL map types - typedef typename boost::remove_const::type KeyType; + using KeyType = typename std::remove_const::type; - typedef TransformIter KeyIter; - typedef TransformIter ValIter; + typedef TransformIter KeyIter; + typedef TransformIter ValIter; static KeyType takeFirst (PairType const& pair) { return pair.first; } static ValType takeSecond(PairType const& pair) { return pair.second;} @@ -325,16 +334,16 @@ namespace lib { template typename _PairIterT::KeyIter - takePairFirst (IT const& source) + takePairFirst (IT&& source) { - return transformIterator(source, _PairIterT::takeFirst ); + return transformIterator(forward(source), _PairIterT::takeFirst ); } template typename _PairIterT::ValIter - takePairSecond (IT const& source) + takePairSecond (IT&& source) { - return transformIterator(source, _PairIterT::takeSecond ); + return transformIterator(forward(source), _PairIterT::takeSecond ); } } //(END) type helpers... @@ -346,11 +355,12 @@ namespace lib { */ template typename _IterT::Iter - wrapIter (IT const& source) + wrapIter (IT&& source) { - typedef typename _IterT::Val ValType; + using Src = typename _IterT::Src; + using Val = typename _IterT::Val; - return IterSource::build (new WrappedLumieraIter (source)); + return IterSource::build (new WrappedLumieraIter (forward(source))); } @@ -365,14 +375,14 @@ namespace lib { */ template typename _TransformIterT::Iter - transform (IT const& source, FUN processingFunc) + transform (IT&& source, FUN processingFunc) { typedef typename _TransformIterT::ResVal ValType; typedef typename _TransformIterT::TransIter TransIT; return IterSource::build ( new WrappedLumieraIter ( - transformIterator (source, processingFunc))); + transformIterator (forward(source), processingFunc))); } @@ -383,7 +393,7 @@ namespace lib { typename _MapT::KeyIter eachMapKey (MAP& map) { - typedef RangeIter Range; + using Range = RangeIter; Range contents (map.begin(), map.end()); return wrapIter (takePairFirst (contents)); @@ -397,7 +407,7 @@ namespace lib { typename _MapT::ValIter eachMapVal (MAP& map) { - typedef RangeIter Range; + using Range = RangeIter; Range contents (map.begin(), map.end()); return wrapIter (takePairSecond(contents)); @@ -413,7 +423,7 @@ namespace lib { typename _MapT::KeyIter eachDistinctKey (MAP& map) { - typedef RangeIter Range; + using Range = RangeIter; Range contents (map.begin(), map.end()); return wrapIter (filterRepetitions (takePairFirst(contents))); @@ -428,8 +438,8 @@ namespace lib { typename _MapT::ValIter eachValForKey (MAP& map, typename _MapT::Key key) { - typedef typename MAP::iterator Pos; - typedef RangeIter Range; + using Pos = typename MAP::iterator; + using Range = RangeIter; std::pair valuesForKey = map.equal_range(key); Range contents (valuesForKey.first, valuesForKey.second); @@ -447,8 +457,8 @@ namespace lib { typename _SeqT::Iter eachEntry (CON& container) { - typedef typename _SeqT::Val ValType; - typedef RangeIter Range; + using ValType = typename _SeqT::Val; + using Range = RangeIter; Range contents (container.begin(), container.end()); return IterSource::build (new WrappedLumieraIter(contents)); @@ -462,8 +472,8 @@ namespace lib { typename _RangeT::Iter eachEntry (IT const& begin, IT const& end) { - typedef typename _RangeT::Val ValType; - typedef RangeIter Range; + using ValType = typename _RangeT::Val; + using Range = RangeIter; Range contents (begin, end); return IterSource::build (new WrappedLumieraIter(contents)); diff --git a/src/lib/itertools.hpp b/src/lib/itertools.hpp index 1b08009ae..dcd37c300 100644 --- a/src/lib/itertools.hpp +++ b/src/lib/itertools.hpp @@ -74,11 +74,13 @@ #include "lib/util.hpp" #include +#include namespace lib { + using std::forward; using std::function; using util::unConst; @@ -102,8 +104,11 @@ namespace lib { { IT source_; + IdentityCore (IT&& orig) + : source_{forward(orig)} + { } IdentityCore (IT const& orig) - : source_(orig) + : source_{orig} { } IT& @@ -184,8 +189,8 @@ namespace lib { typedef typename CORE::value_type value_type; - IterTool (CORE const& setup) - : core_(setup) + IterTool (CORE&& setup) + : core_{std::move(setup)} { hasData(); } @@ -268,8 +273,8 @@ namespace lib { struct FilterCore : IdentityCore { - typedef IdentityCore Raw; - typedef typename IT::reference Val; + using Raw = IdentityCore; + using Val = typename IT::reference; function predicate_; @@ -302,12 +307,20 @@ namespace lib { template - FilterCore (IT const& source, PRED prediDef) - : Raw(source) + FilterCore (IT&& source, PRED prediDef) + : Raw{forward(source)} , predicate_(prediDef) // induces a signature check , cached_(false) // not yet cached , isOK_() // some value { } + + template + FilterCore (IT const& source, PRED prediDef) + : Raw{source} + , predicate_(prediDef) + , cached_(false) + , isOK_() + { } }; @@ -326,12 +339,17 @@ namespace lib { FilterIter () - : _Impl(FilterCore(IT(), acceptAll)) + : _Impl{FilterCore(IT(), acceptAll)} { } template FilterIter (IT const& src, PRED filterPredicate) - : _Impl(_Filter(src,filterPredicate)) + : _Impl{_Filter(src, filterPredicate)} + { } + + template + FilterIter (IT&& src, PRED filterPredicate) + : _Impl{_Filter(forward(src), filterPredicate)} { } ENABLE_USE_IN_STD_RANGE_FOR_LOOPS (FilterIter) @@ -344,10 +362,18 @@ namespace lib { * @return Iterator filtering contents of the source */ template - inline FilterIter + inline auto filterIterator (IT const& src, PRED filterPredicate) { - return FilterIter(src,filterPredicate); + return FilterIter{src, filterPredicate}; + } + + template + inline auto + filterIterator (IT&& src, PRED filterPredicate) + { + using SrcIT = typename std::remove_reference::type; + return FilterIter{forward(src), filterPredicate}; } @@ -378,8 +404,8 @@ namespace lib { class ExtensibleFilterIter : public FilterIter { - typedef FilterCore _Filter; - typedef typename _Filter::Val Val; + using _Filter = FilterCore; + using Val = typename _Filter::Val; void reEvaluate() @@ -391,13 +417,17 @@ namespace lib { public: ExtensibleFilterIter() { } + template + ExtensibleFilterIter (IT&& src, PRED initialFilterPredicate) + : FilterIter{forward(src), initialFilterPredicate} + { } template ExtensibleFilterIter (IT const& src, PRED initialFilterPredicate) - : FilterIter(src, initialFilterPredicate) + : FilterIter{src, initialFilterPredicate} { } - ExtensibleFilterIter (IT const& src) - : ExtensibleFilterIter(src, FilterIter::acceptAll) + ExtensibleFilterIter (IT&& src) + : ExtensibleFilterIter{forward(src), FilterIter::acceptAll} { } // standard copy operations acceptable @@ -570,6 +600,14 @@ namespace lib { , treated_() { } + template + TransformingCore (IT&& orig, FUN processor) + : trafo_(processor) // induces a signature check + , source_(forward (orig)) + { + processItem(); + } + template TransformingCore (IT const& orig, FUN processor) : trafo_(processor) // induces a signature check @@ -612,8 +650,8 @@ namespace lib { class TransformIter : public IterTool> { - typedef TransformingCore _Trafo; - typedef IterTool<_Trafo> _IteratorImpl; + using _Trafo = TransformingCore; + using _IteratorImpl = IterTool<_Trafo> ; public: TransformIter () @@ -621,8 +659,12 @@ namespace lib { { } template + TransformIter (IT&& src, FUN trafoFunc) + : _IteratorImpl{_Trafo(forward(src), trafoFunc)} + { } + template TransformIter (IT const& src, FUN trafoFunc) - : _IteratorImpl(_Trafo(src,trafoFunc)) + : _IteratorImpl{_Trafo(src, trafoFunc)} { } ENABLE_USE_IN_STD_RANGE_FOR_LOOPS (TransformIter) @@ -642,7 +684,16 @@ namespace lib { transformIterator (IT const& src, FUN processingFunc) { using OutVal = typename lib::meta::_Fun::Ret; - return TransformIter(src,processingFunc); + return TransformIter{src,processingFunc}; + } + + template + inline auto + transformIterator (IT&& src, FUN processingFunc) + { + using SrcIT = typename std::remove_reference::type; + using OutVal = typename lib::meta::_Fun::Ret; + return TransformIter{forward(src), processingFunc}; } @@ -687,11 +738,20 @@ namespace lib { /** filters away repeated values * emitted by source iterator */ template - inline FilterIter + inline auto filterRepetitions (IT const& source) { - typedef typename IT::value_type Val; - return filterIterator(source, SkipRepetition() ); + using Val = typename IT::value_type; + return filterIterator (source, SkipRepetition()); + } + + template + inline auto + filterRepetitions (IT&& source) + { + using SrcIT = typename std::remove_reference::type; + using Val = typename SrcIT::value_type; + return filterIterator (forward(source), SkipRepetition() ); } diff --git a/src/proc/mobject/session/placement-index.cpp b/src/proc/mobject/session/placement-index.cpp index ab32b3df3..3556f2beb 100644 --- a/src/proc/mobject/session/placement-index.cpp +++ b/src/proc/mobject/session/placement-index.cpp @@ -309,7 +309,7 @@ namespace session { /* == access for self-test == */ - typedef lib::IterSource::iterator IDIter; + using IDIter = lib::IterSource::iterator; PlacementMO* _root_4check () { return root_.get(); } PlacementMO* _element_4check (ID id){ return base_entry(id).element.get();}