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