<div dir="ltr">Looked pretty good at a quick glance - thanks!</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 25, 2016 at 4:30 PM, Mehdi Amini <span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On Mar 25, 2016, at 10:05 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><br><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Fri, Mar 25, 2016 at 9:36 AM, Mehdi Amini via llvm-commits<span> </span><span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: mehdi_amini<br>Date: Fri Mar 25 11:36:00 2016<br>New Revision: 264418<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=264418&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=264418&view=rev</a><br>Log:<br>Improve StringMap unittests: reintroduce move count, but shield against std::pair internals<br><br>From: Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>><br><br>Modified:<br>   <span> </span>llvm/trunk/unittests/ADT/StringMapTest.cpp<br><br>Modified: llvm/trunk/unittests/ADT/StringMapTest.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringMapTest.cpp?rev=264418&r1=264417&r2=264418&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringMapTest.cpp?rev=264418&r1=264417&r2=264418&view=diff</a><br>==============================================================================<br>--- llvm/trunk/unittests/ADT/StringMapTest.cpp (original)<br>+++ llvm/trunk/unittests/ADT/StringMapTest.cpp Fri Mar 25 11:36:00 2016<br>@@ -391,14 +391,21 @@ TEST(StringMapCustomTest, InitialSizeTes<br>   for (auto Size : {1, 32, 67}) {<br>     StringMap<CountCtorCopyAndMove> Map(Size);<br>     auto NumBuckets = Map.getNumBuckets();<br>+<br>+    // Prepare the elts in a vector. We do this as a pre-step to shield us<br>+    // against the internals of std::pair which can introduce spurious move/copy<br>+    std::vector<std::pair<std::string, CountCtorCopyAndMove>> Elts;<br>+    for (int i = 0; i < Size; ++i)<br>+      Elts.emplace_back(Twine(i).str(), CountCtorCopyAndMove());<br>+<br>     CountCtorCopyAndMove::Move = 0;<br>     CountCtorCopyAndMove::Copy = 0;<br>     for (int i = 0; i < Size; ++i)<br>-      Map.insert(std::make_pair(Twine(i).str(), CountCtorCopyAndMove()));<br>-    //  This relies on move-construction elision, and cannot be reliably tested.<br>-    //   EXPECT_EQ((unsigned)Size * 3, CountCtorCopyAndMove::Move);<br>-    // No copy is expected.<br>-    EXPECT_EQ(0u, CountCtorCopyAndMove::Copy);<br>+      Map.insert(Elts[i]);<br></blockquote><div><br>TBH I'd probably favor the piecewise_construct option - seems (to me) to be a more direct representation of what's required, avoids extra storage, complexity, etc, no?<br></div></div></div></blockquote><div><br></div></div></div><div>Have a look at r264475 and let me know!</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-- </div><div>Mehdi</div></font></span><span class=""><div><br></div><div><br></div><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">+    // After the inital copy, the map will move the Elts in the Entry.<br>+    EXPECT_EQ((unsigned)Size, CountCtorCopyAndMove::Move);<br>+    // We copy once the pair from the Elts vector<br>+    EXPECT_EQ((unsigned)Size, CountCtorCopyAndMove::Copy);<br>     // Check that the map didn't grow<br>     EXPECT_EQ(Map.getNumBuckets(), NumBuckets);<br>   }<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></blockquote></div></div></blockquote></span></div><br></div></blockquote></div><br></div>