[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo updated this revision to Diff 400696.
glotchimo marked an inline comment as done.
glotchimo added a comment.

Simplify by removing look-ahead (unnecessary b/c of difference between ident 
and tokens).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,34 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
+  verifyFormat("/* long long padding */ int() = default;\n"
+   "int &operator()   = default;\n"
+   "int &operator/**/ =();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,11 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+FormatToken *Previous = C.Tok->getPreviousNonComment();
+if (Previous && Previous->is(tok::kw_operator))
+  return false;
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,34 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
+  verifyFormat("/* long long padding */ int() = default;\n"
+   "int &operator()   = default;\n"
+   "int &operator/**/ =();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,11 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+FormatToken *Previous = C.Tok->getPreviousNonComment();
+if (Previous && Previous->is(tok::kw_operator))
+  return false;
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo added a comment.

Amazing, thank you. This is my first patch so I will need help landing, but I 
will also apply for commit access as I intend to keep contributing. Here is my 
name and email for this commit: Elliott Maguire 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117421: Fix 31568 (i.e. incorrect alignment of operator= overloads).

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo created this revision.
glotchimo added a comment.
glotchimo updated this revision to Diff 400390.
glotchimo added reviewers: MyDeveloperDay, djasper.
glotchimo added a project: clang-format.
glotchimo added a subscriber: cfe-commits.
glotchimo published this revision for review.
Herald added a project: clang.

I don't know why, but `clang-format` reformatted most if not all of the 
long/block comments in `WhitespaceManager.cpp`. Will it be necessary for me to 
revert the changes to those comments and skip formatting when updating the diff 
so as to keep it minimal?


glotchimo added a comment.

Same change as before but without the changes to long and block comments.


Added a look-ahead for brackets in the operator= context to avoid incorrect 
alignment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117421

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,14 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  auto *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,14 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  auto *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo added a comment.

Excellent, thank you! I'll get right on these edits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo updated this revision to Diff 400439.
glotchimo added a comment.

Explicitly declare `FormatToken` type instead of using `auto`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,14 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,14 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo updated this revision to Diff 400455.
glotchimo marked an inline comment as done.
glotchimo added a comment.

Add equality operator tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,26 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,26 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo updated this revision to Diff 400606.
glotchimo marked an inline comment as done.
glotchimo added a comment.

Add overload declaration look-ahead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,30 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.isOneOf(tok::l_brace, tok::semi))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,30 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,16 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.isOneOf(tok::l_brace, tok::semi))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo added a comment.

I was tinkering with the use of `getPreviousNonComment` last night before 
signing off and the problem that I noticed was that, though it stops the 
`operator=` from being split and aligned with previous lines, it adds a single 
space:

  /* long long padding */ int() = default;
   int &operator()   = default;
  -int &operator/**/=();
  +int &operator/**/ =();

Going to mess around with it some more today, but do you have any ideas as to 
why that might be happening?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo added a comment.

In D117421#3247632 , @curdeius wrote:

> Could you check if your patch fixes 
> https://github.com/llvm/llvm-project/issues/33044 as well?
> If so, please add tests.

As of right now, it doesn't fix 33044. Should I investigate and see if I can 
get this addressed in the same patch, or should it be considered a separate bug?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo updated this revision to Diff 400623.
glotchimo marked 4 inline comments as done.
glotchimo added a comment.

Use `getPreviousNonComment` to account for inline comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,34 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
+  verifyFormat("/* long long padding */ int() = default;\n"
+   "int &operator()   = default;\n"
+   "int &operator/**/ =();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,17 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+FormatToken *Previous = C.Tok->getPreviousNonComment();
+if (Previous && Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.isOneOf(tok::l_brace, tok::semi))
+  return false;
+Next = Next->Next;
+  }
+}
+
 return C.Tok->is(tok::equal);
   },
   Changes, /*StartAt=*/0, Style.AlignConsecutiveAssignments);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16167,6 +16167,34 @@
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default; // comment\n"
+   "int &operator() = default; // comment\n"
+   "int &operator=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator==() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator<=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator!=() {",
+   Alignment);
+  verifyFormat("int()   = default;\n"
+   "int &operator() = default;\n"
+   "int &operator=();",
+   Alignment);
+  verifyFormat("/* long long padding */ int() = default;\n"
+   "int &operator()   = default;\n"
+   "int &operator/**/ =();",
+   Alignment);
 
   // Bug 25167
   /* Uncomment when fixed
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -731,6 +731,17 @@
 if (&C != &Changes.back() && (&C + 1)->NewlinesBefore > 0)
   return false;
 
+// Do not align operator= overloads.
+FormatToken *Previous = C.Tok->getPreviousNonComment();
+if (Previous && Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

[Elliott Maguire via Phabricator via cfe-commits]

glotchimo added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:735-742
+if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) {
+  FormatToken *Next = C.Tok->Next;
+  while (Next && Next->NewlinesBefore == 0) {
+if (Next->Tok.is(tok::l_brace))
+  return false;
+Next = Next->Next;
+  }

curdeius wrote:
> curdeius wrote:
> > This should fix the `operator/**/ =` problem.
> Can't we simplify this?
I previously thought this would misalign other symbols succeeding `operator` 
but realize now an identifier called `operator` would differ from the actual 
keyword. Away from machine right now but will update the patch later today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117421

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