kadircet created this revision.
kadircet added reviewers: sammccall, jkorous.
Herald added subscribers: cfe-commits, usaxena95, arphaman, dexonsmith,
MaskRay, ilya-biryukov.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80988
Files:
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -328,6 +328,8 @@
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 @@
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) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -136,7 +136,8 @@
ReplayPreamble(const IncludeStructure &Includes, PPCallbacks *Delegate,
const SourceManager &SM, Preprocessor &PP,
const LangOptions &LangOpts, const PreambleBounds &PB)
- : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) {
+ : IncludesToReplay(Includes.MainFileIncludes), 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 +168,7 @@
}
void replay() {
- for (const auto &Inc : Includes.MainFileIncludes) {
+ for (const auto &Inc : IncludesToReplay) {
const FileEntry *File = nullptr;
if (Inc.Resolved != "")
if (auto FE = SM.getFileManager().getFile(Inc.Resolved))
@@ -227,7 +228,7 @@
}
}
- const IncludeStructure &Includes;
+ std::vector<Inclusion> IncludesToReplay;
PPCallbacks *Delegate;
const SourceManager &SM;
Preprocessor &PP;
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -328,6 +328,8 @@
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 @@
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) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -136,7 +136,8 @@
ReplayPreamble(const IncludeStructure &Includes, PPCallbacks *Delegate,
const SourceManager &SM, Preprocessor &PP,
const LangOptions &LangOpts, const PreambleBounds &PB)
- : Includes(Includes), Delegate(Delegate), SM(SM), PP(PP) {
+ : IncludesToReplay(Includes.MainFileIncludes), 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 +168,7 @@
}
void replay() {
- for (const auto &Inc : Includes.MainFileIncludes) {
+ for (const auto &Inc : IncludesToReplay) {
const FileEntry *File = nullptr;
if (Inc.Resolved != "")
if (auto FE = SM.getFileManager().getFile(Inc.Resolved))
@@ -227,7 +228,7 @@
}
}
- const IncludeStructure &Includes;
+ std::vector<Inclusion> IncludesToReplay;
PPCallbacks *Delegate;
const SourceManager &SM;
Preprocessor &PP;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
