[APINotes] Avoid assertion failure with expensive checks

[bjope]

Found assertion failures when using EXPENSIVE_CHECKS and running lit tests for APINotes:
Assertion `left.first != right.first && "two entries for the same version"' failed.

It seems like std::is_sorted is verifying that the comparison function is reflective (comp(a,a)=false) when using expensive checks. So we would get callbacks to the lambda used for comparison, even for vectors with a single element in APINotesReader::VersionedInfo::VersionedInfo, with "left" and "right" being the same object. Therefore the assert checking that we never found equal values would fail.

Fix makes sure that we skip the check for equal values when "left" and "right" is the same object.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Björn Pettersson (bjope)

Changes

Found assertion failures when using EXPENSIVE_CHECKS and running lit tests for APINotes:
Assertion `left.first != right.first && "two entries for the same version"' failed.

It seems like std::is_sorted is verifying that the comparison function is reflective (comp(a,a)=false) when using expensive checks. So we would get callbacks to the lambda used for comparison, even for vectors with a single element in APINotesReader::VersionedInfo<T>::VersionedInfo, with "left" and "right" being the same object. Therefore the assert checking that we never found equal values would fail.

Fix makes sure that we skip the check for equal values when "left" and "right" is the same object.

---

Full diff: https://github.com/llvm/llvm-project/pull/120487.diff

1 Files Affected:

• (modified) clang/lib/APINotes/APINotesReader.cpp (+6-1)

diff --git a/clang/lib/APINotes/APINotesReader.cpp b/clang/lib/APINotes/APINotesReader.cpp
index fa06dffdd14b03..646eabd2a5ecd3 100644
--- a/clang/lib/APINotes/APINotesReader.cpp
+++ b/clang/lib/APINotes/APINotesReader.cpp
@@ -2045,7 +2045,12 @@ APINotesReader::VersionedInfo<T>::VersionedInfo(
       Results.begin(), Results.end(),
       [](const std::pair<llvm::VersionTuple, T> &left,
          const std::pair<llvm::VersionTuple, T> &right) -> bool {
-        assert(left.first != right.first && "two entries for the same version");
+        // The comparison function should be reflective, and with expensive
+        // checks we can get callbacks basically checking that lambda(a,a) is
+        // false. We could still check that we do not find equal elements when
+        // left!=right.
+        assert((&left == &right || left.first != right.first) &&
+               "two entries for the same version");
         return left.first < right.first;
       }));
 

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 5ca3794e82bd4d96e5aa32821bed033e40f51814 b3d95e794735c3f8242643c3d187c24a1ad51421 --extensions cpp -- clang/lib/APINotes/APINotesReader.cpp

View the diff from clang-format here.

diff --git a/clang/lib/APINotes/APINotesReader.cpp b/clang/lib/APINotes/APINotesReader.cpp
index 646eabd2a5..e70146467f 100644
--- a/clang/lib/APINotes/APINotesReader.cpp
+++ b/clang/lib/APINotes/APINotesReader.cpp
@@ -2041,18 +2041,19 @@ APINotesReader::VersionedInfo<T>::VersionedInfo(
     : Results(std::move(R)) {
 
   assert(!Results.empty());
-  assert(std::is_sorted(
-      Results.begin(), Results.end(),
-      [](const std::pair<llvm::VersionTuple, T> &left,
-         const std::pair<llvm::VersionTuple, T> &right) -> bool {
-        // The comparison function should be reflective, and with expensive
-        // checks we can get callbacks basically checking that lambda(a,a) is
-        // false. We could still check that we do not find equal elements when
-        // left!=right.
-        assert((&left == &right || left.first != right.first) &&
-               "two entries for the same version");
-        return left.first < right.first;
-      }));
+  assert(
+      std::is_sorted(Results.begin(), Results.end(),
+                     [](const std::pair<llvm::VersionTuple, T> &left,
+                        const std::pair<llvm::VersionTuple, T> &right) -> bool {
+                       // The comparison function should be reflective, and with
+                       // expensive checks we can get callbacks basically
+                       // checking that lambda(a,a) is false. We could still
+                       // check that we do not find equal elements when
+                       // left!=right.
+                       assert((&left == &right || left.first != right.first) &&
+                              "two entries for the same version");
+                       return left.first < right.first;
+                     }));
 
   Selected = std::nullopt;
   for (unsigned i = 0, n = Results.size(); i != n; ++i) {