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 @@ -283,5 +283,11 @@ << " at line" << Inc.HashLine; } +bool operator==(const Inclusion &LHS, const Inclusion &RHS) { + return std::tie(LHS.Directive, LHS.FileKind, LHS.HashOffset, LHS.HashLine, + LHS.Resolved, LHS.Written) == + std::tie(RHS.Directive, RHS.FileKind, RHS.HashOffset, RHS.HashLine, + 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" @@ -26,6 +27,7 @@ #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" @@ -270,6 +272,20 @@ Inputs.FS.get()); } +void escapeBackslashAndQuotes(llvm::StringRef Text, llvm::raw_ostream &OS) { + for (char C : Text) { + switch (C) { + case '\\': + case '"': + OS << '\\'; + break; + default: + break; + } + OS << C; + } +} + PreamblePatch PreamblePatch::create(llvm::StringRef FileName, const ParseInputs &Modified, const PreambleData &Baseline) { @@ -298,6 +314,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,9 +333,19 @@ ExistingIncludes.insert({Inc.Directive, Inc.Written}); // Calculate extra includes that needs to be inserted. llvm::raw_string_ostream Patch(PP.PatchContents); + // Set default filename for subsequent #line directives + Patch << "#line 0 \""; + // FileName part of a line directive is subject to backslash escaping, which + // might lead to problems on windows especially. + escapeBackslashAndQuotes(FileName, Patch); + Patch << "\"\n"; for (const auto &Inc : *ModifiedIncludes) { if (ExistingIncludes.count({Inc.Directive, Inc.Written})) continue; + // 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. + auto LineCol = offsetToClangLineColumn(Modified.Contents, Inc.HashOffset); + Patch << llvm::formatv("#line {0}\n", LineCol.first); Patch << llvm::formatv("#{0} {1}\n", spellingForIncDirective(Inc.Directive), Inc.Written); } 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,90 @@ #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, + llvm::StringRef MainFileName = "main.cpp") { + std::string MainFile = testPath(MainFileName); + 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,71 +120,44 @@ ^#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'; - } - - 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; + auto Includes = + collectPatchedIncludes(Code, /*BaselineContents=*/"").MainFileIncludes; + auto Points = Test.points(); + ASSERT_EQ(Includes.size(), Points.size()); + for (size_t I = 0, E = Includes.size(); I != E; ++I) + EXPECT_EQ(Includes[I].HashLine, Points[I].line); } } 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 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; + constexpr llvm::StringLiteral BaselineContents = R"cpp( + #include + #include // This will be removed + #include + )cpp"; + constexpr llvm::StringLiteral ModifiedContents = R"cpp( + #include + #include // This has changed a line. + #include // This is a duplicate. + #include // This is newly introduced. + )cpp"; + auto Includes = collectPatchedIncludes(ModifiedContents, BaselineContents) + .MainFileIncludes; + EXPECT_THAT(Includes, ElementsAre(AllOf(Field(&Inclusion::Written, ""), + Field(&Inclusion::HashLine, 4)))); } +TEST(PreamblePatchTest, MainFileIsEscaped) { + auto Includes = collectPatchedIncludes("#include ", "", "file\"name.cpp") + .MainFileIncludes; + EXPECT_THAT(Includes, ElementsAre(AllOf(Field(&Inclusion::Written, ""), + Field(&Inclusion::HashLine, 0)))); +} } // namespace } // namespace clangd } // namespace clang