ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments.
================ Comment at: clang-tools-extra/clangd/Hover.cpp:188 +// returns D. +const NamedDecl *getExplicitSpec(const NamedDecl *D) { + if (auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D)) { ---------------- kadircet wrote: > ilya-biryukov wrote: > > kadircet wrote: > > > ilya-biryukov wrote: > > > > What's the purpose of this function? > > > > I don't think its description has semantic meaning in C++, "implicit > > > > instantiations" do not have an "explicit specialization"... > > > > > > > > It seems to be doing something to change a decl into something that > > > > could be used to query the index. If that's the case, we could probably > > > > have a name that's closer to the described goal. > > > yeah naming is hard :/ > > > > > > it is not just something that can be used to query index, but also for > > > AST, as the decl of instantiation doesn't contain comments attached to > > > specialization. > > > this is basically returning the explicit specialization used to > > > instantiate `D`. > > > > > > any suggestions for the name ? > > I would go with the following (a different comment is welcome, just quickly > > came up with something): > > ``` > > /// Returns a decl that should be used to produce hover, i.e. the one > > /// we should query for documentation comment and use in index queries. > > Decl *getDeclForHoverInfo(Decl *D) > > ``` > > > > The name is not smart and one would definitely need to read the comment to > > understand why we need it in the first place, but at least it would make > > sure we avoid confusion. > the problem is, this is not the decl used to produce hover exactly (for > example we want to make use of implicit instantiation for printing the name). > we only want to make use of it for comment retrieval, modifying according to > that. If it's just for the comment, `getDeclForComment` is perfect :-) ================ Comment at: clang-tools-extra/clangd/Hover.cpp:189 +const NamedDecl *getDeclForComment(const NamedDecl *D) { + // Return explicit specialization for implicit instantiations, as comments are + // attached to that. ---------------- There's no such thing as `explicit specialization for implicit instantiations`. Maybe remove this comment? It merely duplicates the code and it doesn't really matter what happens there, as long as we get the comment in hover. 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