[PATCH] D119680: [clang-format] Fix SpacesInLineCommentPrefix deleting tokens.

[Marek Kurdej via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe967d97a35a9: [clang-format] Fix SpacesInLineCommentPrefix 
deleting tokens. (authored by curdeius).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119680/new/

https://reviews.llvm.org/D119680

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3645,6 +3645,11 @@
 "   // World\n"
 "}",
 format(WrapCode, Style));
+  EXPECT_EQ("// x\n"
+"// y",
+format("//   x\n"
+   "// y",
+   Style));
 
   Style.SpacesInLineCommentPrefix = {3, 3};
   EXPECT_EQ("//   Lorem ipsum\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -747,6 +747,7 @@
   assert(Tok.is(TT_LineComment) &&
  "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
+  const int Minimum = Style.SpacesInLineCommentPrefix.Minimum;
   // How many spaces we changed in the first line of the section, this will be
   // applied in all following lines
   int FirstLineSpaceChange = 0;
@@ -769,7 +770,7 @@
   Lines[i] = Lines[i].ltrim(Blanks);
   StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   OriginalPrefix[i] = IndentPrefix;
-  const unsigned SpacesInPrefix = llvm::count(IndentPrefix, ' ');
+  const int SpacesInPrefix = llvm::count(IndentPrefix, ' ');
 
   // This lambda also considers multibyte character that is not handled in
   // functions like isPunctuation provided by CharInfo.
@@ -792,12 +793,11 @@
   // e.g. from "///" to "//".
   if (i == 0 || OriginalPrefix[i].rtrim(Blanks) !=
 OriginalPrefix[i - 1].rtrim(Blanks)) {
-if (SpacesInPrefix < Style.SpacesInLineCommentPrefix.Minimum &&
-Lines[i].size() > IndentPrefix.size() &&
+if (SpacesInPrefix < Minimum && Lines[i].size() > IndentPrefix.size() 
&&
 !NoSpaceBeforeFirstCommentChar()) {
-  FirstLineSpaceChange =
-  Style.SpacesInLineCommentPrefix.Minimum - SpacesInPrefix;
-} else if (SpacesInPrefix > Style.SpacesInLineCommentPrefix.Maximum) {
+  FirstLineSpaceChange = Minimum - SpacesInPrefix;
+} else if (static_cast(SpacesInPrefix) >
+   Style.SpacesInLineCommentPrefix.Maximum) {
   FirstLineSpaceChange =
   Style.SpacesInLineCommentPrefix.Maximum - SpacesInPrefix;
 } else {
@@ -808,10 +808,9 @@
   if (Lines[i].size() != IndentPrefix.size()) {
 PrefixSpaceChange[i] = FirstLineSpaceChange;
 
-if (SpacesInPrefix + PrefixSpaceChange[i] <
-Style.SpacesInLineCommentPrefix.Minimum) {
-  PrefixSpaceChange[i] += Style.SpacesInLineCommentPrefix.Minimum -
-  (SpacesInPrefix + PrefixSpaceChange[i]);
+if (SpacesInPrefix + PrefixSpaceChange[i] < Minimum) {
+  PrefixSpaceChange[i] +=
+  Minimum - (SpacesInPrefix + PrefixSpaceChange[i]);
 }
 
 assert(Lines[i].size() > IndentPrefix.size());


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3645,6 +3645,11 @@
 "   // World\n"
 "}",
 format(WrapCode, Style));
+  EXPECT_EQ("// x\n"
+"// y",
+format("//   x\n"
+   "// y",
+   Style));
 
   Style.SpacesInLineCommentPrefix = {3, 3};
   EXPECT_EQ("//   Lorem ipsum\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -747,6 +747,7 @@
   assert(Tok.is(TT_LineComment) &&
  "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
+  const int Minimum = Style.SpacesInLineCommentPrefix.Minimum;
   // How many spaces we changed in the first line of the section, this will be
   // applied in all following lines
   int FirstLineSpaceChange = 0;
@@ -769,7 +770,7 @@
   Lines[i] = Lines[i].ltrim(Blanks);
   StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   OriginalPrefix[i] = IndentPrefix;
-  const unsigned SpacesInPrefix = llvm::count(IndentPrefix, ' ');
+  const int SpacesInPrefix = llvm::count(IndentPrefix, ' ');
 
   // This 

[PATCH] D119680: [clang-format] Fix SpacesInLineCommentPrefix deleting tokens.

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.

Never would have thought about that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119680/new/

https://reviews.llvm.org/D119680

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

[PATCH] D119680: [clang-format] Fix SpacesInLineCommentPrefix deleting tokens.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

I thought about using signed ints for Minimum and Maximum in the style, but 
then, we won't be able to use `-1` for Maximum the same way as now and signed 
int makes no sense for Minimum...




Comment at: clang/lib/Format/BreakableToken.cpp:799-800
+  FirstLineSpaceChange = Minimum - SpacesInPrefix;
+} else if (static_cast(SpacesInPrefix) >
+   Style.SpacesInLineCommentPrefix.Maximum) {
   FirstLineSpaceChange =

Not strictly necessary, but without the cast we have a signed/unsigned mismatch 
warning on comparison.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119680/new/

https://reviews.llvm.org/D119680

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

[PATCH] D119680: [clang-format] Fix SpacesInLineCommentPrefix deleting tokens.

[Marek Kurdej via Phabricator via cfe-commits]

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

Fixes https://github.com/llvm/llvm-project/issues/53799.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119680

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestComments.cpp


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3645,6 +3645,11 @@
 "   // World\n"
 "}",
 format(WrapCode, Style));
+  EXPECT_EQ("// x\n"
+"// y",
+format("//   x\n"
+   "// y",
+   Style));
 
   Style.SpacesInLineCommentPrefix = {3, 3};
   EXPECT_EQ("//   Lorem ipsum\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -747,6 +747,7 @@
   assert(Tok.is(TT_LineComment) &&
  "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
+  const int Minimum = Style.SpacesInLineCommentPrefix.Minimum;
   // How many spaces we changed in the first line of the section, this will be
   // applied in all following lines
   int FirstLineSpaceChange = 0;
@@ -769,7 +770,7 @@
   Lines[i] = Lines[i].ltrim(Blanks);
   StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   OriginalPrefix[i] = IndentPrefix;
-  const unsigned SpacesInPrefix = llvm::count(IndentPrefix, ' ');
+  const int SpacesInPrefix = llvm::count(IndentPrefix, ' ');
 
   // This lambda also considers multibyte character that is not handled in
   // functions like isPunctuation provided by CharInfo.
@@ -792,12 +793,11 @@
   // e.g. from "///" to "//".
   if (i == 0 || OriginalPrefix[i].rtrim(Blanks) !=
 OriginalPrefix[i - 1].rtrim(Blanks)) {
-if (SpacesInPrefix < Style.SpacesInLineCommentPrefix.Minimum &&
-Lines[i].size() > IndentPrefix.size() &&
+if (SpacesInPrefix < Minimum && Lines[i].size() > IndentPrefix.size() 
&&
 !NoSpaceBeforeFirstCommentChar()) {
-  FirstLineSpaceChange =
-  Style.SpacesInLineCommentPrefix.Minimum - SpacesInPrefix;
-} else if (SpacesInPrefix > Style.SpacesInLineCommentPrefix.Maximum) {
+  FirstLineSpaceChange = Minimum - SpacesInPrefix;
+} else if (static_cast(SpacesInPrefix) >
+   Style.SpacesInLineCommentPrefix.Maximum) {
   FirstLineSpaceChange =
   Style.SpacesInLineCommentPrefix.Maximum - SpacesInPrefix;
 } else {
@@ -808,10 +808,9 @@
   if (Lines[i].size() != IndentPrefix.size()) {
 PrefixSpaceChange[i] = FirstLineSpaceChange;
 
-if (SpacesInPrefix + PrefixSpaceChange[i] <
-Style.SpacesInLineCommentPrefix.Minimum) {
-  PrefixSpaceChange[i] += Style.SpacesInLineCommentPrefix.Minimum -
-  (SpacesInPrefix + PrefixSpaceChange[i]);
+if (SpacesInPrefix + PrefixSpaceChange[i] < Minimum) {
+  PrefixSpaceChange[i] +=
+  Minimum - (SpacesInPrefix + PrefixSpaceChange[i]);
 }
 
 assert(Lines[i].size() > IndentPrefix.size());


Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3645,6 +3645,11 @@
 "   // World\n"
 "}",
 format(WrapCode, Style));
+  EXPECT_EQ("// x\n"
+"// y",
+format("//   x\n"
+   "// y",
+   Style));
 
   Style.SpacesInLineCommentPrefix = {3, 3};
   EXPECT_EQ("//   Lorem ipsum\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -747,6 +747,7 @@
   assert(Tok.is(TT_LineComment) &&
  "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
+  const int Minimum = Style.SpacesInLineCommentPrefix.Minimum;
   // How many spaces we changed in the first line of the section, this will be
   // applied in all following lines
   int FirstLineSpaceChange = 0;
@@ -769,7 +770,7 @@
   Lines[i] = Lines[i].ltrim(Blanks);
   StringRef IndentPrefix = getLineCommentIndentPrefix(Lines[i], Style);
   OriginalPrefix[i] = IndentPrefix;
-  const unsigned SpacesInPrefix = llvm::count(IndentPrefix, ' ');
+  const int SpacesInPrefix = llvm::count(IndentPrefix, ' ');