hokein 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; ---------------- 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**). ================ Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:219 + R"cpp( + class $Class[[Foo]] { + public: ---------------- instead adding a highlighting test, could we add this testcase to `ClangdUnitTests.cpp` instead? 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