- 
                Notifications
    
You must be signed in to change notification settings  - Fork 15.1k
 
[clang-format] Add xxxMaxDigitsNoSeparator #164286
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
base: main
Are you sure you want to change the base?
Conversation
This basically adds a Leave option for a specific range of literals.
| 
          
 @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-format Author: Björn Schäpers (HazardyKnusperkeks) ChangesThis basically adds a Leave option for a specific range of literals. Full diff: https://github.com/llvm/llvm-project/pull/164286.diff 5 Files Affected: 
 diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 570cab262c115..25160b137535e 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -4651,7 +4651,12 @@ the configuration (without a prefix: ``Auto``).
 
   You can also specify a minimum number of digits (``BinaryMinDigits``,
   ``DecimalMinDigits``, and ``HexMinDigits``) the integer literal must
-  have in order for the separators to be inserted.
+  have in order for the separators to be inserted, and a maximum number of
+  digits (``BinaryMaxDigitsNoSeparator``, ``DecimalMaxDigitsNoSeparator``,
+  and ``HexMaxDigitsNoSeparator``) until the separators are removed. This
+  divides the literals in 3 regions, always without separator (up until
+  including ``xxxMaxDigitsNoSeparator``), maybe with, or without separators
+  (up until excluding ``xxxMinDigits``), and finally always with separators.
 
   * ``int8_t Binary`` Format separators in binary literals.
 
@@ -4671,6 +4676,18 @@ the configuration (without a prefix: ``Auto``).
       b1 = 0b101101;
       b2 = 0b1'101'101;
 
+  * ``int8_t BinaryMaxDigitsNoSeparator`` Remove separators in binary literals with a maximum number of digits.
+
+    .. code-block:: text
+
+      // Binary: 3
+      // BinaryMinDigits: 7
+      // BinaryMaxDigitsNoSeparator: 4
+      b0 = 0b1011; // Always removed.
+      b1 = 0b101101; // Not added.
+      b2 = 0b101'101; // Not removed.
+      b3 = 0b1'101'101; // Always added.
+
   * ``int8_t Decimal`` Format separators in decimal literals.
 
     .. code-block:: text
@@ -4688,6 +4705,18 @@ the configuration (without a prefix: ``Auto``).
       d1 = 2023;
       d2 = 10'000;
 
+  * ``int8_t DecimalMaxDigitsNoSeparator`` Remove separators in decimal literals with a maximum number of digits.
+
+    .. code-block:: text
+
+      // Decimal: 3
+      // DecimalMinDigits: 7
+      // DecimalMaxDigitsNoSeparator: 4
+      d0 = 2023; // Always removed.
+      d1 = 123456; // Not added.
+      d2 = 123'456; // Not removed.
+      d3 = 5'000'000; // Always added.
+
   * ``int8_t Hex`` Format separators in hexadecimal literals.
 
     .. code-block:: text
@@ -4706,6 +4735,19 @@ the configuration (without a prefix: ``Auto``).
       h1 = 0xABCDE;
       h2 = 0xAB'CD'EF;
 
