diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h --- a/clang-tools-extra/clangd/Headers.h +++ b/clang-tools-extra/clangd/Headers.h @@ -60,6 +60,7 @@ SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User; }; llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &); +bool operator==(const Inclusion &LHS, const Inclusion &RHS); // Contains information about one file in the build grpah and its direct // dependencies. Doesn't own the strings it references (IncludeGraph is diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp --- a/clang-tools-extra/clangd/Headers.cpp +++ b/clang-tools-extra/clangd/Headers.cpp @@ -253,5 +253,11 @@ << Inc.R; } +bool operator==(const Inclusion &LHS, const Inclusion &RHS) { + return std::tie(LHS.Directive, LHS.FileKind, LHS.HashOffset, LHS.R, + LHS.Resolved, LHS.Written) == + std::tie(RHS.Directive, RHS.FileKind, RHS.HashOffset, RHS.R, + RHS.Resolved, RHS.Written); +} } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -9,6 +9,7 @@ #include "Preamble.h" #include "Compiler.h" #include "Headers.h" +#include "SourceCode.h" #include "support/Logger.h" #include "support/Trace.h" #include "clang/Basic/Diagnostic.h" @@ -298,6 +299,9 @@ ModifiedIncludes.takeError()); return {}; } + // No patch needed if includes are equal. + if (*BaselineIncludes == *ModifiedIncludes) + return {}; PreamblePatch PP; // This shouldn't coincide with any real file name. @@ -314,11 +318,20 @@ ExistingIncludes.insert({Inc.Directive, Inc.Written}); // Calculate extra includes that needs to be inserted. llvm::raw_string_ostream Patch(PP.PatchContents); + Patch << llvm::formatv("#line 0 \"{0}\"\n", FileName); for (const auto &Inc : *ModifiedIncludes) { if (ExistingIncludes.count({Inc.Directive, Inc.Written})) continue; - Patch << llvm::formatv("#{0} {1}\n", spellingForIncDirective(Inc.Directive), - Inc.Written); + // Include is new in the modified preamble. Inject it into the patch and use + // #line to set the presumed location to where it is spelled + // FIXME: Traverse once. + auto LineCol = offsetToClangLineColumn(Modified.Contents, Inc.HashOffset); + Patch << llvm::formatv("#line {0}\n", LineCol.first); + llvm::StringRef Directive = spellingForIncDirective(Inc.Directive); + size_t Padding = Inc.R.start.character - Directive.size() - LineCol.second; + Patch << llvm::formatv("{0}#{1}{2}{3}\n", + std::string(LineCol.second - 1, ' '), Directive, + std::string(Padding, ' '), Inc.Written); } Patch.flush(); diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -10,31 +10,88 @@ #include "Compiler.h" #include "Preamble.h" #include "TestFS.h" +#include "TestTU.h" +#include "clang/Frontend/PrecompiledPreamble.h" #include "clang/Lex/PreprocessorOptions.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/VirtualFileSystem.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include #include #include +using testing::Field; + namespace clang { namespace clangd { namespace { -using testing::_; -using testing::Contains; -using testing::Pair; - -MATCHER_P(HasContents, Contents, "") { return arg->getBuffer() == Contents; } - -TEST(PreamblePatchTest, IncludeParsing) { - MockFSProvider FS; +// Builds a preamble for BaselineContents, patches it for ModifiedContents and +// returns the includes in the patch. +IncludeStructure collectPatchedIncludes(llvm::StringRef ModifiedContents, + llvm::StringRef BaselineContents) { + std::string MainFile = testPath("main.cpp"); + ParseInputs PI; + PI.FS = new llvm::vfs::InMemoryFileSystem; MockCompilationDatabase CDB; + // ms-compatibility changes meaning of #import, make sure it is turned off. + CDB.ExtraClangFlags.push_back("-fno-ms-compatibility"); + PI.CompileCommand = CDB.getCompileCommand(MainFile).getValue(); + // Create invocation IgnoreDiagnostics Diags; - ParseInputs PI; - PI.FS = FS.getFileSystem(); + auto CI = buildCompilerInvocation(PI, Diags); + assert(CI && "failed to create compiler invocation"); + // Build baseline preamble. + PI.Contents = BaselineContents.str(); + PI.Version = "baseline preamble"; + auto BaselinePreamble = buildPreamble(MainFile, *CI, PI, true, nullptr); + assert(BaselinePreamble && "failed to build baseline preamble"); + // Create the patch. + PI.Contents = ModifiedContents.str(); + PI.Version = "modified contents"; + auto PP = PreamblePatch::create(MainFile, PI, *BaselinePreamble); + // Collect patch contents. + PP.apply(*CI); + llvm::StringRef PatchContents; + for (const auto &Rempaped : CI->getPreprocessorOpts().RemappedFileBuffers) { + if (Rempaped.first == testPath("__preamble_patch__.h")) { + PatchContents = Rempaped.second->getBuffer(); + break; + } + } + // Run preprocessor over the modified contents with patched Invocation to and + // BaselinePreamble to collect includes in the patch. We trim the input to + // only preamble section to not collect includes in the mainfile. + auto Bounds = Lexer::ComputePreamble(ModifiedContents, *CI->getLangOpts()); + auto Clang = + prepareCompilerInstance(std::move(CI), &BaselinePreamble->Preamble, + llvm::MemoryBuffer::getMemBufferCopy( + ModifiedContents.slice(0, Bounds.Size).str()), + PI.FS, Diags); + Clang->getPreprocessorOpts().ImplicitPCHInclude.clear(); + PreprocessOnlyAction Action; + if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) { + ADD_FAILURE() << "failed begin source file"; + return {}; + } + IncludeStructure Includes; + Clang->getPreprocessor().addPPCallbacks( + collectIncludeStructureCallback(Clang->getSourceManager(), &Includes)); + if (llvm::Error Err = Action.Execute()) { + ADD_FAILURE() << "failed to execute action: " << std::move(Err); + return {}; + } + Action.EndSourceFile(); + return Includes; +} +// Check preamble lexing logic by building an empty preamble and patching it +// with all the contents. +TEST(PreamblePatchTest, IncludeParsing) { // We expect any line with a point to show up in the patch. llvm::StringRef Cases[] = { // Only preamble @@ -61,69 +118,36 @@ ^#include )cpp", }; - // ms-compatibility changes meaning of #import, make sure it is turned off. - CDB.ExtraClangFlags.push_back("-fno-ms-compatibility"); - const auto FileName = testPath("foo.cc"); for (const auto Case : Cases) { Annotations Test(Case); const auto Code = Test.code(); - PI.CompileCommand = *CDB.getCompileCommand(FileName); - SCOPED_TRACE(Code); - // Check preamble lexing logic by building an empty preamble and patching it - // with all the contents. - PI.Contents = ""; - const auto CI = buildCompilerInvocation(PI, Diags); - const auto EmptyPreamble = buildPreamble(FileName, *CI, PI, true, nullptr); - PI.Contents = Code.str(); - std::string ExpectedBuffer; - const auto Points = Test.points(); - for (const auto &P : Points) { - // Copy the whole line. - auto StartOffset = llvm::cantFail(positionToOffset(Code, P)); - ExpectedBuffer.append(Code.substr(StartOffset) - .take_until([](char C) { return C == '\n'; }) - .str()); - ExpectedBuffer += '\n'; + auto Includes = + collectPatchedIncludes(Code, /*BaselineContents=*/"").MainFileIncludes; + auto Points = Test.points(); + EXPECT_EQ(Includes.size(), Points.size()); + for (size_t I = 0, E = Includes.size(); I != E; ++I) { + auto &Inc = Includes[I]; + auto StartOffset = llvm::cantFail(positionToOffset(Code, Points[I])); + EXPECT_EQ(Inc.HashOffset, StartOffset); } - - PreamblePatch::create(FileName, PI, *EmptyPreamble).apply(*CI); - EXPECT_THAT(CI->getPreprocessorOpts().RemappedFileBuffers, - Contains(Pair(_, HasContents(ExpectedBuffer)))); - for (const auto &RB : CI->getPreprocessorOpts().RemappedFileBuffers) - delete RB.second; } } TEST(PreamblePatchTest, ContainsNewIncludes) { - MockFSProvider FS; - MockCompilationDatabase CDB; - IgnoreDiagnostics Diags; - // ms-compatibility changes meaning of #import, make sure it is turned off. - CDB.ExtraClangFlags.push_back("-fno-ms-compatibility"); - - const auto FileName = testPath("foo.cc"); - ParseInputs PI; - PI.FS = FS.getFileSystem(); - PI.CompileCommand = *CDB.getCompileCommand(FileName); - PI.Contents = "#include \n"; - - // Check diffing logic by adding a new header to the preamble and ensuring - // only it is patched. - const auto CI = buildCompilerInvocation(PI, Diags); - const auto FullPreamble = buildPreamble(FileName, *CI, PI, true, nullptr); - + constexpr llvm::StringLiteral BaseLineContents = "#include \n"; constexpr llvm::StringLiteral Patch = "#include \n#import \n"; // We provide the same includes twice, they shouldn't be included in the // patch. - PI.Contents = (Patch + PI.Contents + PI.Contents).str(); - PreamblePatch::create(FileName, PI, *FullPreamble).apply(*CI); - EXPECT_THAT(CI->getPreprocessorOpts().RemappedFileBuffers, - Contains(Pair(_, HasContents(Patch)))); - for (const auto &RB : CI->getPreprocessorOpts().RemappedFileBuffers) - delete RB.second; + auto Includes = + collectPatchedIncludes( + (Patch + BaseLineContents + BaseLineContents).str(), BaseLineContents) + .MainFileIncludes; + EXPECT_THAT(Includes, + ElementsAre(Field(&Inclusion::Written, ""), + Field(&Inclusion::Written, ""))); } } // namespace