[clang] [llvm] [clang] Use separator for large numeric values in overflow diagnostic (PR #80939)

Atousa Duprat via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 6 22:02:25 PST 2024


https://github.com/Atousa updated https://github.com/llvm/llvm-project/pull/80939

>From 6ca8091362baf63fcbfeff60816830cc88c5a36f Mon Sep 17 00:00:00 2001
From: Atousa Duprat <atousa.p at gmail.com>
Date: Tue, 6 Feb 2024 21:02:05 -0800
Subject: [PATCH] [clang] Use separator for large numeric values in overflow
 diagnostic

Add functionality to APInt::toString() that allows it to insert
separators between groups of digits, using the C++ litteral
separator ' between groups.

Fixes issue #58228
---
 clang/lib/AST/ExprConstant.cpp       |  3 ++-
 llvm/include/llvm/ADT/APInt.h        |  3 ++-
 llvm/include/llvm/ADT/StringExtras.h |  5 ++--
 llvm/lib/Support/APInt.cpp           | 24 ++++++++++++++++++-
 llvm/unittests/ADT/APIntTest.cpp     | 35 ++++++++++++++++++++++++++++
 5 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 089bc2094567f..a46887d6cf1be 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2774,7 +2774,8 @@ static bool CheckedIntArithmetic(EvalInfo &Info, const Expr *E,
     if (Info.checkingForUndefinedBehavior())
       Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
                                        diag::warn_integer_constant_overflow)
-          << toString(Result, 10) << E->getType() << E->getSourceRange();
+          << toString(Result, 10, Result.isSigned(), false, true, true)
+          << E->getType() << E->getSourceRange();
     return HandleOverflow(Info, E, Value, E->getType());
   }
   return true;
diff --git a/llvm/include/llvm/ADT/APInt.h b/llvm/include/llvm/ADT/APInt.h
index 6f2f25548cc84..dd6d71b93334b 100644
--- a/llvm/include/llvm/ADT/APInt.h
+++ b/llvm/include/llvm/ADT/APInt.h
@@ -1626,7 +1626,8 @@ class [[nodiscard]] APInt {
   /// SmallString. If Radix > 10, UpperCase determine the case of letter
   /// digits.
   void toString(SmallVectorImpl<char> &Str, unsigned Radix, bool Signed,
-                bool formatAsCLiteral = false, bool UpperCase = true) const;
+                bool formatAsCLiteral = false, bool UpperCase = true,
+                bool insertSeparators = false) const;
 
   /// Considers the APInt to be unsigned and converts it into a string in the
   /// radix given. The radix can be 2, 8, 10 16, or 36.
diff --git a/llvm/include/llvm/ADT/StringExtras.h b/llvm/include/llvm/ADT/StringExtras.h
index a24368924bc90..32f5828c92d05 100644
--- a/llvm/include/llvm/ADT/StringExtras.h
+++ b/llvm/include/llvm/ADT/StringExtras.h
@@ -329,9 +329,10 @@ inline std::string itostr(int64_t X) {
 }
 
 inline std::string toString(const APInt &I, unsigned Radix, bool Signed,
-                            bool formatAsCLiteral = false) {
+                            bool formatAsCLiteral = false,
+                            bool upperCase = true, bool addSeparators = false) {
   SmallString<40> S;
-  I.toString(S, Radix, Signed, formatAsCLiteral);
+  I.toString(S, Radix, Signed, formatAsCLiteral, upperCase, addSeparators);
   return std::string(S);
 }
 
diff --git a/llvm/lib/Support/APInt.cpp b/llvm/lib/Support/APInt.cpp
index 05b1526da95ff..a186b81bc6797 100644
--- a/llvm/lib/Support/APInt.cpp
+++ b/llvm/lib/Support/APInt.cpp
@@ -2161,7 +2161,8 @@ void APInt::fromString(unsigned numbits, StringRef str, uint8_t radix) {
 }
 
 void APInt::toString(SmallVectorImpl<char> &Str, unsigned Radix, bool Signed,
-                     bool formatAsCLiteral, bool UpperCase) const {
+                     bool formatAsCLiteral, bool UpperCase,
+                     bool insertSeparators) const {
   assert((Radix == 10 || Radix == 8 || Radix == 16 || Radix == 2 ||
           Radix == 36) &&
          "Radix should be 2, 8, 10, 16, or 36!");
@@ -2187,6 +2188,12 @@ void APInt::toString(SmallVectorImpl<char> &Str, unsigned Radix, bool Signed,
     }
   }
 
+  // Number of digits in a group between separators
+  int Grouping = 4;
+  if (Radix == 8 || Radix == 10) {
+    Grouping = 3;
+  }
+
   // First, check for a zero value and just short circuit the logic below.
   if (isZero()) {
     while (*Prefix) {
@@ -2223,9 +2230,14 @@ void APInt::toString(SmallVectorImpl<char> &Str, unsigned Radix, bool Signed,
       ++Prefix;
     };
 
+    int Pos = 0;
     while (N) {
+      if (insertSeparators && Pos % Grouping == 0 && Pos > 0) {
+        *--BufPtr = '\'';
+      }
       *--BufPtr = Digits[N % Radix];
       N /= Radix;
+      Pos++;
     }
     Str.append(BufPtr, std::end(Buffer));
     return;
@@ -2257,17 +2269,27 @@ void APInt::toString(SmallVectorImpl<char> &Str, unsigned Radix, bool Signed,
     unsigned ShiftAmt = (Radix == 16 ? 4 : (Radix == 8 ? 3 : 1));
     unsigned MaskAmt = Radix - 1;
 
+    int Pos = 0;
     while (Tmp.getBoolValue()) {
       unsigned Digit = unsigned(Tmp.getRawData()[0]) & MaskAmt;
+      if (insertSeparators && Pos % Grouping == 0 && Pos > 0) {
+        Str.push_back('\'');
+      }
       Str.push_back(Digits[Digit]);
       Tmp.lshrInPlace(ShiftAmt);
+      Pos++;
     }
   } else {
+    int Pos = 0;
     while (Tmp.getBoolValue()) {
       uint64_t Digit;
       udivrem(Tmp, Radix, Tmp, Digit);
       assert(Digit < Radix && "divide failed");
+      if (insertSeparators && Pos % Grouping == 0 && Pos > 0) {
+        Str.push_back('\'');
+      }
       Str.push_back(Digits[Digit]);
+      Pos++;
     }
   }
 
diff --git a/llvm/unittests/ADT/APIntTest.cpp b/llvm/unittests/ADT/APIntTest.cpp
index 3b909f8f7d14e..e3e7e1e7bac65 100644
--- a/llvm/unittests/ADT/APIntTest.cpp
+++ b/llvm/unittests/ADT/APIntTest.cpp
@@ -1379,6 +1379,23 @@ TEST(APIntTest, toString) {
   EXPECT_EQ(std::string(S), "0");
   S.clear();
 
+  // with separators
+  APInt(64, 140).toString(S, 2, false, true, false, true);
+  EXPECT_EQ(std::string(S), "0b1000'1100");
+  S.clear();
+  APInt(64, 1024).toString(S, 8, false, true, false, true);
+  EXPECT_EQ(std::string(S), "02'000");
+  S.clear();
+  APInt(64, 1000000).toString(S, 10, false, true, false, true);
+  EXPECT_EQ(std::string(S), "1'000'000");
+  S.clear();
+  APInt(64, 1000000).toString(S, 16, false, true, true, true);
+  EXPECT_EQ(std::string(S), "0xF'4240");
+  S.clear();
+  APInt(64, 1'000'000'000).toString(S, 36, false, false, false, true);
+  EXPECT_EQ(std::string(S), "gj'dgxs");
+  S.clear();
+
   isSigned = false;
   APInt(8, 255, isSigned).toString(S, 2, isSigned, true);
   EXPECT_EQ(std::string(S), "0b11111111");
@@ -1415,6 +1432,24 @@ TEST(APIntTest, toString) {
   APInt(8, 255, isSigned).toString(S, 36, isSigned, false);
   EXPECT_EQ(std::string(S), "-1");
   S.clear();
+
+  // negative with separators
+  APInt(64, -140, isSigned).toString(S, 2, isSigned, true, false, true);
+  EXPECT_EQ(std::string(S), "-0b1000'1100");
+  S.clear();
+  APInt(64, -1024, isSigned).toString(S, 8, isSigned, true, false, true);
+  EXPECT_EQ(std::string(S), "-02'000");
+  S.clear();
+  APInt(64, -1000000, isSigned).toString(S, 10, isSigned, true, false, true);
+  EXPECT_EQ(std::string(S), "-1'000'000");
+  S.clear();
+  APInt(64, -1000000, isSigned).toString(S, 16, isSigned, true, true, true);
+  EXPECT_EQ(std::string(S), "-0xF'4240");
+  S.clear();
+  APInt(64, -1'000'000'000, isSigned)
+      .toString(S, 36, isSigned, false, false, true);
+  EXPECT_EQ(std::string(S), "-gj'dgxs");
+  S.clear();
 }
 
 TEST(APIntTest, Log2) {



More information about the llvm-commits mailing list