ilya-biryukov added inline comments.
Herald added a subscriber: kadircet.
================
Comment at: clangd/XRefs.cpp:105
+ // Sort results. Declarations being referenced explicitly come first.
+ std::sort(Result.begin(), Result.end(),
+ [](const DeclInfo &L, const DeclInfo &R) {
----------------
ilya-biryukov wrote:
> Maybe sort further at the callers instead?
> It would be a more intrusive change, but would also give a well-defined
> result order for findDefinitions and other cases. E.g. findDefinitions
> currently gives results in an arbitrary order (specifically, the one supplied
> by DenseMap+sort) when multiple decls are returned.
> WDYT?
Just to clarify the original suggestion.
Maybe we can sort the `(location, is_explicit)` pairs instead of the `(decl,
is_explicit)` pairs?
Sorting based on pointer equality (see `L.D < R.D`) provides non-deterministic
results and we can have fully deterministic order on locations.
DeclarationAndMacrosFinder can return the results in arbitrary orders and the
client code would be responsible for getting locations and finally sorting them.
WDYT?
================
Comment at: clangd/XRefs.cpp:296
// Emit all symbol locations (declaration or definition) from AST.
- for (const auto *D : Symbols.Decls) {
+ for (const auto &DI : Symbols.Decls) {
+ const auto *D = DI.D;
----------------
Maybe use explicit type here? Using 'auto' is a bit confusing.
================
Comment at: clangd/XRefs.cpp:297
+ for (const auto &DI : Symbols.Decls) {
+ const auto *D = DI.D;
// Fake key for symbols don't have USR (no SymbolID).
----------------
Maybe use explicit type here too or rename the field from 'D' to something more
descriptive (e.g. `Decl `)?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50438
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
