[PATCH] D136702: [llvm-cov] Look up object files using debuginfod

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:421
+    llvm::sort(ProfileBinaryIDs, Compare);
+    std::unique(ProfileBinaryIDs.begin(), ProfileBinaryIDs.end(), Compare);
+
----------------
mysterymath wrote:
> dblaikie wrote:
> > mysterymath wrote:
> > > jgorbe wrote:
> > > > We're getting build errors here (and the similar call to `std::unique` on line 426) with `-Wunused-result`.
> > > > 
> > > > This looks like a bug: `std::unique` returns the new end of the range, and typically you need to call `container.erase(new_end, container.end())` after the call to `std::unique` to reduce the container size to match the new logical size. See https://en.cppreference.com/w/cpp/algorithm/unique.
> > > Thanks; did a quick fix forward for this. This uncovered a mistake with Compare as well; std::unique requires a == comparator, while sort and set_difference a < comparator.
> > Is there test coverage for these bugs/fixes?
> Partially; the unused result lint caught the issue with std::unique; fixing that caused the original test cases to fail due to the comparator issue.
Could you see about adding a test case that would've failed with the code as originally committed? (something that demonstrates the need for the unique call in the first place)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136702/new/

https://reviews.llvm.org/D136702