ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land.
lg. Thanks for adding the test! ================ Comment at: clangd/index/FileIndex.cpp:37 + std::vector<Decl *> TopLevelDecls( + AST.getTranslationUnitDecl()->decls().begin(), + AST.getTranslationUnitDecl()->decls().end()); ---------------- ilya-biryukov wrote: > 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... Sounds good. Thanks for the clarification! 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
