[llvm] r264418 - Improve StringMap unittests: reintroduce move count, but shield against std::pair internals

[Mehdi Amini via llvm-commits]

> On Mar 25, 2016, at 10:05 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Fri, Mar 25, 2016 at 9:36 AM, Mehdi Amini via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Author: mehdi_amini
> Date: Fri Mar 25 11:36:00 2016
> New Revision: 264418
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=264418&view=rev <http://llvm.org/viewvc/llvm-project?rev=264418&view=rev>
> Log:
> Improve StringMap unittests: reintroduce move count, but shield against std::pair internals
> 
> From: Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>>
> 
> Modified:
>     llvm/trunk/unittests/ADT/StringMapTest.cpp
> 
> Modified: llvm/trunk/unittests/ADT/StringMapTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringMapTest.cpp?rev=264418&r1=264417&r2=264418&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringMapTest.cpp?rev=264418&r1=264417&r2=264418&view=diff>
> ==============================================================================
> --- llvm/trunk/unittests/ADT/StringMapTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/StringMapTest.cpp Fri Mar 25 11:36:00 2016
> @@ -391,14 +391,21 @@ TEST(StringMapCustomTest, InitialSizeTes
>    for (auto Size : {1, 32, 67}) {
>      StringMap<CountCtorCopyAndMove> Map(Size);
>      auto NumBuckets = Map.getNumBuckets();
> +
> +    // Prepare the elts in a vector. We do this as a pre-step to shield us
> +    // against the internals of std::pair which can introduce spurious move/copy
> +    std::vector<std::pair<std::string, CountCtorCopyAndMove>> Elts;
> +    for (int i = 0; i < Size; ++i)
> +      Elts.emplace_back(Twine(i).str(), CountCtorCopyAndMove());
> +
>      CountCtorCopyAndMove::Move = 0;
>      CountCtorCopyAndMove::Copy = 0;
>      for (int i = 0; i < Size; ++i)
> -      Map.insert(std::make_pair(Twine(i).str(), CountCtorCopyAndMove()));
> -    //  This relies on move-construction elision, and cannot be reliably tested.
> -    //   EXPECT_EQ((unsigned)Size * 3, CountCtorCopyAndMove::Move);
> -    // No copy is expected.
> -    EXPECT_EQ(0u, CountCtorCopyAndMove::Copy);
> +      Map.insert(Elts[i]);
> 
> 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?

Have a look at r264475 and let me know!

-- 
Mehdi



>  
> +    // After the inital copy, the map will move the Elts in the Entry.
> +    EXPECT_EQ((unsigned)Size, CountCtorCopyAndMove::Move);
> +    // We copy once the pair from the Elts vector
> +    EXPECT_EQ((unsigned)Size, CountCtorCopyAndMove::Copy);
>      // Check that the map didn't grow
>      EXPECT_EQ(Map.getNumBuckets(), NumBuckets);
>    }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160325/32c5e84a/attachment-0001.html>