benn/lumiera_
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Library: investigate and fix an insidious problem with move-forwarding (util::join / transformIter)

Browse source 
As it turned out, we had two bugs luring in the code base,
with the happy result of one cancelling out the adverse effects of the other

:-D

 - a mistake in the invocation of the Itertools (transform, filter,...)
   caused them to move and consume any input passed by forwarding, instead
   of consuming only the RValue references.
 - but util::join did an extraneous copy on its data source, meaning that
   in all relevant cases where a *copy* got passed into the Itertools,
   only that spurious temporary was consumed by Bug #1.

(Note that most usages of Itertools rely on RValues anyway, since the whole
point of Itertools is to write concise in-line transformation pipelines...)

*** Added additional testcode to prove util::stringify() behaves correct
    now in all cases.
This commit is contained in:
Fischlurch 2017-12-02 04:06:11 +01:00
parent 63a49bccfd
commit 1df77cc4ff
5 changed files with 123 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
doc/technical/code/c++11.txt
View file

@ -163,7 +163,11 @@ of arbitrary type to the constructor of type `TY`. Note the following details
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
- `std::forward` needs an explicit type parameter to be able to deduce the right target type in any case.
It is _crucial to pass this type parameter in the correct way,_ as demonstrated above. Because, if we
invoke this function for _copying_ (i.e. with a lvalue reference), this type parameter will _carry on_
this refrerence. Only a rvalue reference is stripped. This is the only way ``downstream'' receivers can
tell the copy or move case apart.
- 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

3
src/lib/format-util.hpp
View file

