Skip to content

[Clang] [NFC] Add "human" diagnostic argument format #115835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 13, 2024

Conversation

bricknerb
Copy link
Contributor

This allows formatting large integers in a human friendly way. Example: "5321584" -> "5.32M".
Use it where such human numbers are generated manually today.

This allows formatting large integers in a human friendly way.
Example: "5321584" -> "5.32M".
Use it where such human numbers are generated manually today.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2024

@llvm/pr-subscribers-clang

Author: Boaz Brickner (bricknerb)

Changes

This allows formatting large integers in a human friendly way. Example: "5321584" -> "5.32M".
Use it where such human numbers are generated manually today.


Full diff: https://github.com/llvm/llvm-project/pull/115835.diff

5 Files Affected:

  • (modified) clang/docs/InternalsManual.rst (+12)
  • (modified) clang/include/clang/Basic/DiagnosticCommonKinds.td (+6-6)
  • (modified) clang/lib/Basic/Diagnostic.cpp (+31)
  • (modified) clang/lib/Basic/SourceManager.cpp (+4-31)
  • (modified) clang/utils/TableGen/ClangDiagnosticsEmitter.cpp (+6-1)
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;
     }

Copy link

github-actions bot commented Nov 12, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@bricknerb bricknerb merged commit 9a365bc into llvm:main Nov 13, 2024
9 checks passed
@bricknerb bricknerb deleted the format branch January 14, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants