[clang-tools-extra] 717bef6 - [clangd] Preserve line information while build PreamblePatch
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Thu May 7 03:25:49 PDT 2020
Author: Kadir Cetinkaya
Date: 2020-05-07T12:24:28+02:00
New Revision: 717bef6623291b550d3cd66c32092cc0ca1c236b
URL: https://github.com/llvm/llvm-project/commit/717bef6623291b550d3cd66c32092cc0ca1c236b
DIFF: https://github.com/llvm/llvm-project/commit/717bef6623291b550d3cd66c32092cc0ca1c236b.diff
LOG: [clangd] Preserve line information while build PreamblePatch
Summary: Depends on D78740.
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D78743
Added:
Modified:
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp
index ffd6bd1dd6cc..2b9f8feb7db8 100644
--- a/clang-tools-extra/clangd/Headers.cpp
+++ b/clang-tools-extra/clangd/Headers.cpp
@@ -283,5 +283,11 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Inclusion &Inc) {
<< " 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/Headers.h b/clang-tools-extra/clangd/Headers.h
index d7053b396d39..164a2005ac91 100644
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -60,6 +60,7 @@ struct Inclusion {
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/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index 2768bd1ec6a0..640a2c66b3c9 100644
--- 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 @@ bool isPreambleCompatible(const PreambleData &Preamble,
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 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
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 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
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
index 5ff7ebc27e09..c1801980b1d5 100644
--- 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 <clang/Frontend/FrontendActions.h>
#include <string>
#include <vector>
+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 @@ TEST(PreamblePatchTest, IncludeParsing) {
^#include <b.h>)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 <existing.h>\n";
-
- // Check
diff ing 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 <test>\n#import <existing.h>\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 <a.h>
+ #include <b.h> // This will be removed
+ #include <c.h>
+ )cpp";
+ constexpr llvm::StringLiteral ModifiedContents = R"cpp(
+ #include <a.h>
+ #include <c.h> // This has changed a line.
+ #include <c.h> // This is a duplicate.
+ #include <d.h> // This is newly introduced.
+ )cpp";
+ auto Includes = collectPatchedIncludes(ModifiedContents, BaselineContents)
+ .MainFileIncludes;
+ EXPECT_THAT(Includes, ElementsAre(AllOf(Field(&Inclusion::Written, "<d.h>"),
+ Field(&Inclusion::HashLine, 4))));
}
+TEST(PreamblePatchTest, MainFileIsEscaped) {
+ auto Includes = collectPatchedIncludes("#include <a.h>", "", "file\"name.cpp")
+ .MainFileIncludes;
+ EXPECT_THAT(Includes, ElementsAre(AllOf(Field(&Inclusion::Written, "<a.h>"),
+ Field(&Inclusion::HashLine, 0))));
+}
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list