[clang-tools-extra] b26c88e - [clang-tidy] Store all ranges in clang::tooling::Diagnostic

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 27 10:39:55 PST 2020


Author: Joe Turner
Date: 2020-02-27T19:39:42+01:00
New Revision: b26c88e3c6e08f8f78ab4291bc85b5685241f493

URL: https://github.com/llvm/llvm-project/commit/b26c88e3c6e08f8f78ab4291bc85b5685241f493
DIFF: https://github.com/llvm/llvm-project/commit/b26c88e3c6e08f8f78ab4291bc85b5685241f493.diff

LOG: [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 <joturner at google.com>.
Differential Revision: https://reviews.llvm.org/D69782

Added: 
    

Modified: 
    clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
    clang/include/clang/Tooling/Core/Diagnostic.h
    clang/include/clang/Tooling/DiagnosticsYaml.h
    clang/lib/Tooling/Core/Diagnostic.cpp
    clang/unittests/Tooling/DiagnosticsYamlTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp b/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
index 0a2a6779c340..ceb18168636f 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 4e0feba6d7dc..123874f9ccf7 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<Replacements> 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<DiagnosticMessage, 1> &Notes, Level DiagLevel,
-             llvm::StringRef BuildDirectory);
+             llvm::StringRef BuildDirectory,
+             const SmallVector<FileByteRange, 1> &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<FileByteRange, 1> 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 366ee6f6703b..38e49645dbb8 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<clang::tooling::FileByteRange> {
+  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<clang::tooling::DiagnosticMessage> {
   static void mapping(IO &Io, clang::tooling::DiagnosticMessage &M) {
     Io.mapRequired("Message", M.Message);
@@ -58,11 +67,12 @@ template <> struct MappingTraits<clang::tooling::Diagnostic> {
 
     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<clang::tooling::Diagnostic> {
     SmallVector<clang::tooling::DiagnosticMessage, 1> Notes;
     clang::tooling::Diagnostic::Level DiagLevel;
     std::string BuildDirectory;
+    SmallVector<clang::tooling::FileByteRange, 1> Ranges;
   };
 
   static void mapping(IO &Io, clang::tooling::Diagnostic &D) {
@@ -79,6 +90,7 @@ template <> struct MappingTraits<clang::tooling::Diagnostic> {
     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 
diff erent fields.
   }

diff  --git a/clang/lib/Tooling/Core/Diagnostic.cpp b/clang/lib/Tooling/Core/Diagnostic.cpp
index 34677bf3eb6d..b0c4ea8c5608 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<DiagnosticMessage, 1> &Notes,
-                       Level DiagLevel, llvm::StringRef BuildDirectory)
+                       Level DiagLevel, llvm::StringRef BuildDirectory,
+                       const SmallVector<FileByteRange, 1> &Ranges)
     : DiagnosticName(DiagnosticName), Message(Message), Notes(Notes),
-      DiagLevel(DiagLevel), BuildDirectory(BuildDirectory) {}
+      DiagLevel(DiagLevel), BuildDirectory(BuildDirectory), Ranges(Ranges) {}
 
 const llvm::StringMap<Replacements> *selectFirstFix(const Diagnostic& D) {
    if (!D.Message.Fix.empty())

diff  --git a/clang/unittests/Tooling/DiagnosticsYamlTest.cpp b/clang/unittests/Tooling/DiagnosticsYamlTest.cpp
index 334c31732223..ef8f3ec45e64 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<Replacements> &Fix) {
+                                 const StringMap<Replacements> &Fix,
+                                 const SmallVector<FileByteRange, 1> &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<Replacements> Fix2 = {
       {"path/to/header.h",
        Replacements({"path/to/header.h", 62, 2, "replacement #2"})}};
+  SmallVector<FileByteRange, 1> 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<Replacement> Fixes3 = getFixes(D3.Message.Fix);
   EXPECT_TRUE(Fixes3.empty());
+  EXPECT_TRUE(D3.Ranges.empty());
 }


        


More information about the cfe-commits mailing list