ilya-biryukov added a comment. Added the test.
================ Comment at: clangd/index/FileIndex.cpp:37 + std::vector<Decl *> TopLevelDecls( + AST.getTranslationUnitDecl()->decls().begin(), + AST.getTranslationUnitDecl()->decls().end()); ---------------- ioeric wrote: > ilya-biryukov wrote: > > ioeric wrote: > > > Would this give us decls in namespaces? It seems that > > > `AST.getTranslationUnitDecl()->decls()` are only decls that are > > > immediately inside the TU context. > > I'll double check and add a test for that, but I think the indexer visits > > the namespaces that we provide, I'm not sure if we have tests though. > Oh, right! In that case, it seems that a single TU decl which contains all > interesting decls would be sufficient? Is it possible that preamble doesn't > have all available top level decls in TU decl? This seems to be possible for > ParsedAST, which is why we used `getTopLevelDecls`, I think? > In that case, it seems that a single TU decl which contains all interesting > decls would be sufficient? I've tried doing the TU decl only and it does not work, because `indexTopLevelDecl` checks that location of decl that you pass to it is valid xD. Obviously, that's not the case for the TU decl. But it does visit the namespaces recursively, new test also checks for that, so we should be fine with the current code. > This seems to be possible for ParsedAST, which is why we used > getTopLevelDecls, I think? `getTopLevelDecls` was copied from the `ASTUnit` without much thought. IIRC, it allows less deserialization from preamble when traversing the AST for things like "go to def", etc. This is definitely not something we're optimizing for here, since we run on the preamble AST itself. Now that I think about it, I don't think we need to store top-level decls of preamble at all for any of our features... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47272 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
