qdelacru created this revision.
qdelacru added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
qdelacru requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Fixes https://github.com/clangd/clangd/issues/819
SourceLocation of macros change when a header file is included above it. This
is not checked when creating a PreamblePatch, resulting in reusing previously
built preamble with an incorrect source location for the macro in the example
test case.
This patch stores the SourceLocation in the struct TextualPPDirective so that
it gets checked when comparing old vs new preambles.
Also creates a preamble patch for code completion parsing so that clangd does
not crash when following the example test case with a large file.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D108045
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.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
@@ -546,6 +546,14 @@
// Ensure they are dropeed when a patched preamble is used.
EXPECT_FALSE(createPatchedAST("", Code)->getDiagnostics());
}
+
+TEST(PreamblePatch, MacroLoc) {
+ llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+ llvm::StringLiteral Modified =
+ "#include \"header.h\"\n#define MACRO 12\nint num = MACRO;";
+ auto AST = createPatchedAST(Baseline, Modified);
+ ASSERT_TRUE(AST);
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3260,6 +3260,23 @@
EXPECT_THAT(Results.Completions, UnorderedElementsAre(Labeled("BarExt")));
}
}
+
+TEST(CompletionTest, PreambleCodeComplete) {
+ llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+ llvm::StringLiteral ModifiedCC =
+ "#include \"header.h\"\n#define MACRO 12\nint num = MACRO; int num2 =
M^";
+
+ Annotations Test(ModifiedCC);
+ auto BaselineTU = TestTU::withCode(Baseline);
+ auto ModifiedTU = TestTU::withCode(Test.code());
+
+ MockFS FS;
+ auto Inputs = ModifiedTU.inputs(FS);
+ auto result = codeComplete(testPath(ModifiedTU.Filename), Test.point(),
+ BaselineTU.preamble().get(), Inputs, {});
+ EXPECT_THAT(result.Completions, Not(testing::IsEmpty()));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -142,10 +142,11 @@
unsigned DirectiveLine;
// Full text that's representing the directive, including the `#`.
std::string Text;
+ SourceLocation Loc;
bool operator==(const TextualPPDirective &RHS) const {
- return std::tie(DirectiveLine, Text) ==
- std::tie(RHS.DirectiveLine, RHS.Text);
+ return std::tie(DirectiveLine, Text, Loc) ==
+ std::tie(RHS.DirectiveLine, RHS.Text, RHS.Loc);
}
};
@@ -213,6 +214,7 @@
TextualPPDirective &TD = TextualDirectives.back();
const auto *MI = MD->getMacroInfo();
+ TD.Loc = MI->getDefinitionLoc();
TD.Text =
spellDirective("#define ",
CharSourceRange::getTokenRange(
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1865,10 +1865,11 @@
return (!Preamble || Opts.RunParser == CodeCompleteOptions::NeverParse)
? std::move(Flow).runWithoutSema(ParseInput.Contents, *Offset,
*ParseInput.TFS)
- : std::move(Flow).run({FileName, *Offset, *Preamble,
- // We want to serve code completions with
- // low latency, so don't bother patching.
- /*PreamblePatch=*/llvm::None, ParseInput});
+ : std::move(Flow).run(
+ {FileName, *Offset, *Preamble,
+ /*PreamblePatch=*/
+ PreamblePatch::create(FileName, ParseInput, *Preamble),
+ ParseInput});
}
SignatureHelp signatureHelp(PathRef FileName, Position Pos,
Index: clang-tools-extra/clangd/unittests/PreambleTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/PreambleTests.cpp
+++ clang-tools-extra/clangd/unittests/PreambleTests.cpp
@@ -546,6 +546,14 @@
// Ensure they are dropeed when a patched preamble is used.
EXPECT_FALSE(createPatchedAST("", Code)->getDiagnostics());
}
+
+TEST(PreamblePatch, MacroLoc) {
+ llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+ llvm::StringLiteral Modified =
+ "#include \"header.h\"\n#define MACRO 12\nint num = MACRO;";
+ auto AST = createPatchedAST(Baseline, Modified);
+ ASSERT_TRUE(AST);
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3260,6 +3260,23 @@
EXPECT_THAT(Results.Completions, UnorderedElementsAre(Labeled("BarExt")));
}
}
+
+TEST(CompletionTest, PreambleCodeComplete) {
+ llvm::StringLiteral Baseline = "\n#define MACRO 12\nint num = MACRO;";
+ llvm::StringLiteral ModifiedCC =
+ "#include \"header.h\"\n#define MACRO 12\nint num = MACRO; int num2 = M^";
+
+ Annotations Test(ModifiedCC);
+ auto BaselineTU = TestTU::withCode(Baseline);
+ auto ModifiedTU = TestTU::withCode(Test.code());
+
+ MockFS FS;
+ auto Inputs = ModifiedTU.inputs(FS);
+ auto result = codeComplete(testPath(ModifiedTU.Filename), Test.point(),
+ BaselineTU.preamble().get(), Inputs, {});
+ EXPECT_THAT(result.Completions, Not(testing::IsEmpty()));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -142,10 +142,11 @@
unsigned DirectiveLine;
// Full text that's representing the directive, including the `#`.
std::string Text;
+ SourceLocation Loc;
bool operator==(const TextualPPDirective &RHS) const {
- return std::tie(DirectiveLine, Text) ==
- std::tie(RHS.DirectiveLine, RHS.Text);
+ return std::tie(DirectiveLine, Text, Loc) ==
+ std::tie(RHS.DirectiveLine, RHS.Text, RHS.Loc);
}
};
@@ -213,6 +214,7 @@
TextualPPDirective &TD = TextualDirectives.back();
const auto *MI = MD->getMacroInfo();
+ TD.Loc = MI->getDefinitionLoc();
TD.Text =
spellDirective("#define ",
CharSourceRange::getTokenRange(
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1865,10 +1865,11 @@
return (!Preamble || Opts.RunParser == CodeCompleteOptions::NeverParse)
? std::move(Flow).runWithoutSema(ParseInput.Contents, *Offset,
*ParseInput.TFS)
- : std::move(Flow).run({FileName, *Offset, *Preamble,
- // We want to serve code completions with
- // low latency, so don't bother patching.
- /*PreamblePatch=*/llvm::None, ParseInput});
+ : std::move(Flow).run(
+ {FileName, *Offset, *Preamble,
+ /*PreamblePatch=*/
+ PreamblePatch::create(FileName, ParseInput, *Preamble),
+ ParseInput});
}
SignatureHelp signatureHelp(PathRef FileName, Position Pos,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
