[clang] [Clang] [NFC] Add "human" diagnostic argument format (PR #115835)

Boaz Brickner via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 12 04:30:20 PST 2024


https://github.com/bricknerb updated https://github.com/llvm/llvm-project/pull/115835

>From ccf18e49f884b4430dff70d59fb3236259661f9d Mon Sep 17 00:00:00 2001
From: Boaz Brickner <brickner at google.com>
Date: Tue, 12 Nov 2024 10:19:58 +0100
Subject: [PATCH 1/3] [Clang] [NFC] Add "human" diagnostic argument format This
 allows formatting large integers in a human friendly way. Example: "5321584"
 -> "5.32M". Use it where such human numbers are generated manually today.

---
 clang/docs/InternalsManual.rst                | 12 +++++++
 .../clang/Basic/DiagnosticCommonKinds.td      | 12 +++----
 clang/lib/Basic/Diagnostic.cpp                | 31 ++++++++++++++++
 clang/lib/Basic/SourceManager.cpp             | 35 +++----------------
 .../TableGen/ClangDiagnosticsEmitter.cpp      |  7 +++-
 5 files changed, 59 insertions(+), 38 deletions(-)

diff --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst
index b70ec35f3da229..e7dd41c8595bd4 100644
--- a/clang/docs/InternalsManual.rst
+++ b/clang/docs/InternalsManual.rst
@@ -315,6 +315,18 @@ Description:
   than ``1`` are not supported.  This formatter is currently hard-coded to use
   English ordinals.
 
+**"human" format**
+
+Example:
+  ``"total size is %human0 bytes"``
+Class:
+  Integers
+Description:
+  This is a formatter which represents the argument number in a human readable
+  format: the value ``123`` stays ``123``, ``12345`` becomes ``12.34k``,
+  ``6666666` becomes ``6.67M``, and so on for 'G' and 'T'.  Values less than
+  ``0`` are not supported.
+
 **"objcclass" format**
 
 Example:
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 457abea0b81471..0c131166aff28d 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -389,14 +389,14 @@ def remark_sloc_usage : Remark<
   "source manager location address space usage:">,
   InGroup<DiagGroup<"sloc-usage">>, DefaultRemark, ShowInSystemHeader;
 def note_total_sloc_usage : Note<
-  "%0B (%1B) in local locations, %2B (%3B) "
-  "in locations loaded from AST files, for a total of %4B (%5B) "
-  "(%6%% of available space)">;
+  "%0B (%human0B) in local locations, %1B (%human1B) "
+  "in locations loaded from AST files, for a total of %2B (%human2B) "
+  "(%3%% of available space)">;
 def note_file_sloc_usage : Note<
-  "file entered %0 time%s0 using %1B (%2B) of space"
-  "%plural{0:|: plus %3B (%4B) for macro expansions}3">;
+  "file entered %0 time%s0 using %1B (%human1B) of space"
+  "%plural{0:|: plus %2B (%human2B) for macro expansions}2">;
 def note_file_misc_sloc_usage : Note<
-  "%0 additional files entered using a total of %1B (%2B) of space">;
+  "%0 additional files entered using a total of %1B (%human1B) of space">;
 
 // Modules
 def err_module_format_unhandled : Error<
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index e23362fc7af00d..9a3f2aec5df6fa 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -652,6 +652,33 @@ static void HandleOrdinalModifier(unsigned ValNo,
   Out << ValNo << llvm::getOrdinalSuffix(ValNo);
 }
 
+// 123 -> "123".
+// 1234 -> "1.23k".
+// 123456 -> "123.46k".
+// 1234567 -> "1.23M".
+// 1234567890 -> "1.23G".
+// 1234567890123 -> "1.23T".
+static void HandleIntegerHumanModifier(unsigned ValNo,
+                                       SmallVectorImpl<char> &OutStr) {
+  static constexpr std::array<std::pair<uint64_t, char>, 4> Units = {
+      {{1'000'000'000'000UL, 'T'},
+       {1'000'000'000UL, 'G'},
+       {1'000'000UL, 'M'},
+       {1'000UL, 'k'}}};
+
+  llvm::raw_svector_ostream Out(OutStr);
+  for (const auto &[UnitSize, UnitSign] : Units) {
+    if (ValNo >= UnitSize) {
+      // Out << llvm::formatv("{0:F}{1}", ValNo / static_cast<double>(UnitSize),
+                          //  UnitSign);
+      Out << llvm::format("%0.2f%c", ValNo / static_cast<double>(UnitSize),
+                          UnitSign);
+      return;
+    }
+  }
+  Out << ValNo;
+}
+
 /// PluralNumber - Parse an unsigned integer and advance Start.
 static unsigned PluralNumber(const char *&Start, const char *End) {
   // Programming 101: Parse a decimal number :-)
@@ -988,6 +1015,8 @@ FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
                              OutStr);
       } else if (ModifierIs(Modifier, ModifierLen, "ordinal")) {
         HandleOrdinalModifier((unsigned)Val, OutStr);
+      } else if (ModifierIs(Modifier, ModifierLen, "human")) {
+        HandleIntegerHumanModifier(Val, OutStr);
       } else {
         assert(ModifierLen == 0 && "Unknown integer modifier");
         llvm::raw_svector_ostream(OutStr) << Val;
@@ -1006,6 +1035,8 @@ FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
                              OutStr);
       } else if (ModifierIs(Modifier, ModifierLen, "ordinal")) {
         HandleOrdinalModifier(Val, OutStr);
+      } else if (ModifierIs(Modifier, ModifierLen, "human")) {
+        HandleIntegerHumanModifier(Val, OutStr);
       } else {
         assert(ModifierLen == 0 && "Unknown integer modifier");
         llvm::raw_svector_ostream(OutStr) << Val;
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index af60348ef26555..9b983cefbcf4ca 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -29,7 +29,6 @@
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -2228,28 +2227,6 @@ LLVM_DUMP_METHOD void SourceManager::dump() const {
   }
 }
 
-// 123 -> "123".
-// 1234 -> "1.23k".
-// 123456 -> "123.46k".
-// 1234567 -> "1.23M".
-// 1234567890 -> "1.23G".
-// 1234567890123 -> "1.23T".
-static std::string humanizeNumber(uint64_t Number) {
-  static constexpr std::array<std::pair<uint64_t, char>, 4> Units = {
-      {{1'000'000'000'000UL, 'T'},
-       {1'000'000'000UL, 'G'},
-       {1'000'000UL, 'M'},
-       {1'000UL, 'k'}}};
-
-  for (const auto &[UnitSize, UnitSign] : Units) {
-    if (Number >= UnitSize) {
-      return llvm::formatv("{0:F}{1}", Number / static_cast<double>(UnitSize),
-                           UnitSign);
-    }
-  }
-  return std::to_string(Number);
-}
-
 void SourceManager::noteSLocAddressSpaceUsage(
     DiagnosticsEngine &Diag, std::optional<unsigned> MaxNotes) const {
   struct Info {
@@ -2319,9 +2296,8 @@ void SourceManager::noteSLocAddressSpaceUsage(
   int UsagePercent = static_cast<int>(100.0 * double(LocalUsage + LoadedUsage) /
                                       MaxLoadedOffset);
   Diag.Report(SourceLocation(), diag::note_total_sloc_usage)
-      << LocalUsage << humanizeNumber(LocalUsage) << LoadedUsage
-      << humanizeNumber(LoadedUsage) << (LocalUsage + LoadedUsage)
-      << humanizeNumber(LocalUsage + LoadedUsage) << UsagePercent;
+      << LocalUsage << LoadedUsage << (LocalUsage + LoadedUsage)
+      << UsagePercent;
 
   // Produce notes on sloc address space usage for each file with a high usage.
   uint64_t ReportedSize = 0;
@@ -2329,17 +2305,14 @@ void SourceManager::noteSLocAddressSpaceUsage(
        llvm::make_range(SortedUsage.begin(), SortedEnd)) {
     Diag.Report(FileInfo.Loc, diag::note_file_sloc_usage)
         << FileInfo.Inclusions << FileInfo.DirectSize
-        << humanizeNumber(FileInfo.DirectSize)
-        << (FileInfo.TotalSize - FileInfo.DirectSize)
-        << humanizeNumber(FileInfo.TotalSize - FileInfo.DirectSize);
+        << (FileInfo.TotalSize - FileInfo.DirectSize);
     ReportedSize += FileInfo.TotalSize;
   }
 
   // Describe any remaining usage not reported in the per-file usage.
   if (ReportedSize != CountedSize) {
     Diag.Report(SourceLocation(), diag::note_file_misc_sloc_usage)
-        << (SortedUsage.end() - SortedEnd) << CountedSize - ReportedSize
-        << humanizeNumber(CountedSize - ReportedSize);
+        << (SortedUsage.end() - SortedEnd) << CountedSize - ReportedSize;
   }
 }
 
diff --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
index 34e2e8f47ae71a..8f0256d5262998 100644
--- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -440,6 +440,7 @@ enum ModifierType {
   MT_Plural,
   MT_Diff,
   MT_Ordinal,
+  MT_Human,
   MT_S,
   MT_Q,
   MT_ObjCClass,
@@ -458,6 +459,8 @@ static StringRef getModifierName(ModifierType MT) {
     return "plural";
   case MT_Ordinal:
     return "ordinal";
+  case MT_Human:
+    return "human";
   case MT_S:
     return "s";
   case MT_Q:
@@ -1053,6 +1056,7 @@ Piece *DiagnosticTextBuilder::DiagText::parseDiagText(StringRef &Text,
                                .Case("plural", MT_Plural)
                                .Case("s", MT_S)
                                .Case("ordinal", MT_Ordinal)
+                               .Case("human", MT_Human)
                                .Case("q", MT_Q)
                                .Case("objcclass", MT_ObjCClass)
                                .Case("objcinstance", MT_ObjCInstance)
@@ -1154,7 +1158,8 @@ Piece *DiagnosticTextBuilder::DiagText::parseDiagText(StringRef &Text,
     case MT_Placeholder:
     case MT_ObjCClass:
     case MT_ObjCInstance:
-    case MT_Ordinal: {
+    case MT_Ordinal:
+    case MT_Human: {
       Parsed.push_back(New<PlaceholderPiece>(ModType, parseModifier(Text)));
       continue;
     }

>From 5f5772dc66eecfd6b4917b16b7abec9e1fe1406d Mon Sep 17 00:00:00 2001
From: Boaz Brickner <brickner at google.com>
Date: Tue, 12 Nov 2024 10:26:41 +0100
Subject: [PATCH 2/3] Remove commented out code.

---
 clang/lib/Basic/Diagnostic.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 9a3f2aec5df6fa..c556758eba6876 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -669,8 +669,6 @@ static void HandleIntegerHumanModifier(unsigned ValNo,
   llvm::raw_svector_ostream Out(OutStr);
   for (const auto &[UnitSize, UnitSign] : Units) {
     if (ValNo >= UnitSize) {
-      // Out << llvm::formatv("{0:F}{1}", ValNo / static_cast<double>(UnitSize),
-                          //  UnitSign);
       Out << llvm::format("%0.2f%c", ValNo / static_cast<double>(UnitSize),
                           UnitSign);
       return;

>From 76a19b31bbd3342ceaea19dc24fb086a328a1889 Mon Sep 17 00:00:00 2001
From: Boaz Brickner <brickner at google.com>
Date: Tue, 12 Nov 2024 13:30:06 +0100
Subject: [PATCH 3/3] Support negative numbers in %human format

---
 clang/docs/InternalsManual.rst |  3 +--
 clang/lib/Basic/Diagnostic.cpp | 16 ++++++++++------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/clang/docs/InternalsManual.rst b/clang/docs/InternalsManual.rst
index e7dd41c8595bd4..f189cb4e6a2ac3 100644
--- a/clang/docs/InternalsManual.rst
+++ b/clang/docs/InternalsManual.rst
@@ -324,8 +324,7 @@ Class:
 Description:
   This is a formatter which represents the argument number in a human readable
   format: the value ``123`` stays ``123``, ``12345`` becomes ``12.34k``,
-  ``6666666` becomes ``6.67M``, and so on for 'G' and 'T'.  Values less than
-  ``0`` are not supported.
+  ``6666666` becomes ``6.67M``, and so on for 'G' and 'T'.
 
 **"objcclass" format**
 
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index c556758eba6876..c63ee70fb4d512 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -658,15 +658,19 @@ static void HandleOrdinalModifier(unsigned ValNo,
 // 1234567 -> "1.23M".
 // 1234567890 -> "1.23G".
 // 1234567890123 -> "1.23T".
-static void HandleIntegerHumanModifier(unsigned ValNo,
+static void HandleIntegerHumanModifier(int64_t ValNo,
                                        SmallVectorImpl<char> &OutStr) {
-  static constexpr std::array<std::pair<uint64_t, char>, 4> Units = {
-      {{1'000'000'000'000UL, 'T'},
-       {1'000'000'000UL, 'G'},
-       {1'000'000UL, 'M'},
-       {1'000UL, 'k'}}};
+  static constexpr std::array<std::pair<int64_t, char>, 4> Units = {
+      {{1'000'000'000'000L, 'T'},
+       {1'000'000'000L, 'G'},
+       {1'000'000L, 'M'},
+       {1'000L, 'k'}}};
 
   llvm::raw_svector_ostream Out(OutStr);
+  if (ValNo < 0) {
+    Out << "-";
+    ValNo = -ValNo;
+  }
   for (const auto &[UnitSize, UnitSign] : Units) {
     if (ValNo >= UnitSize) {
       Out << llvm::format("%0.2f%c", ValNo / static_cast<double>(UnitSize),



More information about the cfe-commits mailing list