[clang] dd870f6 - Fix warning about unused std::unique result, erase shifted elements

[David Blaikie via cfe-commits]

OK - thanks for the context!

On Mon, Nov 18, 2019 at 5:07 PM Ben Langmuir <blangmuir at apple.com> wrote:

> Nice find.
>
> The change LGTM, and is clearly what I always intended it to do.  I was
> confused how this ever worked at all, but I see that (at least in libc++)
> we move the values into place rather than swap them, so the values at the
> end are still the largest values for types with a trivial move constructor.
>
> I am not working in this area these days and am not setup to provide a
> test case in any reasonable amount of time.  I think it would need a gtest,
> as I'm not sure this change is obvservable otherwise.
>
> Ben
>
> On Nov 18, 2019, at 4:44 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
> Hey Ben - if you're still involved with this part of the project - could
> you check that this change (to code you committed back in 2014, r215810) is
> correct & add a test case if possible?
>
> On Thu, Nov 7, 2019 at 10:39 AM Reid Kleckner via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>>
>> Author: Reid Kleckner
>> Date: 2019-11-07T10:39:29-08:00
>> New Revision: dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571.diff
>>
>> LOG: Fix warning about unused std::unique result, erase shifted elements
>>
>> This is actually a functional change. I haven't added any new test
>> coverage. I'm just trying to fix the warning and hoping for the best.
>>
>> Added:
>>
>>
>> Modified:
>>     clang/include/clang/Serialization/ContinuousRangeMap.h
>>
>> Removed:
>>
>>
>>
>>
>> ################################################################################
>> diff  --git a/clang/include/clang/Serialization/ContinuousRangeMap.h
>> b/clang/include/clang/Serialization/ContinuousRangeMap.h
>> index 0c05537dd108..c2665c097416 100644
>> --- a/clang/include/clang/Serialization/ContinuousRangeMap.h
>> +++ b/clang/include/clang/Serialization/ContinuousRangeMap.h
>> @@ -118,14 +118,17 @@ class ContinuousRangeMap {
>>
>>      ~Builder() {
>>        llvm::sort(Self.Rep, Compare());
>> -      std::unique(Self.Rep.begin(), Self.Rep.end(),
>> -                  [](const_reference A, const_reference B) {
>> -        // FIXME: we should not allow any duplicate keys, but there are
>> a lot of
>> -        // duplicate 0 -> 0 mappings to remove first.
>> -        assert((A == B || A.first != B.first) &&
>> -               "ContinuousRangeMap::Builder given non-unique keys");
>> -        return A == B;
>> -      });
>> +      Self.Rep.erase(
>> +          std::unique(
>> +              Self.Rep.begin(), Self.Rep.end(),
>> +              [](const_reference A, const_reference B) {
>> +                // FIXME: we should not allow any duplicate keys, but
>> there are
>> +                // a lot of duplicate 0 -> 0 mappings to remove first.
>> +                assert((A == B || A.first != B.first) &&
>> +                       "ContinuousRangeMap::Builder given non-unique
>> keys");
>> +                return A == B;
>> +              }),
>> +          Self.Rep.end());
>>      }
>>
>>      void insert(const value_type &Val) {
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191118/77601768/attachment.html>