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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits