[PATCH] D50438: [clangd] Sort GoToDefinition results.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/XRefs.cpp:71
 
+struct DeclInfo {
+  const Decl *D;
----------------
NIT: maybe call `Occurence` instead? As this is actually a `Decl` with some extra data, computed based on the expression it originated from. Occurence seems like a nice that for that kind of thing.


================
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) {
----------------
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?


================
Comment at: clangd/XRefs.cpp:128
+  void insertDecl(const Decl *D, bool IsExplicitReferenced) {
+    auto It = Decls.find(D);
+    if (It != Decls.end())
----------------
Maybe simplify to `Decls[D] |= IsExplicitReferenced`?


================
Comment at: clangd/XRefs.cpp:143
+      auto hasImplicitExpr = [](const Expr *E) {
+        if (E && E->child_begin() != E->child_end()) {
+          // Use the first child is good enough for most cases -- normally the
----------------
NIT: [use early exits](https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code) by inverting condition


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50438