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

Ben Langmuir via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 18 17:07:15 PST 2019


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 <mailto: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 <https://github.com/llvm/llvm-project/commit/dd870f6929ee0b1dfb5fd000c7b826a1bd2d2571>
> DIFF: 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 <mailto:cfe-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <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/c8613830/attachment.html>


More information about the cfe-commits mailing list