Author: Kadir Cetinkaya Date: 2023-02-22T15:54:15+01:00 New Revision: 8c2a12f7f9b6dc078bfb18df9333379fdf27c6a3
URL: https://github.com/llvm/llvm-project/commit/8c2a12f7f9b6dc078bfb18df9333379fdf27c6a3 DIFF: https://github.com/llvm/llvm-project/commit/8c2a12f7f9b6dc078bfb18df9333379fdf27c6a3.diff LOG: [clangd] Provide patched diagnostics with preamble patch Translates diagnostics from baseline preamble to relevant modified contents. Translation is done by looking for a set of lines that have the same contents in diagnostic/note/fix ranges inside baseline and modified contents. A diagnostic is preserved if its main range is outside of main file or there's a translation from baseline to modified contents. Later on fixes and notes attached to that diagnostic with relevant ranges are also translated and preserved. Depends on D143095 Differential Revision: https://reviews.llvm.org/D143096 Added: Modified: 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 Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index bf639a6fb58e..54a0f979c730 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -675,8 +675,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, 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); diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index 6a7efcd255d2..e97564497954 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/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,9 @@ #include "llvm/Support/Path.h" #include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/raw_ostream.h" -#include <iterator> +#include <cstddef> #include <memory> +#include <optional> #include <string> #include <system_error> #include <utility> @@ -306,6 +312,8 @@ struct DirectiveCollector : public PPCallbacks { 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 +340,7 @@ scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) { 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 +371,7 @@ scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) { return std::move(Err); Action.EndSourceFile(); SP.Includes = std::move(Includes.MainFileIncludes); + llvm::append_range(SP.Lines, llvm::split(Contents, "\n")); return SP; } @@ -465,6 +474,93 @@ class TimerFS : public llvm::vfs::ProxyFileSystem { WallTimer Timer; }; +// Helpers for patching diagnostics between two versions of file contents. +class DiagPatcher { + llvm::ArrayRef<llvm::StringRef> OldLines; + llvm::ArrayRef<llvm::StringRef> CurrentLines; + llvm::StringMap<llvm::SmallVector<int>> CurrentContentsToLine; + + // Translates a range from old lines to current lines. + // Finds the consecutive set of lines that corresponds to the same contents in + // old and current, and applies the same translation to the range. + // Returns true if translation succeeded. + bool translateRange(Range &R) { + int OldStart = R.start.line; + int OldEnd = R.end.line; + assert(OldStart <= OldEnd); + + size_t RangeLen = OldEnd - OldStart + 1; + auto RangeContents = OldLines.slice(OldStart).take_front(RangeLen); + // Make sure the whole range is covered in old contents. + if (RangeContents.size() < RangeLen) + return false; + + std::optional<int> Closest; + for (int AlternateLine : CurrentContentsToLine.lookup(RangeContents[0])) { + // Check if AlternateLine matches all lines in the range. + if (RangeContents != + CurrentLines.slice(AlternateLine).take_front(RangeLen)) + continue; + int Delta = AlternateLine - OldStart; + if (!Closest.has_value() || abs(Delta) < abs(*Closest)) + Closest = Delta; + } + // Couldn't find any viable matches in the current contents. + if (!Closest.has_value()) + return false; + R.start.line += *Closest; + R.end.line += *Closest; + return true; + } + + // Translates a Note by patching its range when inside main file. Returns true + // on success. + bool translateNote(Note &N) { + if (!N.InsideMainFile) + return true; + if (translateRange(N.Range)) + return true; + return false; + } + + // Tries to translate all the edit ranges inside the fix. Returns true on + // success. On failure fixes might be in an invalid state. + bool translateFix(Fix &F) { + return llvm::all_of( + F.Edits, [this](TextEdit &E) { return translateRange(E.range); }); + } + +public: + DiagPatcher(llvm::ArrayRef<llvm::StringRef> OldLines, + llvm::ArrayRef<llvm::StringRef> CurrentLines) { + this->OldLines = OldLines; + this->CurrentLines = CurrentLines; + for (int Line = 0, E = CurrentLines.size(); Line != E; ++Line) { + llvm::StringRef Contents = CurrentLines[Line]; + CurrentContentsToLine[Contents].push_back(Line); + } + } + // 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 + // new contents. + // Drops the whole diagnostic if main range can't be patched. + std::optional<Diag> translateDiag(const Diag &D) { + Range NewRange = D.Range; + // Patch range if it's inside main file. + if (D.InsideMainFile && !translateRange(NewRange)) { + // Drop the diagnostic if we couldn't patch the range. + return std::nullopt; + } + + Diag NewD = D; + NewD.Range = NewRange; + // Translate ranges inside notes and fixes too, dropping the ones that are + // no longer relevant. + llvm::erase_if(NewD.Notes, [this](Note &N) { return !translateNote(N); }); + llvm::erase_if(NewD.Fixes, [this](Fix &F) { return !translateFix(F); }); + return NewD; + } +}; } // namespace std::shared_ptr<const PreambleData> @@ -612,6 +708,22 @@ void escapeBackslashAndQuotes(llvm::StringRef Text, llvm::raw_ostream &OS) { } } +// 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; + DiagPatcher Patcher(BaselineScan.Lines, ModifiedScan.Lines); + for (auto &D : BaselineDiags) { + if (auto NewD = Patcher.translateDiag(D)) + PatchedDiags.emplace_back(std::move(*NewD)); + } + return PatchedDiags; +} + PreamblePatch PreamblePatch::create(llvm::StringRef FileName, const ParseInputs &Modified, const PreambleData &Baseline, @@ -629,7 +741,7 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName, // 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 +842,8 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName, Patch << TD.Text << '\n'; } } + + PP.PatchedDiags = patchDiags(Baseline.Diags, *BaselineScan, *ModifiedScan); dlog("Created preamble patch: {0}", Patch.str()); Patch.flush(); return PP; @@ -771,6 +885,7 @@ PreamblePatch PreamblePatch::unmodified(const PreambleData &Preamble) { PreamblePatch PP; PP.PreambleIncludes = Preamble.Includes.MainFileIncludes; PP.ModifiedBounds = Preamble.Preamble.getBounds(); + PP.PatchedDiags = Preamble.Diags; return PP; } diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index 8bc65331f3e0..b5a5c107ab48 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/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,9 @@ class PreamblePatch { /// 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 +172,11 @@ class PreamblePatch { 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}; }; diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp index 2425e430ae6e..94870abf1569 100644 --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -723,9 +723,8 @@ TEST(PreamblePatch, TranslatesDiagnosticsInPreamble) { #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. @@ -734,18 +733,15 @@ TEST(PreamblePatch, TranslatesDiagnosticsInPreamble) { #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. @@ -756,18 +752,79 @@ TEST(PreamblePatch, TranslatesDiagnosticsInPreamble) { #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())))); + } + { + // Make sure orphaned notes are not promoted to diags. + 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
