sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clangd/XRefs.cpp:261
}
+// Returns the preferred location between an AST location and an index
location.
+SymbolLocation getPreferredLocation(const Location &ASTLoc,
----------------
nit: move this and toIndexLocation together?
================
Comment at: clangd/XRefs.cpp:364
+ if (Sym.CanonicalDeclaration) {
+ // We only do this for declarations as definitions from AST
+ // is generally preferred (e.g. definitions in main file).
----------------
a little unclear what "we only do this" refers to - maybe missing a first line
of the comment here.
// Use merge logic to choose AST or index declaration.
================
Comment at: clangd/index/Merge.cpp:155
+ getPreferredLocation(S.CanonicalDeclaration, O.CanonicalDeclaration);
+ S.Definition = getPreferredLocation(S.Definition, O.Definition);
S.References += O.References;
----------------
I think it might be clearer what this is doing, and more similar with
surrounding code, if the function returns a bool:
```
bool prefer(const SymbolLocation &Candidate, const SymbolLocation &Baseline);
...
if (!S.Definition || prefer(O.Definition, S.Definition))
S.Definition = O.Definition;
```
up to you
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58037/new/
https://reviews.llvm.org/D58037
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
