[PATCH] D66349: [clangd] Fix one testcase in XRefsTests.
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 19 01:29:48 PDT 2019
hokein 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;
});
----------------
ilya-biryukov wrote:
> 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.
> What are the elements References for the problematic case?
The testcase is using declarations (see the existing test) -- we will get 3 refs on `using ::fo^o`, each ref has a different decl.
```
void [[foo]](int);
void [[foo]](double);
namespace ns {
using ::[[fo^o]];
}
```
> 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.
I think we could make it deterministic, but only return one refs, WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66349/new/
https://reviews.llvm.org/D66349
More information about the cfe-commits
mailing list