From b26c88e3c6e08f8f78ab4291bc85b5685241f493 Mon Sep 17 00:00:00 2001 From: Joe Turner Date: Tue, 25 Feb 2020 14:56:57 +0100 Subject: [PATCH] [clang-tidy] Store all ranges in clang::tooling::Diagnostic Summary: Instead of dropping all the ranges associated with a Diagnostic when converting them to a ClangTidy error, instead attach them to the ClangTidyError, so they can be consumed by other APIs. Patch by Joe Turner . Differential Revision: https://reviews.llvm.org/D69782 --- .../ApplyReplacementsTest.cpp | 3 +- clang/include/clang/Tooling/Core/Diagnostic.h | 18 +++++++++++- clang/include/clang/Tooling/DiagnosticsYaml.h | 16 ++++++++-- clang/lib/Tooling/Core/Diagnostic.cpp | 16 ++++++++-- clang/unittests/Tooling/DiagnosticsYamlTest.cpp | 34 ++++++++++++++++++---- 5 files changed, 76 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp b/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp index 0a2a677..ceb1816 100644 --- a/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp +++ b/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp @@ -28,7 +28,8 @@ makeTUDiagnostics(const std::string &MainSourceFile, StringRef DiagnosticName, Message, {}, Diagnostic::Warning, - BuildDirectory}}}); + BuildDirectory, + {}}}}); return TUs; } diff --git a/clang/include/clang/Tooling/Core/Diagnostic.h b/clang/include/clang/Tooling/Core/Diagnostic.h index 4e0feba..123874f 100644 --- a/clang/include/clang/Tooling/Core/Diagnostic.h +++ b/clang/include/clang/Tooling/Core/Diagnostic.h @@ -47,6 +47,17 @@ struct DiagnosticMessage { llvm::StringMap Fix; }; +/// Represents a range within a specific source file. +struct FileByteRange { + FileByteRange() = default; + + FileByteRange(const SourceManager &Sources, CharSourceRange Range); + + std::string FilePath; + unsigned FileOffset; + unsigned Length; +}; + /// Represents the diagnostic with the level of severity and possible /// fixes to be applied. struct Diagnostic { @@ -62,7 +73,8 @@ struct Diagnostic { Diagnostic(llvm::StringRef DiagnosticName, const DiagnosticMessage &Message, const SmallVector &Notes, Level DiagLevel, - llvm::StringRef BuildDirectory); + llvm::StringRef BuildDirectory, + const SmallVector &Ranges); /// Name identifying the Diagnostic. std::string DiagnosticName; @@ -84,6 +96,10 @@ struct Diagnostic { /// /// Note: it is empty in unittest. std::string BuildDirectory; + + /// Extra source ranges associated with the diagnostic (in addition to the + /// location of the Message above). + SmallVector Ranges; }; /// Collection of Diagnostics generated from a single translation unit. diff --git a/clang/include/clang/Tooling/DiagnosticsYaml.h b/clang/include/clang/Tooling/DiagnosticsYaml.h index 366ee6f..38e4964 100644 --- a/clang/include/clang/Tooling/DiagnosticsYaml.h +++ b/clang/include/clang/Tooling/DiagnosticsYaml.h @@ -22,10 +22,19 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::Diagnostic) LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::DiagnosticMessage) +LLVM_YAML_IS_SEQUENCE_VECTOR(clang::tooling::FileByteRange) namespace llvm { namespace yaml { +template <> struct MappingTraits { + static void mapping(IO &Io, clang::tooling::FileByteRange &R) { + Io.mapRequired("FilePath", R.FilePath); + Io.mapRequired("FileOffset", R.FileOffset); + Io.mapRequired("Length", R.Length); + } +}; + template <> struct MappingTraits { static void mapping(IO &Io, clang::tooling::DiagnosticMessage &M) { Io.mapRequired("Message", M.Message); @@ -58,11 +67,12 @@ template <> struct MappingTraits { NormalizedDiagnostic(const IO &, const clang::tooling::Diagnostic &D) : DiagnosticName(D.DiagnosticName), Message(D.Message), Notes(D.Notes), - DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory) {} + DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory), + Ranges(D.Ranges) {} clang::tooling::Diagnostic denormalize(const IO &) { return clang::tooling::Diagnostic(DiagnosticName, Message, Notes, - DiagLevel, BuildDirectory); + DiagLevel, BuildDirectory, Ranges); } std::string DiagnosticName; @@ -71,6 +81,7 @@ template <> struct MappingTraits { SmallVector Notes; clang::tooling::Diagnostic::Level DiagLevel; std::string BuildDirectory; + SmallVector Ranges; }; static void mapping(IO &Io, clang::tooling::Diagnostic &D) { @@ -79,6 +90,7 @@ template <> struct MappingTraits { Io.mapRequired("DiagnosticName", Keys->DiagnosticName); Io.mapRequired("DiagnosticMessage", Keys->Message); Io.mapOptional("Notes", Keys->Notes); + Io.mapOptional("Ranges", Keys->Ranges); // FIXME: Export properly all the different fields. } diff --git a/clang/lib/Tooling/Core/Diagnostic.cpp b/clang/lib/Tooling/Core/Diagnostic.cpp index 34677bf..b0c4ea8 100644 --- a/clang/lib/Tooling/Core/Diagnostic.cpp +++ b/clang/lib/Tooling/Core/Diagnostic.cpp @@ -11,6 +11,7 @@ //===----------------------------------------------------------------------===// #include "clang/Tooling/Core/Diagnostic.h" +#include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "llvm/ADT/STLExtras.h" @@ -34,6 +35,16 @@ DiagnosticMessage::DiagnosticMessage(llvm::StringRef Message, FileOffset = Sources.getFileOffset(Loc); } +FileByteRange::FileByteRange( + const SourceManager &Sources, CharSourceRange Range) + : FileOffset(0), Length(0) { + FilePath = std::string(Sources.getFilename(Range.getBegin())); + if (!FilePath.empty()) { + FileOffset = Sources.getFileOffset(Range.getBegin()); + Length = Sources.getFileOffset(Range.getEnd()) - FileOffset; + } +} + Diagnostic::Diagnostic(llvm::StringRef DiagnosticName, Diagnostic::Level DiagLevel, StringRef BuildDirectory) : DiagnosticName(DiagnosticName), DiagLevel(DiagLevel), @@ -42,9 +53,10 @@ Diagnostic::Diagnostic(llvm::StringRef DiagnosticName, Diagnostic::Diagnostic(llvm::StringRef DiagnosticName, const DiagnosticMessage &Message, const SmallVector &Notes, - Level DiagLevel, llvm::StringRef BuildDirectory) + Level DiagLevel, llvm::StringRef BuildDirectory, + const SmallVector &Ranges) : DiagnosticName(DiagnosticName), Message(Message), Notes(Notes), - DiagLevel(DiagLevel), BuildDirectory(BuildDirectory) {} + DiagLevel(DiagLevel), BuildDirectory(BuildDirectory), Ranges(Ranges) {} const llvm::StringMap *selectFirstFix(const Diagnostic& D) { if (!D.Message.Fix.empty()) diff --git a/clang/unittests/Tooling/DiagnosticsYamlTest.cpp b/clang/unittests/Tooling/DiagnosticsYamlTest.cpp index 334c317..ef8f3ec 100644 --- a/clang/unittests/Tooling/DiagnosticsYamlTest.cpp +++ b/clang/unittests/Tooling/DiagnosticsYamlTest.cpp @@ -13,6 +13,7 @@ #include "clang/Tooling/DiagnosticsYaml.h" #include "clang/Tooling/Core/Diagnostic.h" #include "clang/Tooling/ReplacementsYaml.h" +#include "llvm/ADT/SmallVector.h" #include "gtest/gtest.h" using namespace llvm; @@ -30,13 +31,24 @@ static DiagnosticMessage makeMessage(const std::string &Message, int FileOffset, return DiagMessage; } +static FileByteRange makeByteRange(int FileOffset, + int Length, + const std::string &FilePath) { + FileByteRange Range; + Range.FileOffset = FileOffset; + Range.Length = Length; + Range.FilePath = FilePath; + return Range; +} + static Diagnostic makeDiagnostic(StringRef DiagnosticName, const std::string &Message, int FileOffset, const std::string &FilePath, - const StringMap &Fix) { + const StringMap &Fix, + const SmallVector &Ranges) { return Diagnostic(DiagnosticName, makeMessage(Message, FileOffset, FilePath, Fix), {}, - Diagnostic::Warning, "path/to/build/directory"); + Diagnostic::Warning, "path/to/build/directory", Ranges); } static const char *YAMLContent = @@ -63,6 +75,10 @@ static const char *YAMLContent = " Offset: 62\n" " Length: 2\n" " ReplacementText: 'replacement #2'\n" + " Ranges:\n" + " - FilePath: 'path/to/source.cpp'\n" + " FileOffset: 10\n" + " Length: 10\n" " - DiagnosticName: 'diagnostic#3'\n" " DiagnosticMessage:\n" " Message: 'message #3'\n" @@ -88,16 +104,18 @@ TEST(DiagnosticsYamlTest, serializesDiagnostics) { {"path/to/source.cpp", Replacements({"path/to/source.cpp", 100, 12, "replacement #1"})}}; TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#1", "message #1", 55, - "path/to/source.cpp", Fix1)); + "path/to/source.cpp", Fix1, {})); StringMap Fix2 = { {"path/to/header.h", Replacements({"path/to/header.h", 62, 2, "replacement #2"})}}; + SmallVector Ranges2 = + {makeByteRange(10, 10, "path/to/source.cpp")}; TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#2", "message #2", 60, - "path/to/header.h", Fix2)); + "path/to/header.h", Fix2, Ranges2)); TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#3", "message #3", 72, - "path/to/source2.cpp", {})); + "path/to/source2.cpp", {}, {})); TUD.Diagnostics.back().Notes.push_back( makeMessage("Note1", 88, "path/to/note1.cpp", {})); TUD.Diagnostics.back().Notes.push_back( @@ -142,6 +160,7 @@ TEST(DiagnosticsYamlTest, deserializesDiagnostics) { EXPECT_EQ(100u, Fixes1[0].getOffset()); EXPECT_EQ(12u, Fixes1[0].getLength()); EXPECT_EQ("replacement #1", Fixes1[0].getReplacementText()); + EXPECT_TRUE(D1.Ranges.empty()); Diagnostic D2 = TUDActual.Diagnostics[1]; EXPECT_EQ("diagnostic#2", D2.DiagnosticName); @@ -154,6 +173,10 @@ TEST(DiagnosticsYamlTest, deserializesDiagnostics) { EXPECT_EQ(62u, Fixes2[0].getOffset()); EXPECT_EQ(2u, Fixes2[0].getLength()); EXPECT_EQ("replacement #2", Fixes2[0].getReplacementText()); + EXPECT_EQ(1u, D2.Ranges.size()); + EXPECT_EQ("path/to/source.cpp", D2.Ranges[0].FilePath); + EXPECT_EQ(10u, D2.Ranges[0].FileOffset); + EXPECT_EQ(10u, D2.Ranges[0].Length); Diagnostic D3 = TUDActual.Diagnostics[2]; EXPECT_EQ("diagnostic#3", D3.DiagnosticName); @@ -169,4 +192,5 @@ TEST(DiagnosticsYamlTest, deserializesDiagnostics) { EXPECT_EQ("path/to/note2.cpp", D3.Notes[1].FilePath); std::vector Fixes3 = getFixes(D3.Message.Fix); EXPECT_TRUE(Fixes3.empty()); + EXPECT_TRUE(D3.Ranges.empty()); } -- 2.7.4