@ -110,6 +110,7 @@ namespace util {
/** convert a sequence of elements to string
* @param elms sequence of arbitrary elements
* @tparam CON the container type to collect the results
* @return a collection of type CON, initialised by the
* string representation of the given elements
*/
@ -192,7 +193,7 @@ namespace util {
using Coll = typename lib::meta::Strip<CON>::TypePlain;
_RangeIter<Coll> range(std::forward<CON>(coll));
auto strings = stringify (range.iter);
auto strings = stringify (std::move (range.iter));
if (!strings) return "";
std::ostringstream buffer;

6
src/lib/itertools.hpp
View file

@ -383,7 +383,7 @@ namespace lib {
filterIterator (IT&& src, PRED filterPredicate)
{
using SrcIT = typename std::remove_reference<IT>::type;
return FilterIter<SrcIT>{forward<SrcIT>(src), filterPredicate};
return FilterIter<SrcIT>{forward<IT>(src), filterPredicate};
}
@ -799,7 +799,7 @@ namespace lib {
{
using SrcIT = typename std::remove_reference<IT>::type;
using OutVal = typename lib::meta::_Fun<FUN>::Ret;
return TransformIter<SrcIT,OutVal>{forward<SrcIT>(src), processingFunc};
return TransformIter<SrcIT,OutVal>{forward<IT>(src), processingFunc};
}
@ -857,7 +857,7 @@ namespace lib {
{
using SrcIT = typename std::remove_reference<IT>::type;
using Val = typename SrcIT::value_type;
return filterIterator (forward<SrcIT>(source), SkipRepetition<Val>() );
return filterIterator (forward<IT>(source), SkipRepetition<Val>() );
}

25
tests/library/format-helper-test.cpp
View file

@ -36,6 +36,7 @@
using lib::transformIterator;
using lib::iter_stl::snapshot;
using lib::iter_stl::eachElm;
using lib::eachNum;
using util::_Fmt;
@ -157,6 +158,30 @@ namespace test {
// another variant: collect arbitrary number of arguments
VecS vals = stringify<VecS> (short(12), 345L, "67", '8');
CHECK (vals == VecS({"12", "345", "67", "8"}));
// stringify can both consume (RValue-Ref) or take a copy from its source
auto nn = snapshot (eachNum (5, 10));
CHECK (5 == *nn);
++nn;
CHECK (6 == *nn);
auto sn = stringify (nn);
CHECK ("6" == *sn);
++sn;
CHECK ("7" == *sn);
CHECK (6 == *nn);
++++nn;
CHECK (8 == *nn);
CHECK ("7" == *sn);
sn = stringify (std::move(nn));
CHECK ("8" == *sn);
CHECK (isnil (nn)); // was consumed by moving it into sn
++sn;
CHECK ("9" == *sn);
++sn;
CHECK (isnil (sn));
}

88
wiki/thinkPad.ichthyo.mm
View file

@ -5419,6 +5419,22 @@
<icon BUILTIN="forward"/>
</node>
</node>
<node COLOR="#338800" CREATED="1512179383254" FOLDED="true" ID="ID_1554412479" MODIFIED="1512326315343" TEXT="mu&#xdf; value_type from Funktor gewinnen">
<icon BUILTIN="button_ok"/>
<node CREATED="1512181398331" ID="ID_1715550480" MODIFIED="1512181407142" TEXT="value_type rebinden"/>
<node CREATED="1512181407778" ID="ID_782984508" MODIFIED="1512181425706" TEXT="generischen Helper von IterAdapter">
<icon BUILTIN="idea"/>
</node>
<node COLOR="#338800" CREATED="1512181416881" ID="ID_229445720" MODIFIED="1512181423185" TEXT="verallgemeinern, rund machen">
<icon BUILTIN="button_ok"/>
</node>
<node COLOR="#338800" CREATED="1512183524170" ID="ID_1697656461" MODIFIED="1512326304993" TEXT="TODO: isStringLike in meta/traits.hpp">
<icon BUILTIN="button_ok"/>
<node CREATED="1512183546319" ID="ID_1699019563" MODIFIED="1512183561888" TEXT="warum geht &quot;basically&quot; bis auf den Pointer?"/>
<node CREATED="1512183562621" ID="ID_57742709" MODIFIED="1512183571160" TEXT="logisch w&#xe4;re nur bis zur Referenz"/>
<node CREATED="1512183572244" ID="ID_1399145205" MODIFIED="1512183587645" TEXT="denn: string* ist nix String-artiges!!!"/>
</node>
</node>
</node>
</node>
</node>
@ -5530,6 +5546,78 @@
</node>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1511835691963" ID="ID_175353270" MODIFIED="1511835736414" TEXT="Transform: generic Lambda">
<icon BUILTIN="flag-yellow"/>
<node COLOR="#338800" CREATED="1512181480616" FOLDED="true" ID="ID_1099744034" MODIFIED="1512349511547" TEXT="Beobachtung: move in join">
<icon BUILTIN="button_ok"/>
<node CREATED="1512181493551" ID="ID_1834475937" MODIFIED="1512181507297" TEXT="Bei &#xdc;bergabe in stringify() fehlt std::forward"/>
<node CREATED="1512181508053" ID="ID_1058623354" MODIFIED="1512181516343" TEXT="transformIterator strippt die Referenz"/>
<node CREATED="1512181517955" ID="ID_517823516" MODIFIED="1512181526366" TEXT="aus LValue wird RValue"/>
<node CREATED="1512181527834" ID="ID_1446020077" MODIFIED="1512181535898" TEXT="Peng">
<icon BUILTIN="clanbomber"/>
</node>
<node CREATED="1512181547263" ID="ID_1700050621" MODIFIED="1512349487204" TEXT="Aufkl&#xe4;ren: warum wird Referenz gestrippt">
<font ITALIC="true" NAME="SansSerif" SIZE="12"/>
<icon BUILTIN="button_ok"/>
<node CREATED="1512349360360" ID="ID_1400970540" MODIFIED="1512349365789" TEXT="vmtl Programmierfehler"/>
<node CREATED="1512349367230" ID="ID_1844975610" MODIFIED="1512349379887" TEXT="IterTool braucht den Typ des Basis-Iterators"/>
<node CREATED="1512349380643" ID="ID_1515581078" MODIFIED="1512349403999">
<richcontent TYPE="NODE"><html>
<head>
</head>
<body>
<p>
versehentlich wurde <i>auch der</i>&#160;an std::forward gegeben
</p>
</body>
</html>
</richcontent>
</node>
<node CREATED="1512349405280" ID="ID_1121668073" MODIFIED="1512349446256" TEXT="war nachweislich eine echte Fehlfunktion">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
habs mit FormatUtils_test bewiesen
</p>
<p>
Dazu in NumIter einen explizit tracenden move-ctor eingebaut
</p>
</body>
</html>
</richcontent>
</node>
</node>
<node CREATED="1512181592137" ID="ID_1472433606" MODIFIED="1512181641886" TEXT="im Ergebnis folgenlos, aber gef&#xe4;hrlich">
<richcontent TYPE="NOTE"><html>
<head>
</head>
<body>
<p>
weil der Aufruf von join(&amp;&amp;) selber wasserdicht ist
</p>
<p>
D.h. er frisst keine Werte.
</p>
<p>
Deshalb f&#228;llt dieses doppelte Problem nicht auf
</p>
</body>
</html>
</richcontent>
<icon BUILTIN="info"/>
<node CREATED="1512349447978" ID="ID_1467669966" MODIFIED="1512349453141" TEXT="und zwar wegen util::join"/>
<node CREATED="1512349454562" ID="ID_1337232290" MODIFIED="1512349470555" TEXT="dieses hat --versehentlich-- eine Kopie zu viel gemacht"/>
<node CREATED="1512349471143" ID="ID_540600531" MODIFIED="1512349476953" TEXT="Hurrgha!!!!!">
<icon BUILTIN="ksmiletris"/>
</node>
</node>
<node COLOR="#338800" CREATED="1512349501091" ID="ID_1425293642" MODIFIED="1512349506090" TEXT="beide defekte gefixt">
<icon BUILTIN="button_ok"/>
</node>
</node>
</node>
<node BACKGROUND_COLOR="#eee5c3" COLOR="#990000" CREATED="1511835901999" ID="ID_1717235881" MODIFIED="1511836238870" TEXT="Transform: Core&amp; -&gt; irgendwas">
<icon BUILTIN="help"/>