[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:188
+// Returns the decl that should be used for querying comments, either from index
+// or ast.
+const NamedDecl *getDeclForComment(const NamedDecl *D) {
----------------
NIT: AST


================
Comment at: clang-tools-extra/clangd/Hover.cpp:205
                       const SymbolIndex *Index) {
-  if (!Index || !llvm::isa<NamedDecl>(D))
-    return;
-  const NamedDecl &ND = *cast<NamedDecl>(D);
   // We only add documentation, so don't bother if we already have some.
+  if (!Hover.Documentation.empty() || !Index)
----------------
NIT: maybe add `assert(&ND == getDeclForComment(&ND))`?


================
Comment at: clang-tools-extra/clangd/Hover.cpp:377
 
-  if (HI.Name.empty()) {
+    const auto *ND = getDeclForComment(D);
+    HI.Documentation = getDeclComment(ASTCtx, *ND);
----------------
NIT: `ND` suggests it's the same as `D` with a different type.
I'd use a different name, even if it's larger, e.g. `CommentD`


================
Comment at: clang-tools-extra/clangd/Hover.cpp:418
+const Decl *preferInstantiation(llvm::iterator_range<const Decl **> Range) {
+  if (Range.empty())
+    return nullptr;
----------------
This seems to be doing exactly the same thing as `explicitReferenceTargets` from `FindTarget.cpp`

WDYT about exposing the latter in `FindTarget.h` and using it here?
It seems to be pretty well-defined and useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596