Author: Kadir Cetinkaya Date: 2020-06-02T14:31:45+02:00 New Revision: 8506877c87aa602736aee1fffbd80b886fa40b79
URL: https://github.com/llvm/llvm-project/commit/8506877c87aa602736aee1fffbd80b886fa40b79 DIFF: https://github.com/llvm/llvm-project/commit/8506877c87aa602736aee1fffbd80b886fa40b79.diff LOG: [clangd] Copy existing includes in ReplayPreamble Summary: ReplayPreamble was just grabbing the reference of IncludeStructure passed to it and then replayed any includes seen so while exiting built-in file. This implies any include seen in built-in files being replayed as part of preamble, even though they are not. This wasn't an issue until we've started patching preambles, as includes from built-in files were not mapped back to main-file. This patch copies over existing includes at the time of ReplayPreamble::attach and only replies those to prevent any includes from the preamble patch getting mixed-in. Reviewers: sammccall, jkorous Subscribers: ilya-biryukov, MaskRay, dexonsmith, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D80988 Added: Modified: clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/unittests/ParsedASTTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 082c7cae0021..f3568c34f99a 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -116,7 +116,7 @@ class ReplayPreamble : private PPCallbacks { // Attach preprocessor hooks such that preamble events will be injected at // the appropriate time. // Events will be delivered to the *currently registered* PP callbacks. - static void attach(const IncludeStructure &Includes, CompilerInstance &Clang, + static void attach(std::vector<Inclusion> Includes, CompilerInstance &Clang, const PreambleBounds &PB) { auto &PP = Clang.getPreprocessor(); auto *ExistingCallbacks = PP.getPPCallbacks(); @@ -124,7 +124,7 @@ class ReplayPreamble : private PPCallbacks { if (!ExistingCallbacks) return; PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(new ReplayPreamble( - Includes, ExistingCallbacks, Clang.getSourceManager(), PP, + std::move(Includes), ExistingCallbacks, Clang.getSourceManager(), PP, Clang.getLangOpts(), PB))); // We're relying on the fact that addPPCallbacks keeps the old PPCallbacks // around, creating a chaining wrapper. Guard against other implementations. @@ -133,10 +133,10 @@ class ReplayPreamble : private PPCallbacks { } private: - ReplayPreamble(const IncludeStructure &Includes, PPCallbacks *Delegate, + ReplayPreamble(std::vector<Inclusion> Includes, PPCallbacks *Delegate, const SourceManager &SM, Preprocessor &PP, const LangOptions &LangOpts, const PreambleBounds &PB) - : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) { + : Includes(std::move(Includes)), Delegate(Delegate), SM(SM), PP(PP) { // Only tokenize the preamble section of the main file, as we are not // interested in the rest of the tokens. MainFileTokens = syntax::tokenize( @@ -167,7 +167,7 @@ class ReplayPreamble : private PPCallbacks { } void replay() { - for (const auto &Inc : Includes.MainFileIncludes) { + for (const auto &Inc : Includes) { const FileEntry *File = nullptr; if (Inc.Resolved != "") if (auto FE = SM.getFileManager().getFile(Inc.Resolved)) @@ -227,7 +227,7 @@ class ReplayPreamble : private PPCallbacks { } } - const IncludeStructure &Includes; + const std::vector<Inclusion> Includes; PPCallbacks *Delegate; const SourceManager &SM; Preprocessor &PP; @@ -382,7 +382,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, Includes = Preamble->Includes; Includes.MainFileIncludes = Patch->preambleIncludes(); // Replay the preamble includes so that clang-tidy checks can see them. - ReplayPreamble::attach(Includes, *Clang, Preamble->Preamble.getBounds()); + ReplayPreamble::attach(Patch->preambleIncludes(), *Clang, + Preamble->Preamble.getBounds()); } // Important: collectIncludeStructure is registered *after* ReplayPreamble! // Otherwise we would collect the replayed includes again... diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp index d86f741d9e72..0ad4a9f20ccb 100644 --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -328,6 +328,8 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) { testing::UnorderedElementsAreArray(TestCase.points())); } +MATCHER_P(WithFileName, Inc, "") { return arg.FileName == Inc; } + TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) { struct Inclusion { Inclusion(const SourceManager &SM, SourceLocation HashLoc, @@ -439,6 +441,24 @@ TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) { Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2)); EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name); } + + // Make sure replay logic works with patched preambles. + TU.Code = ""; + StoreDiags Diags; + auto Inputs = TU.inputs(); + auto CI = buildCompilerInvocation(Inputs, Diags); + auto EmptyPreamble = + buildPreamble(testPath(TU.Filename), *CI, Inputs, true, nullptr); + ASSERT_TRUE(EmptyPreamble); + TU.Code = "#include <a.h>"; + Includes.clear(); + auto PatchedAST = ParsedAST::build(testPath(TU.Filename), TU.inputs(), + std::move(CI), {}, EmptyPreamble); + ASSERT_TRUE(PatchedAST); + // Make sure includes were seen only once. + EXPECT_THAT(Includes, + ElementsAre(WithFileName(testPath("__preamble_patch__.h")), + WithFileName("a.h"))); } TEST(ParsedASTTest, PatchesAdditionalIncludes) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits