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

Reply via email to