+  * ``int8_t HexMaxDigitsNoSeparator`` Remove separators in hexadecimal literals with a maximum number of
+    digits.
+
+    .. code-block:: text
+
+      // Hex: 2
+      // HexMinDigits: 6
+      // HexMaxDigitsNoSeparator: 4
+      h0 = 0xAFFE; // Always removed.
+      h1 = 0xABCDE; // Not added.
+      h2 = 0xA'BC'DE; // Not removed.
+      h3 = 0xAB'CD'EF; // Always added.
+
 
 .. _JavaImportGroups:
 
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 2852c4a2916a4..4e1974b8f9460 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3174,7 +3174,12 @@ struct FormatStyle {
   ///
   /// You can also specify a minimum number of digits (``BinaryMinDigits``,
   /// ``DecimalMinDigits``, and ``HexMinDigits``) the integer literal must
-  /// have in order for the separators to be inserted.
+  /// have in order for the separators to be inserted, and a maximum number of
+  /// digits (``BinaryMaxDigitsNoSeparator``, ``DecimalMaxDigitsNoSeparator``,
+  /// and ``HexMaxDigitsNoSeparator``) until the separators are removed. This
+  /// divides the literals in 3 regions, always without separator (up until
+  /// including ``xxxMaxDigitsNoSeparator``), maybe with, or without separators
+  /// (up until excluding ``xxxMinDigits``), and finally always with separators.
   struct IntegerLiteralSeparatorStyle {
     /// Format separators in binary literals.
     /// \code{.text}
@@ -3192,6 +3197,17 @@ struct FormatStyle {
     ///   b2 = 0b1'101'101;
     /// \endcode
     int8_t BinaryMinDigits;
+    /// Remove separators in binary literals with a maximum number of digits.
+    /// \code{.text}
+    ///   // Binary: 3
+    ///   // BinaryMinDigits: 7
+    ///   // BinaryMaxDigitsNoSeparator: 4
+    ///   b0 = 0b1011; // Always removed.
+    ///   b1 = 0b101101; // Not added.
+    ///   b2 = 0b101'101; // Not removed.
+    ///   b3 = 0b1'101'101; // Always added.
+    /// \endcode
+    int8_t BinaryMaxDigitsNoSeparator;
     /// Format separators in decimal literals.
     /// \code{.text}
     ///   /* -1: */ d = 18446744073709550592ull;
@@ -3207,6 +3223,17 @@ struct FormatStyle {
     ///   d2 = 10'000;
     /// \endcode
     int8_t DecimalMinDigits;
+    /// Remove separators in decimal literals with a maximum number of digits.
+    /// \code{.text}
+    ///   // Decimal: 3
+    ///   // DecimalMinDigits: 7
+    ///   // DecimalMaxDigitsNoSeparator: 4
+    ///   d0 = 2023; // Always removed.
+    ///   d1 = 123456; // Not added.
+    ///   d2 = 123'456; // Not removed.
+    ///   d3 = 5'000'000; // Always added.
+    /// \endcode
+    int8_t DecimalMaxDigitsNoSeparator;
     /// Format separators in hexadecimal literals.
     /// \code{.text}
     ///   /* -1: */ h = 0xDEADBEEFDEADBEEFuz;
@@ -3223,10 +3250,25 @@ struct FormatStyle {
     ///   h2 = 0xAB'CD'EF;
     /// \endcode
     int8_t HexMinDigits;
+    /// Remove separators in hexadecimal literals with a maximum number of
+    /// digits.
+    /// \code{.text}
+    ///   // Hex: 2
+    ///   // HexMinDigits: 6
+    ///   // HexMaxDigitsNoSeparator: 4
+    ///   h0 = 0xAFFE; // Always removed.
+    ///   h1 = 0xABCDE; // Not added.
+    ///   h2 = 0xA'BC'DE; // Not removed.
+    ///   h3 = 0xAB'CD'EF; // Always added.
+    /// \endcode
+    int8_t HexMaxDigitsNoSeparator;
     bool operator==(const IntegerLiteralSeparatorStyle &R) const {
       return Binary == R.Binary && BinaryMinDigits == R.BinaryMinDigits &&
+             BinaryMaxDigitsNoSeparator == R.BinaryMaxDigitsNoSeparator &&
              Decimal == R.Decimal && DecimalMinDigits == R.DecimalMinDigits &&
-             Hex == R.Hex && HexMinDigits == R.HexMinDigits;
+             DecimalMaxDigitsNoSeparator == R.DecimalMaxDigitsNoSeparator &&
+             Hex == R.Hex && HexMinDigits == R.HexMinDigits &&
+             HexMaxDigitsNoSeparator == R.HexMaxDigitsNoSeparator;
     }
   };
 
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index edd126c7724b8..59495467c01b4 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -399,10 +399,15 @@ template <> struct MappingTraits<FormatStyle::IntegerLiteralSeparatorStyle> {
   static void mapping(IO &IO, FormatStyle::IntegerLiteralSeparatorStyle &Base) {
     IO.mapOptional("Binary", Base.Binary);
     IO.mapOptional("BinaryMinDigits", Base.BinaryMinDigits);
+    IO.mapOptional("BinaryMaxDigitsNoSeparator",
+                   Base.BinaryMaxDigitsNoSeparator);
     IO.mapOptional("Decimal", Base.Decimal);
     IO.mapOptional("DecimalMinDigits", Base.DecimalMinDigits);
+    IO.mapOptional("DecimalMaxDigitsNoSeparator",
+                   Base.DecimalMaxDigitsNoSeparator);
     IO.mapOptional("Hex", Base.Hex);
     IO.mapOptional("HexMinDigits", Base.HexMinDigits);
+    IO.mapOptional("HexMaxDigitsNoSeparator", Base.HexMaxDigitsNoSeparator);
   }
 };
 
@@ -1673,10 +1678,15 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.InsertBraces = false;
   LLVMStyle.InsertNewlineAtEOF = false;
   LLVMStyle.InsertTrailingCommas = FormatStyle::TCS_None;
-  LLVMStyle.IntegerLiteralSeparator = {
-      /*Binary=*/0,  /*BinaryMinDigits=*/0,
-      /*Decimal=*/0, /*DecimalMinDigits=*/0,
-      /*Hex=*/0,     /*HexMinDigits=*/0};
+  LLVMStyle.IntegerLiteralSeparator = {/*Binary=*/0,
+                                       /*BinaryMinDigits=*/0,
+                                       /*BinaryMaxDigitsNoSeparator=*/-1,
+                                       /*Decimal=*/0,
+                                       /*DecimalMinDigits=*/0,
+                                       /*DecimalMaxDigitsNoSeparator=*/-1,
+                                       /*Hex=*/0,
+                                       /*HexMinDigits=*/0,
+                                       /*HexMaxDigitsNoSeparator=*/-1};
   LLVMStyle.JavaScriptQuotes = FormatStyle::JSQS_Leave;
   LLVMStyle.JavaScriptWrapImports = true;
   LLVMStyle.KeepEmptyLines = {
diff --git a/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp b/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
index b51991bfeff4b..ae3b886d7e491 100644
--- a/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ b/clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -72,11 +72,25 @@ IntegerLiteralSeparatorFixer::process(const Environment &Env,
   if (SkipBinary && SkipDecimal && SkipHex)
     return {};
 
-  const auto BinaryMinDigits =
-      std::max((int)Option.BinaryMinDigits, Binary + 1);
-  const auto DecimalMinDigits =
-      std::max((int)Option.DecimalMinDigits, Decimal + 1);
-  const auto HexMinDigits = std::max((int)Option.HexMinDigits, Hex + 1);
+  auto CalcMinAndMax = [](int8_t Digits, int8_t MinDigits,
+                          int8_t MaxDigitsNoSeparator) {
+    std::pair<int, int> Ret;
+    Ret.first = std::max<int>(MinDigits, Digits + 1);
+    if (Ret.first == 0)
+      Ret.second = 0;
+    else if (MaxDigitsNoSeparator < 0)
+      Ret.second = Ret.first - 1;
+    else
+      Ret.second = std::min<int>(MaxDigitsNoSeparator, Ret.first - 1);
+    return Ret;
+  };
+
+  const auto [BinaryMinDigits, BinaryMaxDigitsNoSeparator] = CalcMinAndMax(
+      Binary, Option.BinaryMinDigits, Option.BinaryMaxDigitsNoSeparator);
+  const auto [DecimalMinDigits, DecimalMaxDigitsNoSeparator] = CalcMinAndMax(
+      Decimal, Option.DecimalMinDigits, Option.DecimalMaxDigitsNoSeparator);
+  const auto [HexMinDigits, HexMaxDigitsNoSeparator] =
+      CalcMinAndMax(Hex, Option.HexMinDigits, Option.HexMaxDigitsNoSeparator);
 
   const auto &SourceMgr = Env.getSourceManager();
   AffectedRangeManager AffectedRangeMgr(SourceMgr, Env.getCharRanges());
@@ -139,19 +153,29 @@ IntegerLiteralSeparatorFixer::process(const Environment &Env,
     }
     auto DigitsPerGroup = Decimal;
     auto MinDigits = DecimalMinDigits;
+    auto MaxDigitsNoSeparator = DecimalMaxDigitsNoSeparator;
     if (IsBase2) {
       DigitsPerGroup = Binary;
       MinDigits = BinaryMinDigits;
+      MaxDigitsNoSeparator = BinaryMaxDigitsNoSeparator;
     } else if (IsBase16) {
       DigitsPerGroup = Hex;
       MinDigits = HexMinDigits;
+      MaxDigitsNoSeparator = HexMaxDigitsNoSeparator;
     }
     const auto SeparatorCount = Text.count(Separator);
     const int DigitCount = Length - SeparatorCount;
-    const bool RemoveSeparator = DigitsPerGroup < 0 || DigitCount < MinDigits;
+    const bool RemoveSeparator =
+        DigitsPerGroup < 0 || DigitCount <= MaxDigitsNoSeparator;
+    const bool AddSeparator =
+        DigitsPerGroup > 0 &&
+        (DigitCount >= MinDigits ||
+         (DigitCount > MaxDigitsNoSeparator && SeparatorCount > 0));
+    if (!RemoveSeparator && !AddSeparator)
+      continue;
     if (RemoveSeparator && SeparatorCount == 0)
       continue;
-    if (!RemoveSeparator && SeparatorCount > 0 &&
+    if (AddSeparator && SeparatorCount > 0 &&
         checkSeparator(Text, DigitsPerGroup)) {
       continue;
     }
diff --git a/clang/unittests/Format/IntegerLiteralSeparatorTest.cpp b/clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
index 53b6dd8efadff..d66de86be88b0 100644
--- a/clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
+++ b/clang/unittests/Format/IntegerLiteralSeparatorTest.cpp
@@ -243,6 +243,22 @@ TEST_F(IntegerLiteralSeparatorTest, FloatingPoint) {
                Style);
 }
 
+TEST_F(IntegerLiteralSeparatorTest, MaxDigitsNoSeparator) {
+  auto Style = getLLVMStyle();
+  Style.IntegerLiteralSeparator.Decimal = 3;
+  Style.IntegerLiteralSeparator.DecimalMaxDigitsNoSeparator = 4;
+  Style.IntegerLiteralSeparator.DecimalMinDigits = 7;
+  verifyFormat("d0 = 2023;\n"
+               "d1 = 123456;\n"
+               "d2 = 123'456;\n"
+               "d3 = 5'000'000;",
+               "d0 = 20'2'3;\n"
+               "d1 = 123456;\n"
+               "d2 = 1234'56;\n"
+               "d3 = 5000000;",
+               Style);
+}
+
 } // namespace
 } // namespace test
 } // namespace format
 | 
    
          
 Can you elaborate? If the max decimal digits value is set to 4, and everything else is left at the default, what will be   | 
    
          
 As the test shows, if there is at least one separator it is corrected. Everything default will always remove the separators, so I assume: 
 Leave is in that case not exactly correct. Leave only talks about the choice of separator or no separator, not the placement. I've updated the example in the documentation.  | 
    
| Style.IntegerLiteralSeparator.DecimalMaxDigitsNoSeparator = 4; | ||
| Style.IntegerLiteralSeparator.DecimalMinDigits = 7; | ||
| verifyFormat("d0 = 2023;\n" | ||
| "d1 = 123456;\n" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's weird that unlike d2, d1 doesn't get a separator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not weird at all, maybe not good enough explained by me.
Everything with 4 or less digits gets all separators stripped, everything with 7 or above digits get separators added or corrected, and everything in between gets none added, if there were none, but if there is at least one separator they will be corrected.
| 
           When I designed   | 
    
          
 I want to add a range, where I have the choice if there are separators, or not. Yes that sets an upper bound to where they are removed, but in the same time opens a window where clang-format ensures the separators are up to code, if there is at least one before formatting, but doesn't add them, if there weren't any.  | 
    
This basically adds a Leave option for a specific range of literals.