[PATCH] D146501: [clang-format] Don't format already formatted integer literals

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, rymiel.
owenpan added a project: clang-format.
Herald added a project: All.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes a bug in `IntegerLiteralSeparatorFixer::checkSeparator()` so that only 
unformatted integer literals will be formatted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146501

Files:
  clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
  clang/lib/Format/IntegerLiteralSeparatorFixer.h


Index: clang/lib/Format/IntegerLiteralSeparatorFixer.h
===================================================================
--- clang/lib/Format/IntegerLiteralSeparatorFixer.h
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.h
@@ -26,8 +26,10 @@
                                                      const FormatStyle &Style);
 
 private:
-  bool checkSeparator(const StringRef IntegerLiteral, int DigitsPerGroup) const;
-  std::string format(const StringRef IntegerLiteral, int DigitsPerGroup) const;
+  bool checkSeparator(const StringRef IntegerLiteral, int DigitsPerGroup,
+                      bool HasSeparator) const;
+  std::string format(const StringRef IntegerLiteral, int DigitsPerGroup,
+                     int DigitCount) const;
 
   char Separator;
 };
Index: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
===================================================================
--- clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
+++ clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
@@ -84,6 +84,7 @@
     auto Length = Tok.getLength();
     if (Length < 2)
       continue;
+
     auto Location = Tok.getLocation();
     auto Text = StringRef(SourceMgr.getCharacterData(Location), Length);
     if (Tok.is(tok::comment)) {
@@ -98,6 +99,7 @@
             CharSourceRange::getCharRange(Location, Tok.getEndLoc()))) {
       continue;
     }
+
     const auto B = getBase(Text);
     const bool IsBase2 = B == Base::Binary;
     const bool IsBase10 = B == Base::Decimal;
@@ -115,36 +117,51 @@
         Text.find(Separator) == StringRef::npos) {
       continue;
     }
+
     const auto Start = Text[0] == '0' ? 2 : 0;
     auto End = Text.find_first_of("uUlLzZn");
     if (End == StringRef::npos)
       End = Length;
+
     if (Start > 0 || End < Length) {
       Length = End - Start;
       Text = Text.substr(Start, Length);
     }
+
     auto DigitsPerGroup = Decimal;
     if (IsBase2)
       DigitsPerGroup = Binary;
     else if (IsBase16)
       DigitsPerGroup = Hex;
-    if (DigitsPerGroup > 0 && checkSeparator(Text, DigitsPerGroup))
+
+    unsigned DigitCount = 0;
+    for (auto C : Text)
+      if (C != Separator)
+        ++DigitCount;
+
+    if (checkSeparator(Text, DigitsPerGroup, DigitCount < Length))
       continue;
+
+    const auto &Formatted = format(Text, DigitsPerGroup, DigitCount);
+    assert(Formatted != Text);
+
     if (Start > 0)
       Location = Location.getLocWithOffset(Start);
-    if (const auto &Formatted = format(Text, DigitsPerGroup);
-        Formatted != Text) {
-      cantFail(Result.add(
-          tooling::Replacement(SourceMgr, Location, Length, Formatted)));
-    }
+
+    cantFail(Result.add(
+        tooling::Replacement(SourceMgr, Location, Length, Formatted)));
   }
 
   return {Result, 0};
 }
 
 bool IntegerLiteralSeparatorFixer::checkSeparator(
-    const StringRef IntegerLiteral, int DigitsPerGroup) const {
-  assert(DigitsPerGroup > 0);
+    const StringRef IntegerLiteral, int DigitsPerGroup,
+    bool HasSeparator) const {
+  assert(DigitsPerGroup != 0);
+
+  if (DigitsPerGroup < 0 && HasSeparator)
+    return false;
 
   int I = 0;
   for (auto C : llvm::reverse(IntegerLiteral)) {
@@ -153,9 +170,9 @@
         return false;
       I = 0;
     } else {
-      ++I;
       if (I == DigitsPerGroup)
         return false;
+      ++I;
     }
   }
 
@@ -163,7 +180,8 @@
 }
 
 std::string IntegerLiteralSeparatorFixer::format(const StringRef IntegerLiteral,
-                                                 int DigitsPerGroup) const {
+                                                 int DigitsPerGroup,
+                                                 int DigitCount) const {
   assert(DigitsPerGroup != 0);
 
   std::string Formatted;
@@ -175,11 +193,6 @@
     return Formatted;
   }
 
-  int DigitCount = 0;
-  for (auto C : IntegerLiteral)
-    if (C != Separator)
-      ++DigitCount;
-
   int Remainder = DigitCount % DigitsPerGroup;
 
   int I = 0;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits