ilya-biryukov updated this revision to Diff 148602. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment.
- Rename the field in addition to the getter - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47331 Files: clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/XRefs.cpp unittests/clangd/TestTU.cpp
Index: unittests/clangd/TestTU.cpp =================================================================== --- unittests/clangd/TestTU.cpp +++ unittests/clangd/TestTU.cpp @@ -74,7 +74,7 @@ const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) { const NamedDecl *Result = nullptr; - for (const Decl *D : AST.getTopLevelDecls()) { + for (const Decl *D : AST.getLocalTopLevelDecls()) { auto *ND = dyn_cast<NamedDecl>(D); if (!ND || ND->getNameAsString() != QName) continue; Index: clangd/XRefs.cpp =================================================================== --- clangd/XRefs.cpp +++ clangd/XRefs.cpp @@ -168,7 +168,7 @@ IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = true; - indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(), + indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(), DeclMacrosFinder, IndexOpts); return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()}; @@ -418,7 +418,7 @@ IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = true; - indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(), + indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(), DocHighlightsFinder, IndexOpts); return DocHighlightsFinder.takeHighlights(); Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -44,12 +44,10 @@ // Stores Preamble and associated data. struct PreambleData { - PreambleData(PrecompiledPreamble Preamble, - std::vector<serialization::DeclID> TopLevelDeclIDs, - std::vector<Diag> Diags, std::vector<Inclusion> Inclusions); + PreambleData(PrecompiledPreamble Preamble, std::vector<Diag> Diags, + std::vector<Inclusion> Inclusions); PrecompiledPreamble Preamble; - std::vector<serialization::DeclID> TopLevelDeclIDs; std::vector<Diag> Diags; // Processes like code completions and go-to-definitions will need #include // information, and their compile action skips preamble range. @@ -80,17 +78,19 @@ ~ParsedAST(); + /// Note that the returned ast will not contain decls from the preamble that + /// were not deserialized during parsing. Clients should expect only decls + /// from the main file to be in the AST. ASTContext &getASTContext(); const ASTContext &getASTContext() const; Preprocessor &getPreprocessor(); std::shared_ptr<Preprocessor> getPreprocessorPtr(); const Preprocessor &getPreprocessor() const; - /// This function returns all top-level decls, including those that come - /// from Preamble. Decls, coming from Preamble, have to be deserialized, so - /// this call might be expensive. - ArrayRef<const Decl *> getTopLevelDecls(); + /// This function returns top-level decls present in the main file of the AST. + /// The result does not include the decls that come from the preamble. + ArrayRef<const Decl *> getLocalTopLevelDecls(); const std::vector<Diag> &getDiagnostics() const; @@ -103,11 +103,8 @@ ParsedAST(std::shared_ptr<const PreambleData> Preamble, std::unique_ptr<CompilerInstance> Clang, std::unique_ptr<FrontendAction> Action, - std::vector<const Decl *> TopLevelDecls, std::vector<Diag> Diags, - std::vector<Inclusion> Inclusions); - -private: - void ensurePreambleDeclsDeserialized(); + std::vector<const Decl *> LocalTopLevelDecls, + std::vector<Diag> Diags, std::vector<Inclusion> Inclusions); // In-memory preambles must outlive the AST, it is important that this member // goes before Clang and Action. @@ -122,8 +119,9 @@ // Data, stored after parsing. std::vector<Diag> Diags; - std::vector<const Decl *> TopLevelDecls; - bool PreambleDeclsDeserialized; + // Top-level decls inside the current file. Not that this does not include + // top-level decls from the preamble. + std::vector<const Decl *> LocalTopLevelDecls; std::vector<Inclusion> Inclusions; }; Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -90,37 +90,15 @@ CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback) : File(File), ParsedCallback(ParsedCallback) {} - std::vector<serialization::DeclID> takeTopLevelDeclIDs() { - return std::move(TopLevelDeclIDs); - } - std::vector<Inclusion> takeInclusions() { return std::move(Inclusions); } - void AfterPCHEmitted(ASTWriter &Writer) override { - TopLevelDeclIDs.reserve(TopLevelDecls.size()); - for (Decl *D : TopLevelDecls) { - // Invalid top-level decls may not have been serialized. - if (D->isInvalidDecl()) - continue; - TopLevelDeclIDs.push_back(Writer.getDeclID(D)); - } - } - void AfterExecute(CompilerInstance &CI) override { if (!ParsedCallback) return; trace::Span Tracer("Running PreambleCallback"); ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr()); } - void HandleTopLevelDecl(DeclGroupRef DG) override { - for (Decl *D : DG) { - if (isa<ObjCMethodDecl>(D)) - continue; - TopLevelDecls.push_back(D); - } - } - void BeforeExecute(CompilerInstance &CI) override { SourceMgr = &CI.getSourceManager(); } @@ -135,8 +113,6 @@ private: PathRef File; PreambleParsedCallback ParsedCallback; - std::vector<Decl *> TopLevelDecls; - std::vector<serialization::DeclID> TopLevelDeclIDs; std::vector<Inclusion> Inclusions; SourceManager *SourceMgr = nullptr; }; @@ -200,28 +176,6 @@ std::move(Inclusions)); } -void ParsedAST::ensurePreambleDeclsDeserialized() { - if (PreambleDeclsDeserialized || !Preamble) - return; - - std::vector<const Decl *> Resolved; - Resolved.reserve(Preamble->TopLevelDeclIDs.size()); - - ExternalASTSource &Source = *getASTContext().getExternalSource(); - for (serialization::DeclID TopLevelDecl : Preamble->TopLevelDeclIDs) { - // Resolve the declaration ID to an actual declaration, possibly - // deserializing the declaration in the process. - if (Decl *D = Source.GetExternalDecl(TopLevelDecl)) - Resolved.push_back(D); - } - - TopLevelDecls.reserve(TopLevelDecls.size() + - Preamble->TopLevelDeclIDs.size()); - TopLevelDecls.insert(TopLevelDecls.begin(), Resolved.begin(), Resolved.end()); - - PreambleDeclsDeserialized = true; -} - ParsedAST::ParsedAST(ParsedAST &&Other) = default; ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default; @@ -248,9 +202,8 @@ return Clang->getPreprocessor(); } -ArrayRef<const Decl *> ParsedAST::getTopLevelDecls() { - ensurePreambleDeclsDeserialized(); - return TopLevelDecls; +ArrayRef<const Decl *> ParsedAST::getLocalTopLevelDecls() { + return LocalTopLevelDecls; } const std::vector<Diag> &ParsedAST::getDiagnostics() const { return Diags; } @@ -260,29 +213,27 @@ // FIXME(ibiryukov): we do not account for the dynamically allocated part of // Message and Fixes inside each diagnostic. return AST.getASTAllocatedMemory() + AST.getSideTableAllocatedMemory() + - ::getUsedBytes(TopLevelDecls) + ::getUsedBytes(Diags); + ::getUsedBytes(LocalTopLevelDecls) + ::getUsedBytes(Diags); } const std::vector<Inclusion> &ParsedAST::getInclusions() const { return Inclusions; } PreambleData::PreambleData(PrecompiledPreamble Preamble, - std::vector<serialization::DeclID> TopLevelDeclIDs, std::vector<Diag> Diags, std::vector<Inclusion> Inclusions) - : Preamble(std::move(Preamble)), - TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)), + : Preamble(std::move(Preamble)), Diags(std::move(Diags)), Inclusions(std::move(Inclusions)) {} ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble, std::unique_ptr<CompilerInstance> Clang, std::unique_ptr<FrontendAction> Action, - std::vector<const Decl *> TopLevelDecls, + std::vector<const Decl *> LocalTopLevelDecls, std::vector<Diag> Diags, std::vector<Inclusion> Inclusions) : Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Diags(std::move(Diags)), - TopLevelDecls(std::move(TopLevelDecls)), PreambleDeclsDeserialized(false), + LocalTopLevelDecls(std::move(LocalTopLevelDecls)), Inclusions(std::move(Inclusions)) { assert(this->Clang); assert(this->Action); @@ -427,9 +378,8 @@ " for file " + Twine(FileName)); return std::make_shared<PreambleData>( - std::move(*BuiltPreamble), - SerializedDeclsCollector.takeTopLevelDeclIDs(), - PreambleDiagnostics.take(), SerializedDeclsCollector.takeInclusions()); + std::move(*BuiltPreamble), PreambleDiagnostics.take(), + SerializedDeclsCollector.takeInclusions()); } else { log("Could not build a preamble for file " + Twine(FileName)); return nullptr;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits