[clang][Diags] Automatically format AP(S)Int values with separators

[tbaederr]

This adds an operator<< overload for StreamingDiagnostic that takes an APInt/APSInt and formats it with default options, including adding separators.

This is still an opt-in mechanism since all callers that want to use this feature need to be changed from

  Diag() << toString(MyInt, 10);

to

  Diag() << MyInt;

This patch contains one example of a diagnostic making use of this.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

This adds an operator<< overfload for StreamingDiagnostic that takes an APInt/APSInt and formats it with default options, including adding separators.

This is still an opt-in mechanism since all callers that want to use this feature need to be changed from

  Diag() << toString(MyInt, 10);

to

  Diag() << MyInt;

This patch contains one example of a diagnostic making use of this.

---

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

3 Files Affected:

• (modified) clang/include/clang/Basic/Diagnostic.h (+17)
• (modified) clang/lib/Sema/SemaDecl.cpp (+1-2)
• (modified) clang/test/SemaCXX/bitfield-layout.cpp (+1-1)

diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index af26a04d94889..c79ae90db7c2f 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -25,6 +25,7 @@
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Compiler.h"
 #include <cassert>
@@ -1366,6 +1367,22 @@ inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
   return DB;
 }
 
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+                                             const llvm::APSInt &Int) {
+  DB.AddString(toString(Int, /*Radix=*/10, Int.isSigend(),
+                        /*formatAsCLiteral=*/false,
+                        /*UpperCase=*/true, /*InsertSeparators=*/true));
+  return DB;
+}
+
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+                                             const llvm::APInt &Int) {
+  DB.AddString(toString(Int, /*Radix=*/10, /*Signed=*/false,
+                        /*formatAsCLiteral=*/false,
+                        /*UpperCase=*/true, /*InsertSeparators=*/true));
+  return DB;
+}
+
 inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
                                              int I) {
   DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 9ef7a2698913d..0069b08f1991a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18909,8 +18909,7 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
     // 'bool'.
     if (BitfieldIsOverwide && !FieldTy->isBooleanType() && FieldName) {
       Diag(FieldLoc, diag::warn_bitfield_width_exceeds_type_width)
-          << FieldName << toString(Value, 10)
-          << (unsigned)TypeWidth;
+          << FieldName << Value << (unsigned)TypeWidth;
     }
   }
 
diff --git a/clang/test/SemaCXX/bitfield-layout.cpp b/clang/test/SemaCXX/bitfield-layout.cpp
index 7efd1d38c682f..f30218be01c56 100644
--- a/clang/test/SemaCXX/bitfield-layout.cpp
+++ b/clang/test/SemaCXX/bitfield-layout.cpp
@@ -35,7 +35,7 @@ CHECK_SIZE(Test4, 8);
 CHECK_ALIGN(Test4, 8);
 
 struct Test5 {
-  char c : 0x100000001; // expected-warning {{width of bit-field 'c' (4294967297 bits) exceeds the width of its type; value will be truncated to 8 bits}}
+  char c : 0x100000001; // expected-warning {{width of bit-field 'c' (4'294'967'297 bits) exceeds the width of its type; value will be truncated to 8 bits}}
 };
 // Size and align don't really matter here, just make sure we don't crash.
 CHECK_SIZE(Test5, 1);

[erichkeane]

Oooh, I really like this. Would be neat/a very appreciated followup to see if we could replace all the current places we do the old way, and use this instead. This separators view is way nicer.

[tbaederr]

Oooh, I really like this. Would be neat/a very appreciated followup to see if we could replace all the current places we do the old way, and use this instead. This separators view is way nicer.

Yup, we can create good-first-issue issues for that.