kadircet marked 2 inline comments as done.
kadircet added inline comments.

================
Comment at: clang-tools-extra/clangd/FindTarget.cpp:584
+    void VisitRedeclarableTemplateDecl(const RedeclarableTemplateDecl *TD) {
+      // {Class,Function,Var,TypeAlias}TemplateDecls are visited as part of the
+      // NamedDecl below, we skip them here to not visit them twice.
----------------
kadircet wrote:
> hokein wrote:
> > the code this is correct right now (since 
> > {Class,Function,Var,TypeAlias}TemplateDecls  are the decls that derive from 
> > `RedeclarableTemplateDecl`), but I would do this in another way -- just add 
> > a filter in the VisitNamedDecl below, like
> > 
> > 
> > ```
> > void VisitNamedDecl(const NamedDecl *ND) {
> >    ...
> >    if (ND is one of the  {Class,Function,Var,TypeAlias}TemplateDecls)
> >      return;
> > }
> > ```
> these two have a slightly different behaviour though, I deliberately chose to 
> stop this in here.
> 
> Eliminating here results in dropping template decl, the one containing 
> template parameters.
> Whereas eliminating inside `VisitNamedDecl`, would result in dropping the 
> described template, the real definition coming after template parameters.
> 
> It is always possible to switch between the two using 
> `get{TemplatedDecl,DescribedTemplate}` but I think eliminating the 
> templatedecl aligns better with
> current api, since at the specified position we actually have the 
> `DescribedTemplate`, not the `TemplatedDecl` and template parameters are 
> already being
> reported independent from this.
yeah actually re-thinking about it they will have the same affect since we can 
check for specific types, nvm this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73101



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to