Author: rksimon Date: Thu May 2 08:47:33 2019 New Revision: 359796 URL: http://llvm.org/viewvc/llvm-project?rev=359796&view=rev Log: Revert rL359778 : [clangd] Fix code completion of macros defined in the preamble region of the main file.
Summary: This is a tricky case (we baked the assumption that symbols come from the preamble xor mainfile pretty deeply) and the fix is a bit of a hack: We look at the code to guess the macro names, and deserialize them from the preamble "by hand". Reviewers: ilya-biryukov Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D60937 ........ Fix buildbots http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/47684/ Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/ClangdUnit.h clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=359796&r1=359795&r2=359796&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Thu May 2 08:47:33 2019 @@ -20,8 +20,6 @@ #include "index/Index.h" #include "clang/AST/ASTContext.h" #include "clang/Basic/LangOptions.h" -#include "clang/Basic/SourceManager.h" -#include "clang/Basic/TokenKinds.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" @@ -96,36 +94,6 @@ private: std::vector<Decl *> TopLevelDecls; }; -class CollectMainFileMacros : public PPCallbacks { -public: - explicit CollectMainFileMacros(const SourceManager &SM, - std::vector<std::string> *Out) - : SM(SM), Out(Out) {} - - void FileChanged(SourceLocation Loc, FileChangeReason, - SrcMgr::CharacteristicKind, FileID Prev) { - InMainFile = SM.isWrittenInMainFile(Loc); - } - - void MacroDefined(const Token &MacroName, const MacroDirective *MD) { - assert(MacroName.is(tok::identifier)); - if (InMainFile) - MainFileMacros.insert(MacroName.getIdentifierInfo()->getName()); - } - - void EndOfMainFile() { - for (const auto& Entry : MainFileMacros) - Out->push_back(Entry.getKey()); - llvm::sort(*Out); - } - - private: - const SourceManager &SM; - bool InMainFile = true; - llvm::StringSet<> MainFileMacros; - std::vector<std::string> *Out; -}; - class CppFilePreambleCallbacks : public PreambleCallbacks { public: CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback) @@ -135,10 +103,6 @@ public: IncludeStructure takeIncludes() { return std::move(Includes); } - std::vector<std::string> takeMainFileMacros() { - return std::move(MainFileMacros); - } - CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); } void AfterExecute(CompilerInstance &CI) override { @@ -154,9 +118,7 @@ public: std::unique_ptr<PPCallbacks> createPPCallbacks() override { assert(SourceMgr && "SourceMgr must be set at this point"); - return llvm::make_unique<PPChainedCallbacks>( - collectIncludeStructureCallback(*SourceMgr, &Includes), - llvm::make_unique<CollectMainFileMacros>(*SourceMgr, &MainFileMacros)); + return collectIncludeStructureCallback(*SourceMgr, &Includes); } CommentHandler *getCommentHandler() override { @@ -169,7 +131,6 @@ private: PreambleParsedCallback ParsedCallback; IncludeStructure Includes; CanonicalIncludes CanonIncludes; - std::vector<std::string> MainFileMacros; std::unique_ptr<CommentHandler> IWYUHandler = nullptr; SourceManager *SourceMgr = nullptr; }; @@ -498,13 +459,11 @@ const CanonicalIncludes &ParsedAST::getC PreambleData::PreambleData(PrecompiledPreamble Preamble, std::vector<Diag> Diags, IncludeStructure Includes, - std::vector<std::string> MainFileMacros, std::unique_ptr<PreambleFileStatusCache> StatCache, CanonicalIncludes CanonIncludes) : Preamble(std::move(Preamble)), Diags(std::move(Diags)), - Includes(std::move(Includes)), MainFileMacros(std::move(MainFileMacros)), - StatCache(std::move(StatCache)), CanonIncludes(std::move(CanonIncludes)) { -} + Includes(std::move(Includes)), StatCache(std::move(StatCache)), + CanonIncludes(std::move(CanonIncludes)) {} ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble, std::unique_ptr<CompilerInstance> Clang, @@ -583,8 +542,7 @@ buildPreamble(PathRef FileName, Compiler std::vector<Diag> Diags = PreambleDiagnostics.take(); return std::make_shared<PreambleData>( std::move(*BuiltPreamble), std::move(Diags), - SerializedDeclsCollector.takeIncludes(), - SerializedDeclsCollector.takeMainFileMacros(), std::move(StatCache), + SerializedDeclsCollector.takeIncludes(), std::move(StatCache), SerializedDeclsCollector.takeCanonicalIncludes()); } else { elog("Could not build a preamble for file {0}", FileName); Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=359796&r1=359795&r2=359796&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.h (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.h Thu May 2 08:47:33 2019 @@ -48,7 +48,6 @@ namespace clangd { struct PreambleData { PreambleData(PrecompiledPreamble Preamble, std::vector<Diag> Diags, IncludeStructure Includes, - std::vector<std::string> MainFileMacros, std::unique_ptr<PreambleFileStatusCache> StatCache, CanonicalIncludes CanonIncludes); @@ -58,10 +57,6 @@ struct PreambleData { // Processes like code completions and go-to-definitions will need #include // information, and their compile action skips preamble range. IncludeStructure Includes; - // Macros defined in the preamble section of the main file. - // Users care about headers vs main-file, not preamble vs non-preamble. - // These should be treated as main-file entities e.g. for code completion. - std::vector<std::string> MainFileMacros; // Cache of FS operations performed when building the preamble. // When reusing a preamble, this cache can be consumed to save IO. std::unique_ptr<PreambleFileStatusCache> StatCache; Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=359796&r1=359795&r2=359796&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu May 2 08:47:33 2019 @@ -44,8 +44,6 @@ #include "clang/Format/Format.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendActions.h" -#include "clang/Lex/ExternalPreprocessorSource.h" -#include "clang/Lex/Preprocessor.h" #include "clang/Lex/PreprocessorOptions.h" #include "clang/Sema/CodeCompleteConsumer.h" #include "clang/Sema/DeclSpec.h" @@ -1019,23 +1017,6 @@ struct SemaCompleteInput { llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS; }; -void loadMainFilePreambleMacros(const Preprocessor &PP, - const PreambleData &Preamble) { - // The ExternalPreprocessorSource has our macros, if we know where to look. - // We can read all the macros using PreambleMacros->ReadDefinedMacros(), - // but this includes transitively included files, so may deserialize a lot. - ExternalPreprocessorSource *PreambleMacros = PP.getExternalSource(); - // As we have the names of the macros, we can look up their IdentifierInfo - // and then use this to load just the macros we want. - IdentifierInfoLookup *PreambleIdentifiers = - PP.getIdentifierTable().getExternalIdentifierLookup(); - if (!PreambleIdentifiers || !PreambleMacros) - return; - for (const auto& MacroName : Preamble.MainFileMacros) - if (auto *II = PreambleIdentifiers->get(MacroName)) - PreambleMacros->updateOutOfDateIdentifier(*II); -} - // Invokes Sema code completion on a file. // If \p Includes is set, it will be updated based on the compiler invocation. bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer, @@ -1077,9 +1058,9 @@ bool semaCodeComplete(std::unique_ptr<Co // However, if we're completing *inside* the preamble section of the draft, // overriding the preamble will break sema completion. Fortunately we can just // skip all includes in this case; these completions are really simple. - PreambleBounds PreambleRegion = - ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0); - bool CompletingInPreamble = PreambleRegion.Size > Input.Offset; + bool CompletingInPreamble = + ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0).Size > + Input.Offset; // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise // the remapped buffers do not get freed. IgnoreDiagnostics DummyDiagsConsumer; @@ -1097,14 +1078,6 @@ bool semaCodeComplete(std::unique_ptr<Co Input.FileName); return false; } - // Macros can be defined within the preamble region of the main file. - // They don't fall nicely into our index/Sema dichotomy: - // - they're not indexed for completion (they're not available across files) - // - but Sema code complete won't see them: as part of the preamble, they're - // deserialized only when mentioned. - // Force them to be deserialized so SemaCodeComplete sees them. - if (Input.Preamble) - loadMainFilePreambleMacros(Clang->getPreprocessor(), *Input.Preamble); if (Includes) Clang->getPreprocessor().addPPCallbacks( collectIncludeStructureCallback(Clang->getSourceManager(), Includes)); Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=359796&r1=359795&r2=359796&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Thu May 2 08:47:33 2019 @@ -24,7 +24,6 @@ #include "clang/Basic/SourceManager.h" #include "clang/Basic/Specifiers.h" #include "clang/Index/IndexSymbol.h" -#include "clang/Index/IndexingAction.h" #include "clang/Index/USRGeneration.h" #include "clang/Lex/Preprocessor.h" #include "llvm/Support/Casting.h" Modified: clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp?rev=359796&r1=359795&r2=359796&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp Thu May 2 08:47:33 2019 @@ -2073,28 +2073,19 @@ TEST(CompletionTest, MergeMacrosFromInde UnorderedElementsAre(Named("Clangd_Macro_Test"))); } -TEST(CompletionTest, MacroFromPreamble) { - MockFSProvider FS; - MockCompilationDatabase CDB; - std::string FooHeader = testPath("foo.h"); - FS.Files[FooHeader] = "#define CLANGD_PREAMBLE_HEADER x\n"; - IgnoreDiagnostics DiagConsumer; - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); +TEST(CompletionTest, NoMacroFromPreambleIfIndexIsSet) { auto Results = completions( - R"cpp(#include "foo.h" - #define CLANGD_PREAMBLE_MAIN x + R"cpp(#define CLANGD_PREAMBLE x int x = 0; #define CLANGD_MAIN x void f() { CLANGD_^ } )cpp", {func("CLANGD_INDEX")}); - // We should get results from the main file, including the preamble section. - // However no results from included files (the index should cover them). - EXPECT_THAT(Results.Completions, - UnorderedElementsAre(Named("CLANGD_PREAMBLE_MAIN"), - Named("CLANGD_MAIN"), - Named("CLANGD_INDEX"))); + // Index is overriden in code completion options, so the preamble symbol is + // not seen. + EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("CLANGD_MAIN"), + Named("CLANGD_INDEX"))); } TEST(CompletionTest, DeprecatedResults) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
