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

[David Blaikie via llvm-commits]

Looked pretty good at a quick glance - thanks!

On Fri, Mar 25, 2016 at 4:30 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> 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> 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
>> Log:
>> Improve StringMap unittests: reintroduce move count, but shield against
>> std::pair internals
>>
>> From: Mehdi Amini <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
>>
>> ==============================================================================
>> --- 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
>> 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/e4ad494b/attachment.html>