ilya-biryukov added inline comments.
================ Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68 + if (const auto *TD = dyn_cast<T>(D)) + return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation; + return false; ---------------- jvikstrom wrote: > ilya-biryukov wrote: > > hokein wrote: > > > ilya-biryukov wrote: > > > > We also want to skip `TSK_ExplicitInstantiationDeclaration` and > > > > `TSK_ExplicitInstantiationDefinition` here. > > > > This covers cases like (not sure which one of the two enum values we > > > > get, though): > > > > ``` > > > > template <class T> > > > > int foo(T) { ... } > > > > > > > > template int foo(int); // we'd rather not traverse these, highlightings > > > > will run into the same problems. > > > > ``` > > > > > > > > Semantics I'm describing are roughly similar to > > > > `isImplicitInstatiation(D) == !isExplicitInstantion(D)`, where > > > > `isExplicitInstantiation` is taken from `CodeComplete.cpp`. (If we > > > > ignore `TSK_Undeclared`, which, I believe, should never be encountered > > > > in decls passed to HandleTopLevelDecl). > > > > > > > > Please extract the helper from code complete and this one into a > > > > separate file (e.g. `clangd/AST.h`) and possibly implement one through > > > > the other > > > > Semantics I'm describing are roughly similar to > > > > isImplicitInstatiation(D) == !isExplicitInstantion(D), > > > > > > I think there is a typo here, I believe you mean > > > `isImplicitInstantiation(D) == !isExplicitSpecialization(D) ` (in > > > CodeComplete.cpp, it checks whether a Decl is an explicit > > > **specialization**). > > Yes, there's a typo thanks! > Still want me to move this and the helper from CodeComplete to `AST.h` as it > isn't used anywhere else? (esp. when we can't implement them through of each > other) Yes, it's better to share this code between `codeComplete` and `ClangdUnit`. It might pop up in more places and it's not trivial. ================ Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:82 continue; + if (isImplicitTemplateInstantiation<FunctionDecl>(D) || + isImplicitTemplateInstantiation<CXXRecordDecl>(D) || ---------------- Could we expose the following non-template function instead? ``` bool isImplicitTemplateInstantiation(NamedDecl *); ``` So that the users don't need to specify which `Decl` they are interested in (I believe there's no use for it anyway) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65510/new/ https://reviews.llvm.org/D65510 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits