ilya-biryukov marked an inline comment as done. ilya-biryukov added a comment.
One important comment about somehow distinguishing multiple decls with the same name. ================ Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:110 + template<typename T> + void f(T) {} + void s() { ---------------- jvikstrom wrote: > ilya-biryukov wrote: > > Could you also check that: > > > > 1. explicit specializations are present > > ``` > > template <> > > void f(bool) {} > > ``` > > 2. explicit instantiations are absent > > ``` > > template void f(bool); > > ``` > > 3. partial specializations are present (they should not be affected by this > > change, but testing those here seems appropriate) > > ``` > > template <class T> > > struct vector {}; > > > > template <class T> > > struct vector<T*> {}; // partial specialization, should be present > > ``` > > 4. template variables and classes are also handled: > > ``` > > template <class T> > > T foo = 10; // (!) requires C++17 > > ``` > Explicit instantiations are present in topLevelDecls though, otherwise > RecursiveASTVisitor would not traverse them (so adding a test to make sure > explicit instantiations are included in toplevel). > > Also adding a test to SemanticHighlighting to make sure that explicit > instantiations are visited in that (is some other RecursiveASTVisitor usage I > should add this to instead?) > Yes, sorry, I forgot about the results of your investigation yesterday. Having them in top-level decls seems fine, just wanted to make sure we actually test it. LG ================ Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:134 + template<class T> + struct V<T*> {}; + ---------------- Also add a full specialization (IIRC, they are represented completely differently in the AST): ``` template <> struct V<bool> {}; ``` ================ Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:137 + template<class T> + T foo = T(10); + int i = foo<int>; ---------------- Also add a partial and a full template specializations for the variable declaration: ``` template <class T> int foo<T*> = 0; template <> int foo<bool> = 0; ``` ================ Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:144 + EXPECT_THAT(AST.getLocalTopLevelDecls(), + ElementsAre(DeclNamed("f"), DeclNamed("f"), DeclNamed("f"), + DeclNamed("V"), DeclNamed("V"), DeclNamed("foo"), ---------------- Is there a way to check we are seeing the explicit instantiations? (e.g. by looking at template arguments?) It's not clear whether multiple `DeclNamed("foo")` refer to the decls we expect. 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