[PATCH] D79992: [clangd] Patch PP directives to use stale preambles while building ASTs
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfcde3d5b04b6: [clangd] Patch PP directives to use stale
preambles while building ASTs (authored by kadircet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79992/new/
https://reviews.llvm.org/D79992
Files:
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -26,7 +26,9 @@
#include
#include
+using testing::Contains;
using testing::Field;
+using testing::MatchesRegex;
namespace clang {
namespace clangd {
@@ -181,6 +183,109 @@
ElementsAre(AllOf(Field(&Inclusion::Written, "\"a.h\""),
Field(&Inclusion::Resolved, testPath("a.h");
}
+
+llvm::Optional createPatchedAST(llvm::StringRef Baseline,
+ llvm::StringRef Modified) {
+ auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+ if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return llvm::None;
+ }
+
+ IgnoreDiagnostics Diags;
+ auto TU = TestTU::withCode(Modified);
+ auto CI = buildCompilerInvocation(TU.inputs(), Diags);
+ if (!CI) {
+ADD_FAILURE() << "Failed to build compiler invocation";
+return llvm::None;
+ }
+ return ParsedAST::build(testPath("main.cpp"), TU.inputs(), std::move(CI), {},
+ BaselinePreamble);
+}
+
+std::string getPreamblePatch(llvm::StringRef Baseline,
+ llvm::StringRef Modified) {
+ auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+ if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return "";
+ }
+ auto TU = TestTU::withCode(Modified);
+ return PreamblePatch::create(testPath("main.cpp"), TU.inputs(),
+ *BaselinePreamble)
+ .text()
+ .str();
+}
+
+TEST(PreamblePatchTest, Define) {
+ // BAR should be defined while parsing the AST.
+ struct {
+llvm::StringLiteral Contents;
+llvm::StringLiteral ExpectedPatch;
+ } Cases[] = {
+ {
+ R"cpp(
+#define BAR
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ // multiline macro
+ {
+ R"cpp(
+#define BAR \
+
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ // multiline macro
+ {
+ R"cpp(
+#define \
+BAR
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ };
+
+ for (const auto &Case : Cases) {
+SCOPED_TRACE(Case.Contents);
+Annotations Modified(Case.Contents);
+EXPECT_THAT(getPreamblePatch("", Modified.code()),
+MatchesRegex(Case.ExpectedPatch.str()));
+
+auto AST = createPatchedAST("", Modified.code());
+ASSERT_TRUE(AST);
+EXPECT_THAT(AST->getDiagnostics(),
+Not(Contains(Field(&Diag::Range, Modified.range();
+ }
+}
+
+TEST(PreamblePatchTest, OrderingPreserved) {
+ llvm::StringLiteral Baseline = "#define BAR(X) X";
+ Annotations Modified(R"cpp(
+#define BAR(X, Y) X Y
+#define BAR(X) X
+[[BAR]](int y);
+ )cpp");
+
+ llvm::StringLiteral ExpectedPatch(R"cpp(#line 0 ".*main.cpp"
+#define BAR\(X, Y\) X Y
+#define BAR\(X\) X
+)cpp");
+ EXPECT_THAT(getPreamblePatch(Baseline, Modified.code()),
+ MatchesRegex(ExpectedPatch.str()));
+
+ auto AST = createPatchedAST(Baseline, Modified.code());
+ ASSERT_TRUE(AST);
+ EXPECT_THAT(AST->getDiagnostics(),
+ Not(Contains(Field(&Diag::Range, Modified.range();
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -119,6 +119,9 @@
/// using the presumed-location mechanism.
std::vector preambleIncludes() const;
+ /// Returns textual patch contents.
+ llvm::StringRef text() const { return PatchContents; }
+
private:
PreamblePatch() = default;
std::string PatchContents;
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -15,6 +15,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/F
[PATCH] D79992: [clangd] Patch PP directives to use stale preambles while building ASTs
kadircet updated this revision to Diff 266638.
kadircet added a comment.
- Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79992/new/
https://reviews.llvm.org/D79992
Files:
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -26,7 +26,9 @@
#include
#include
+using testing::Contains;
using testing::Field;
+using testing::MatchesRegex;
namespace clang {
namespace clangd {
@@ -181,6 +183,109 @@
ElementsAre(AllOf(Field(&Inclusion::Written, "\"a.h\""),
Field(&Inclusion::Resolved, testPath("a.h");
}
+
+llvm::Optional createPatchedAST(llvm::StringRef Baseline,
+ llvm::StringRef Modified) {
+ auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+ if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return llvm::None;
+ }
+
+ IgnoreDiagnostics Diags;
+ auto TU = TestTU::withCode(Modified);
+ auto CI = buildCompilerInvocation(TU.inputs(), Diags);
+ if (!CI) {
+ADD_FAILURE() << "Failed to build compiler invocation";
+return llvm::None;
+ }
+ return ParsedAST::build(testPath("main.cpp"), TU.inputs(), std::move(CI), {},
+ BaselinePreamble);
+}
+
+std::string getPreamblePatch(llvm::StringRef Baseline,
+ llvm::StringRef Modified) {
+ auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+ if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return "";
+ }
+ auto TU = TestTU::withCode(Modified);
+ return PreamblePatch::create(testPath("main.cpp"), TU.inputs(),
+ *BaselinePreamble)
+ .text()
+ .str();
+}
+
+TEST(PreamblePatchTest, Define) {
+ // BAR should be defined while parsing the AST.
+ struct {
+llvm::StringLiteral Contents;
+llvm::StringLiteral ExpectedPatch;
+ } Cases[] = {
+ {
+ R"cpp(
+#define BAR
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ // multiline macro
+ {
+ R"cpp(
+#define BAR \
+
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ // multiline macro
+ {
+ R"cpp(
+#define \
+BAR
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ };
+
+ for (const auto &Case : Cases) {
+SCOPED_TRACE(Case.Contents);
+Annotations Modified(Case.Contents);
+EXPECT_THAT(getPreamblePatch("", Modified.code()),
+MatchesRegex(Case.ExpectedPatch.str()));
+
+auto AST = createPatchedAST("", Modified.code());
+ASSERT_TRUE(AST);
+EXPECT_THAT(AST->getDiagnostics(),
+Not(Contains(Field(&Diag::Range, Modified.range();
+ }
+}
+
+TEST(PreamblePatchTest, OrderingPreserved) {
+ llvm::StringLiteral Baseline = "#define BAR(X) X";
+ Annotations Modified(R"cpp(
+#define BAR(X, Y) X Y
+#define BAR(X) X
+[[BAR]](int y);
+ )cpp");
+
+ llvm::StringLiteral ExpectedPatch(R"cpp(#line 0 ".*main.cpp"
+#define BAR\(X, Y\) X Y
+#define BAR\(X\) X
+)cpp");
+ EXPECT_THAT(getPreamblePatch(Baseline, Modified.code()),
+ MatchesRegex(ExpectedPatch.str()));
+
+ auto AST = createPatchedAST(Baseline, Modified.code());
+ ASSERT_TRUE(AST);
+ EXPECT_THAT(AST->getDiagnostics(),
+ Not(Contains(Field(&Diag::Range, Modified.range();
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -119,6 +119,9 @@
/// using the presumed-location mechanism.
std::vector preambleIncludes() const;
+ /// Returns textual patch contents.
+ llvm::StringRef text() const { return PatchContents; }
+
private:
PreamblePatch() = default;
std::string PatchContents;
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -15,6 +15,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
@@ -106,14 +107,70 @@
const SourceManager *SourceMgr = nullptr;
};
-/// Gets the includes in the
[PATCH] D79992: [clangd] Patch PP directives to use stale preambles while building ASTs
kadircet updated this revision to Diff 266186.
kadircet added a comment.
- Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79992/new/
https://reviews.llvm.org/D79992
Files:
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -26,7 +26,9 @@
#include
#include
+using testing::Contains;
using testing::Field;
+using testing::MatchesRegex;
namespace clang {
namespace clangd {
@@ -181,6 +183,109 @@
ElementsAre(AllOf(Field(&Inclusion::Written, "\"a.h\""),
Field(&Inclusion::Resolved, testPath("a.h");
}
+
+llvm::Optional createPatchedAST(llvm::StringRef Baseline,
+ llvm::StringRef Modified) {
+ auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+ if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return llvm::None;
+ }
+
+ IgnoreDiagnostics Diags;
+ auto TU = TestTU::withCode(Modified);
+ auto CI = buildCompilerInvocation(TU.inputs(), Diags);
+ if (!CI) {
+ADD_FAILURE() << "Failed to build compiler invocation";
+return llvm::None;
+ }
+ return ParsedAST::build(testPath("main.cpp"), TU.inputs(), std::move(CI), {},
+ BaselinePreamble);
+}
+
+std::string getPreamblePatch(llvm::StringRef Baseline,
+ llvm::StringRef Modified) {
+ auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+ if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return "";
+ }
+ auto TU = TestTU::withCode(Modified);
+ return PreamblePatch::create(testPath("main.cpp"), TU.inputs(),
+ *BaselinePreamble)
+ .text()
+ .str();
+}
+
+TEST(PreamblePatchTest, Define) {
+ // BAR should be defined while parsing the AST.
+ struct {
+llvm::StringLiteral Contents;
+llvm::StringLiteral ExpectedPatch;
+ } Cases[] = {
+ {
+ R"cpp(
+#define BAR
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ // multiline macro
+ {
+ R"cpp(
+#define BAR \
+
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ // multiline macro
+ {
+ R"cpp(
+#define \
+BAR
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ };
+
+ for (const auto &Case : Cases) {
+SCOPED_TRACE(Case.Contents);
+Annotations Modified(Case.Contents);
+EXPECT_THAT(getPreamblePatch("", Modified.code()),
+MatchesRegex(Case.ExpectedPatch.str()));
+
+auto AST = createPatchedAST("", Modified.code());
+ASSERT_TRUE(AST);
+EXPECT_THAT(AST->getDiagnostics(),
+Not(Contains(Field(&Diag::Range, Modified.range();
+ }
+}
+
+TEST(PreamblePatchTest, OrderingPreserved) {
+ llvm::StringLiteral Baseline = "#define BAR(X) X";
+ Annotations Modified(R"cpp(
+#define BAR(X, Y) X Y
+#define BAR(X) X
+[[BAR]](int y);
+ )cpp");
+
+ llvm::StringLiteral ExpectedPatch(R"cpp(#line 0 ".*main.cpp"
+#define BAR\(X, Y\) X Y
+#define BAR\(X\) X
+)cpp");
+ EXPECT_THAT(getPreamblePatch(Baseline, Modified.code()),
+ MatchesRegex(ExpectedPatch.str()));
+
+ auto AST = createPatchedAST(Baseline, Modified.code());
+ ASSERT_TRUE(AST);
+ EXPECT_THAT(AST->getDiagnostics(),
+ Not(Contains(Field(&Diag::Range, Modified.range();
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -119,6 +119,9 @@
/// using the presumed-location mechanism.
std::vector preambleIncludes() const;
+ /// Returns textual patch contents.
+ llvm::StringRef text() const { return PatchContents; }
+
private:
PreamblePatch() = default;
std::string PatchContents;
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -15,6 +15,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
@@ -106,14 +107,70 @@
const SourceManager *SourceMgr = nullptr;
};
-/// Gets the includes in the
[PATCH] D79992: [clangd] Patch PP directives to use stale preambles while building ASTs
kadircet updated this revision to Diff 265995.
kadircet marked 5 inline comments as done.
kadircet added a comment.
- Assert on patch contents, using regexes for main file match.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79992/new/
https://reviews.llvm.org/D79992
Files:
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -26,7 +26,9 @@
#include
#include
+using testing::Contains;
using testing::Field;
+using testing::MatchesRegex;
namespace clang {
namespace clangd {
@@ -181,6 +183,109 @@
ElementsAre(AllOf(Field(&Inclusion::Written, "\"a.h\""),
Field(&Inclusion::Resolved, testPath("a.h");
}
+
+llvm::Optional createPatchedAST(llvm::StringRef Baseline,
+ llvm::StringRef Modified) {
+ auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+ if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return llvm::None;
+ }
+
+ IgnoreDiagnostics Diags;
+ auto TU = TestTU::withCode(Modified);
+ auto CI = buildCompilerInvocation(TU.inputs(), Diags);
+ if (!CI) {
+ADD_FAILURE() << "Failed to build compiler invocation";
+return llvm::None;
+ }
+ return ParsedAST::build(testPath("main.cpp"), TU.inputs(), std::move(CI), {},
+ BaselinePreamble);
+}
+
+std::string getPreamblePatch(llvm::StringRef Baseline,
+ llvm::StringRef Modified) {
+ auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+ if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return "";
+ }
+ auto TU = TestTU::withCode(Modified);
+ return PreamblePatch::create(testPath("main.cpp"), TU.inputs(),
+ *BaselinePreamble)
+ .text()
+ .str();
+}
+
+TEST(PreamblePatchTest, Define) {
+ // BAR should be defined while parsing the AST.
+ struct {
+llvm::StringLiteral Contents;
+llvm::StringLiteral ExpectedPatch;
+ } Cases[] = {
+ {
+ R"cpp(
+#define BAR
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ // multiline macro
+ {
+ R"cpp(
+#define BAR \
+
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ // multiline macro
+ {
+ R"cpp(
+#define \
+BAR
+[[BAR]])cpp",
+ R"cpp(#line 0 ".*main.cpp"
+#define BAR
+)cpp",
+ },
+ };
+
+ for (const auto &Case : Cases) {
+SCOPED_TRACE(Case.Contents);
+Annotations Modified(Case.Contents);
+EXPECT_THAT(getPreamblePatch("", Modified.code()),
+MatchesRegex(Case.ExpectedPatch.str()));
+
+auto AST = createPatchedAST("", Modified.code());
+ASSERT_TRUE(AST);
+EXPECT_THAT(AST->getDiagnostics(),
+Not(Contains(Field(&Diag::Range, Modified.range();
+ }
+}
+
+TEST(PreamblePatchTest, OrderingPreserved) {
+ llvm::StringLiteral Baseline = "#define BAR(X) X";
+ Annotations Modified(R"cpp(
+#define BAR(X, Y) X Y
+#define BAR(X) X
+[[BAR]](int y);
+ )cpp");
+
+ llvm::StringLiteral ExpectedPatch(R"cpp(#line 0 ".*main.cpp"
+#define BAR\(X, Y\) X Y
+#define BAR\(X\) X
+)cpp");
+ EXPECT_THAT(getPreamblePatch(Baseline, Modified.code()),
+ MatchesRegex(ExpectedPatch.str()));
+
+ auto AST = createPatchedAST(Baseline, Modified.code());
+ ASSERT_TRUE(AST);
+ EXPECT_THAT(AST->getDiagnostics(),
+ Not(Contains(Field(&Diag::Range, Modified.range();
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -119,6 +119,9 @@
/// using the presumed-location mechanism.
std::vector preambleIncludes() const;
+ /// Returns textual patch contents.
+ llvm::StringRef text() const { return PatchContents; }
+
private:
PreamblePatch() = default;
std::string PatchContents;
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -15,6 +15,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
@@ -1
[PATCH] D79992: [clangd] Patch PP directives to use stale preambles while building ASTs
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clang-tools-extra/clangd/Preamble.cpp:110
-/// Gets the includes in the preamble section of the file by running
-/// preprocessor over \p Contents. Returned includes do not contain resolved
-/// paths. \p VFS and \p Cmd is used to build the compiler invocation, which
-/// might stat/read files.
-llvm::Expected>
-scanPreambleIncludes(llvm::StringRef Contents,
- llvm::IntrusiveRefCntPtr VFS,
- const tooling::CompileCommand &Cmd) {
+struct TextualPPDirective {
+ unsigned DirectiveLine;
this is worth a comment saying what this is used for: directives *other* than
includes where we need only basic structure.
Comment at: clang-tools-extra/clangd/unittests/PreambleTests.cpp:205
+
+TEST(PreamblePatchTest, Define) {
+ // BAR should be defined while parsing the AST.
kadircet wrote:
> sammccall wrote:
> > do you think it makes sense to have a test that just asserts on the
> > contents of the preamble patch? it seems like a more direct way to test
> > some of these things.
> >
> > These tests are nice, but debugging them seems like it might be a bit of
> > work.
> I had 2 reasons for not doing that:
> - Not clobbering the API of `PreamblePatch` just for testing.
> - Making tests less fragile for changes in the patch format.
>
> I suppose the first one is not that important, but I believe the latter is
> quite useful. We can always to something like hassubstr I suppose, but in the
> end we only care about its effect while creating an AST. We can do something
> like hassubstr, but we would still need these tests to ensure they interact
> as expected with the preprocessor.
Agree that having robust tests of the whole thing is important.
Maybe we could add an additional (sorry) single smoke test that asserts on the
whole patch?
It'd be brittle indeed but those diffs would actually be really interesting to
me as a reader of the code and reviewer of changes... gives a different
perspective on what the code is doing.
I don't think having a `text()` or `textForTests()` accessor is too bad,
especially if it's trivial.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79992/new/
https://reviews.llvm.org/D79992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79992: [clangd] Patch PP directives to use stale preambles while building ASTs
kadircet updated this revision to Diff 265194.
kadircet added a comment.
- Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79992/new/
https://reviews.llvm.org/D79992
Files:
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/PreambleTests.cpp
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -26,6 +26,7 @@
#include
#include
+using testing::Contains;
using testing::Field;
namespace clang {
@@ -181,6 +182,67 @@
ElementsAre(AllOf(Field(&Inclusion::Written, "\"a.h\""),
Field(&Inclusion::Resolved, testPath("a.h");
}
+
+llvm::Optional createPatchedAST(llvm::StringRef Baseline,
+ llvm::StringRef Modified) {
+ auto BaselinePreamble = TestTU::withCode(Baseline).preamble();
+ if (!BaselinePreamble) {
+ADD_FAILURE() << "Failed to build baseline preamble";
+return llvm::None;
+ }
+
+ IgnoreDiagnostics Diags;
+ auto TU = TestTU::withCode(Modified);
+ auto CI = buildCompilerInvocation(TU.inputs(), Diags);
+ if (!CI) {
+ADD_FAILURE() << "Failed to build compiler invocation";
+return llvm::None;
+ }
+ return ParsedAST::build(testPath("main.cpp"), TU.inputs(), std::move(CI), {},
+ BaselinePreamble);
+}
+
+TEST(PreamblePatchTest, Define) {
+ // BAR should be defined while parsing the AST.
+ llvm::StringLiteral Cases[] = {
+ R"cpp(
+#define BAR
+[[BAR]])cpp",
+ // multiline macro
+ R"cpp(
+#define BAR \
+
+[[BAR]])cpp",
+ // multiline macro
+ R"cpp(
+#define \
+BAR
+[[BAR]])cpp",
+ };
+
+ for (llvm::StringLiteral Case : Cases) {
+SCOPED_TRACE(Case);
+Annotations Modified(Case);
+auto AST = createPatchedAST("", Modified.code());
+ASSERT_TRUE(AST);
+EXPECT_THAT(AST->getDiagnostics(),
+Not(Contains(Field(&Diag::Range, Modified.range();
+ }
+}
+
+TEST(PreamblePatchTest, OrderingPreserved) {
+ llvm::StringLiteral Baseline = "#define BAR(X) X";
+ Annotations Modified(R"cpp(
+#define BAR(X, Y) X Y
+#define BAR(X) X
+[[BAR]](int y);
+ )cpp");
+
+ auto AST = createPatchedAST(Baseline, Modified.code());
+ ASSERT_TRUE(AST);
+ EXPECT_THAT(AST->getDiagnostics(),
+ Not(Contains(Field(&Diag::Range, Modified.range();
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -15,6 +15,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
@@ -106,14 +107,68 @@
const SourceManager *SourceMgr = nullptr;
};
-/// Gets the includes in the preamble section of the file by running
-/// preprocessor over \p Contents. Returned includes do not contain resolved
-/// paths. \p VFS and \p Cmd is used to build the compiler invocation, which
-/// might stat/read files.
-llvm::Expected>
-scanPreambleIncludes(llvm::StringRef Contents,
- llvm::IntrusiveRefCntPtr VFS,
- const tooling::CompileCommand &Cmd) {
+struct TextualPPDirective {
+ unsigned DirectiveLine;
+ // Full text that's representing the directive, including the `#`.
+ std::string Text;
+
+ bool operator==(const TextualPPDirective &RHS) const {
+return std::tie(DirectiveLine, Text) ==
+ std::tie(RHS.DirectiveLine, RHS.Text);
+ }
+};
+
+// Collects #define directives inside the main file.
+struct DirectiveCollector : public PPCallbacks {
+ DirectiveCollector(const Preprocessor &PP,
+ std::vector &TextualDirectives)
+ : LangOpts(PP.getLangOpts()), SM(PP.getSourceManager()),
+TextualDirectives(TextualDirectives) {}
+
+ void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+ SrcMgr::CharacteristicKind FileType,
+ FileID PrevFID) override {
+InMainFile = SM.isWrittenInMainFile(Loc);
+ }
+
+ void MacroDefined(const Token &MacroNameTok,
+const MacroDirective *MD) override {
+if (!InMainFile)
+ return;
+TextualDirectives.emplace_back();
+TextualPPDirective &TD = TextualDirectives.back();
+
+auto DecompLoc = SM.getDecomposedLoc(MacroNameTok.getLocation());
+TD.DirectiveLine = SM.getLineNumber(DecompLoc.first, DecompLoc.second);
+
+SourceRange DefRange(
+MD->getMacroInfo()->getDefin
