[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:389
     llvm::sort(References, [](const Reference &L, const Reference &R) {
-      return std::tie(L.Loc, L.CanonicalTarget, L.Role) <
-             std::tie(R.Loc, R.CanonicalTarget, R.Role);
+      return L.Loc < R.Loc;
     });
----------------
What are the elements `References` for the problematic case?

If we have duplicate elements, then `sort()` would now give us one of the items. Which exact `Decl` we're gonna end up seeing is not well-defined, i.e. it's non-deterministic.


================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:2079
       ExpectedLocations.push_back(RangeIs(R));
+    ASSERT_TRUE(!ExpectedLocations.empty()) << "Testcase must provide ranges!";
     EXPECT_THAT(findReferences(AST, T.point(), 0),
----------------
NIT: use `ASSERT_THAT(ExpectedLocations, Not(IsEmpty()))` for more detailed output in case of assertion failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66349