OK - thanks for the context! On Mon, Nov 18, 2019 at 5:07 PM Ben Langmuir <blangm...@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 <dblai...@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@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@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
