kadircet updated this revision to Diff 496907.
kadircet edited the summary of this revision.
kadircet added a comment.
- Update commit message
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143096/new/
https://reviews.llvm.org/D143096
Files:
clang-tools-extra/clangd/ParsedAST.cpp
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
@@ -724,9 +724,8 @@
#define BAR
#include [[<foo>]])");
auto AST = createPatchedAST(Code.code(), NewCode.code());
- // FIXME: We should point at the correct coordinates in NewCode.
EXPECT_THAT(*AST->getDiagnostics(),
- ElementsAre(Diag(Code.range(), "pp_file_not_found")));
+ ElementsAre(Diag(NewCode.range(), "pp_file_not_found")));
}
{
// Check with removals from preamble.
@@ -735,18 +734,15 @@
#include [[<foo>]])");
Annotations NewCode("#include [[<foo>]]");
auto AST = createPatchedAST(Code.code(), NewCode.code());
- // FIXME: We should point at the correct coordinates in NewCode.
EXPECT_THAT(*AST->getDiagnostics(),
- ElementsAre(Diag(Code.range(), "pp_file_not_found")));
+ ElementsAre(Diag(NewCode.range(), "pp_file_not_found")));
}
{
// Drop line with diags.
Annotations Code("#include [[<foo>]]");
Annotations NewCode("#define BAR\n#define BAZ\n");
auto AST = createPatchedAST(Code.code(), NewCode.code());
- // FIXME: No diagnostics.
- EXPECT_THAT(*AST->getDiagnostics(),
- ElementsAre(Diag(Code.range(), "pp_file_not_found")));
+ EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
}
{
// Picks closest line in case of multiple alternatives.
@@ -757,18 +753,79 @@
#define BAR
#include <foo>)");
auto AST = createPatchedAST(Code.code(), NewCode.code());
- // FIXME: We should point at the correct coordinates in NewCode.
EXPECT_THAT(*AST->getDiagnostics(),
- ElementsAre(Diag(Code.range(), "pp_file_not_found")));
+ ElementsAre(Diag(NewCode.range(), "pp_file_not_found")));
}
{
// Drop diag if line spelling has changed.
Annotations Code("#include [[<foo>]]");
Annotations NewCode(" # include <foo>");
auto AST = createPatchedAST(Code.code(), NewCode.code());
- // FIXME: No diags.
+ EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
+ }
+ {
+ // Multiple lines.
+ Annotations Code(R"(
+#define BAR
+#include [[<fo\
+o>]])");
+ Annotations NewCode(R"(#include [[<fo\
+o>]])");
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
EXPECT_THAT(*AST->getDiagnostics(),
- ElementsAre(Diag(Code.range(), "pp_file_not_found")));
+ ElementsAre(Diag(NewCode.range(), "pp_file_not_found")));
+ }
+ {
+ // Multiple lines with change.
+ Annotations Code(R"(
+#define BAR
+#include <fox>
+#include [[<fo\
+o>]])");
+ Annotations NewCode(R"(#include <fo\
+x>)");
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
+ }
+ {
+ // Preserves notes.
+ Annotations Code(R"(
+#define $note[[BAR]] 1
+#define $main[[BAR]] 2)");
+ Annotations NewCode(R"(
+#define BAZ 0
+#define $note[[BAR]] 1
+#define BAZ 0
+#define $main[[BAR]] 2)");
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(
+ *AST->getDiagnostics(),
+ ElementsAre(AllOf(Diag(NewCode.range("main"), "-Wmacro-redefined"),
+ withNote(Diag(NewCode.range("note"))))));
+ }
+ {
+ // Preserves diag without note.
+ Annotations Code(R"(
+#define $note[[BAR]] 1
+#define $main[[BAR]] 2)");
+ Annotations NewCode(R"(
+#define $main[[BAR]] 2)");
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(
+ *AST->getDiagnostics(),
+ ElementsAre(AllOf(Diag(NewCode.range("main"), "-Wmacro-redefined"),
+ Field(&Diag::Notes, IsEmpty()))));
+ }
+ {
+ // Note is also dropped if diag is gone.
+ Annotations Code(R"(
+#define $note[[BAR]] 1
+#define $main[[BAR]] 2)");
+ Annotations NewCode(R"(
+#define BAZ 0
+#define BAR 1)");
+ auto AST = createPatchedAST(Code.code(), NewCode.code());
+ EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
}
}
} // namespace
Index: clang-tools-extra/clangd/Preamble.h
===================================================================
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -34,6 +34,7 @@
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Lex/Lexer.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/StringRef.h"
#include <memory>
@@ -157,6 +158,8 @@
/// Whether diagnostics generated using this patch are trustable.
bool preserveDiagnostics() const;
+ /// Returns diag locations for Modified contents.
+ llvm::ArrayRef<Diag> patchedDiags() const { return PatchedDiags; }
static constexpr llvm::StringLiteral HeaderName = "__preamble_patch__.h";
private:
@@ -168,9 +171,11 @@
PreamblePatch() = default;
std::string PatchContents;
std::string PatchFileName;
- /// Includes that are present in both \p Baseline and \p Modified. Used for
- /// patching includes of baseline preamble.
+ // Includes that are present in both Baseline and Modified. Used for
+ // patching includes of baseline preamble.
std::vector<Inclusion> PreambleIncludes;
+ // Diags that were attached to a line preserved in Modified contents.
+ std::vector<Diag> PatchedDiags;
PreambleBounds ModifiedBounds = {0, false};
};
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -9,7 +9,9 @@
#include "Preamble.h"
#include "Compiler.h"
#include "Config.h"
+#include "Diagnostics.h"
#include "Headers.h"
+#include "Protocol.h"
#include "SourceCode.h"
#include "clang-include-cleaner/Record.h"
#include "support/Logger.h"
@@ -35,6 +37,9 @@
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
@@ -43,8 +48,8 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Support/raw_ostream.h"
-#include <iterator>
#include <memory>
+#include <optional>
#include <string>
#include <system_error>
#include <utility>
@@ -306,6 +311,8 @@
struct ScannedPreamble {
std::vector<Inclusion> Includes;
std::vector<TextualPPDirective> TextualDirectives;
+ // Literal lines of the preamble contents.
+ std::vector<llvm::StringRef> Lines;
PreambleBounds Bounds = {0, false};
};
@@ -332,7 +339,7 @@
if (!CI)
return error("failed to create compiler invocation");
CI->getDiagnosticOpts().IgnoreWarnings = true;
- auto ContentsBuffer = llvm::MemoryBuffer::getMemBuffer(Contents);
+ auto ContentsBuffer = llvm::MemoryBuffer::getMemBuffer(PI.Contents);
// This means we're scanning (though not preprocessing) the preamble section
// twice. However, it's important to precisely follow the preamble bounds used
// elsewhere.
@@ -363,6 +370,7 @@
return std::move(Err);
Action.EndSourceFile();
SP.Includes = std::move(Includes.MainFileIncludes);
+ llvm::append_range(SP.Lines, llvm::split(Contents, "\n"));
return SP;
}
@@ -612,6 +620,113 @@
}
}
+// Translate diagnostics from baseline into modified for the lines that have the
+// same spelling.
+static std::vector<Diag> patchDiags(llvm::ArrayRef<Diag> BaselineDiags,
+ const ScannedPreamble &BaselineScan,
+ const ScannedPreamble &ModifiedScan) {
+ std::vector<Diag> PatchedDiags;
+ if (BaselineDiags.empty())
+ return PatchedDiags;
+ llvm::StringMap<llvm::SmallVector<int>> ModifiedContentsToLine;
+ for (int Line = 0, E = ModifiedScan.Lines.size(); Line != E; ++Line) {
+ llvm::StringRef Contents = ModifiedScan.Lines[Line];
+ ModifiedContentsToLine[Contents].push_back(Line);
+ }
+ // Translates a range from baseline preamble contents to modified preamble
+ // contents.
+ // Finds the consecutive set of lines that corresponds to the same contents in
+ // baseline and modified, and applies the same translation to the range.
+ // Returns nullopt if no exact line-based match is found.
+ auto TranslateRange = [&](const Range &R) -> std::optional<Range> {
+ int BaselineStart = R.start.line;
+ int BaselineEnd = R.end.line;
+ auto ModifiedStartIt =
+ ModifiedContentsToLine.find(BaselineScan.Lines[BaselineStart]);
+ if (ModifiedStartIt == ModifiedContentsToLine.end())
+ return std::nullopt;
+ int Closest = ModifiedStartIt->second.front();
+ for (auto AlternateLine : ModifiedStartIt->second) {
+ if (abs(AlternateLine - BaselineStart) < abs(Closest - BaselineStart))
+ Closest = AlternateLine;
+ }
+ for (int I = 1; I <= BaselineEnd - BaselineStart; ++I) {
+ if (BaselineScan.Lines[BaselineStart + I] !=
+ ModifiedScan.Lines[Closest + I])
+ return std::nullopt;
+ }
+ Range NewRange = R;
+ NewRange.start.line = Closest;
+ NewRange.end.line = Closest + BaselineEnd - BaselineStart;
+ return NewRange;
+ };
+ // Translates a Note by patching its range when inside main file.
+ auto TranslateNote = [&](const Note &N) -> std::optional<Note> {
+ if (!N.InsideMainFile)
+ return N;
+ if (auto NewRange = TranslateRange(N.Range)) {
+ Note NewN = N;
+ NewN.Range = std::move(*NewRange);
+ return NewN;
+ }
+ return std::nullopt;
+ };
+ // Tries to translate all the edit ranges inside the fix. Returns a fix with
+ // translated ranges on success.
+ auto TranslateFix = [&](const Fix &F) -> std::optional<Fix> {
+ Fix NewFix;
+ for (auto &E : F.Edits) {
+ auto NewRange = TranslateRange(E.range);
+ if (!NewRange.has_value())
+ return std::nullopt;
+ NewFix.Edits.emplace_back(E);
+ NewFix.Edits.back().range = *NewRange;
+ }
+ NewFix.Message = F.Message;
+ return NewFix;
+ };
+ // Translate diagnostic by moving its main range to new location (if inside
+ // the main file). Preserve all the notes and fixes that can be translated to
+ // modified contents.
+ // Drops the whole diagnostic if main range can't be patched.
+ auto TranslateDiag = [&](const Diag &D) -> std::optional<Diag> {
+ Range NewRange;
+ // Patch range if it's inside main file.
+ if (D.InsideMainFile) {
+ auto PatchedRange = TranslateRange(D.Range);
+ // Drop the diagnostic if we couldn't patch the range.
+ if (!PatchedRange.has_value())
+ return std::nullopt;
+ NewRange = *PatchedRange;
+ } else {
+ NewRange = D.Range;
+ }
+
+ Diag NewDiag;
+ // Copy all fields but Notes, Fixes, Name and Tags.
+ static_cast<DiagBase &>(NewDiag) = static_cast<DiagBase>(D);
+ NewDiag.Range = NewRange;
+ // Only keep Notes and Fixes that are still relevant.
+ for (auto &N : D.Notes) {
+ if (auto NewN = TranslateNote(N))
+ NewDiag.Notes.emplace_back(std::move(*NewN));
+ }
+ for (auto &F : D.Fixes) {
+ if (auto NewF = TranslateFix(F))
+ NewDiag.Fixes.emplace_back(std::move(*NewF));
+ }
+ // Name and Tags are not positional.
+ NewDiag.Name = D.Name;
+ NewDiag.Tags = D.Tags;
+ return NewDiag;
+ };
+ for (auto &D : BaselineDiags) {
+ if (auto NewD = TranslateDiag(D))
+ PatchedDiags.emplace_back(std::move(*NewD));
+ }
+ return PatchedDiags;
+}
+
PreamblePatch PreamblePatch::create(llvm::StringRef FileName,
const ParseInputs &Modified,
const PreambleData &Baseline,
@@ -629,7 +744,7 @@
// there's nothing to do but generate an empty patch.
auto BaselineScan = scanPreamble(
// Contents needs to be null-terminated.
- Baseline.Preamble.getContents().str(), Modified.CompileCommand);
+ Baseline.Preamble.getContents(), Modified.CompileCommand);
if (!BaselineScan) {
elog("Failed to scan baseline of {0}: {1}", FileName,
BaselineScan.takeError());
@@ -730,6 +845,8 @@
Patch << TD.Text << '\n';
}
}
+
+ PP.PatchedDiags = patchDiags(Baseline.Diags, *BaselineScan, *ModifiedScan);
dlog("Created preamble patch: {0}", Patch.str());
Patch.flush();
return PP;
@@ -771,6 +888,7 @@
PreamblePatch PP;
PP.PreambleIncludes = Preamble.Includes.MainFileIncludes;
PP.ModifiedBounds = Preamble.Preamble.getBounds();
+ PP.PatchedDiags = Preamble.Diags;
return PP;
}
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -675,8 +675,7 @@
Diags = CompilerInvocationDiags;
// Add diagnostics from the preamble, if any.
if (Preamble)
- Diags->insert(Diags->end(), Preamble->Diags.begin(),
- Preamble->Diags.end());
+ llvm::append_range(*Diags, Patch->patchedDiags());
// Finally, add diagnostics coming from the AST.
{
std::vector<Diag> D = ASTDiags.take(&*CTContext);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
