This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Clean up code looking for if statements
AbandonedPublic

Authored by sstwcw on Mar 15 2022, 4:29 PM.

Details

Summary

Previously several places had code to determine whether the line is an
if / while / for statement. This commit replaces them with a function
call.

About the removal of the check in parseAngle. At first there was this
bug https://bugs.llvm.org/show_bug.cgi?id=46969. Right-shift
operators would sometimes get split because they got regarded as
template closers. They fixed the problem by adding the check in
question, telling the program to assume two consecutive >'s in a
condition is not a template. Then they found this bug
https://bugs.llvm.org/show_bug.cgi?id=46969. It turned out a
right-shift can also occur outside of parentheses and get regarded as a
template closer. So they added the next line to fix it. That check
would succeed whether or not the right shift was in an if condition.
Thus the first check has been unnecessary since we had the second one.

Diff Detail

Event Timeline

sstwcw created this revision.Mar 15 2022, 4:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2022, 4:29 PM
sstwcw requested review of this revision.Mar 15 2022, 4:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2022, 4:29 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
HazardyKnusperkeks edited reviewers, added: MyDeveloperDay, owenpan, curdeius, HazardyKnusperkeks; removed: Restricted Project.Mar 16 2022, 1:47 PM

Please add tests in TokenAnnotatorTests for TT_ConditionLParen.

clang/lib/Format/FormatToken.h
526

Please document what this means.

clang/lib/Format/TokenAnnotator.cpp
133

Any reason why one doesn't need this check anymore?

1446–1450

Unrelated Change.

clang/lib/Format/UnwrappedLineParser.cpp
2426

Please use setFixedType.
Then you don't need to add TT_ConditionLParen to the exclude list in the continuation indenter.

sstwcw updated this revision to Diff 416052.Mar 16 2022, 8:15 PM
sstwcw edited the summary of this revision. (Show Details)
sstwcw marked 4 inline comments as done.Mar 16 2022, 8:18 PM
sstwcw added inline comments.
clang/lib/Format/TokenAnnotator.cpp
133

The next check already covers what the first is supposed to cover, as explained in the commit message.

1446–1450

Sorry. This part has changed several times in the main line since I started working on this patch. After a few merge conflicts I got tired and started running sort uniq on this part.

HazardyKnusperkeks added inline comments.
clang/lib/Format/TokenAnnotator.cpp
1446–1450

Nothing against that, but in a separate patch.
Although my goal is the get this list smaller again with the finalized type.

This revision is now accepted and ready to land.Mar 17 2022, 1:05 PM
MyDeveloperDay requested changes to this revision.Mar 18 2022, 1:48 AM

Sorry I don't believe we've covered the changes with unit tests.

clang/lib/Format/FormatToken.h
530

IncludeSpecial seems confusing at best if not obtuse.

540

What about MacroIf

clang/lib/Format/TokenAnnotator.cpp
3018

There has to be a missed unit test here..this condition before only handled if and for

Did you run the regression suite as well as the unit tests?

This revision now requires changes to proceed.Mar 18 2022, 1:48 AM
sstwcw marked 2 inline comments as done.Mar 21 2022, 4:29 PM

It turned out this patch does change behavior.

-  while (
-      FormatTok->isOneOf(tok::identifier, tok::kw_requires, tok::coloncolon)) {
+  while (FormatTok->isOneOf(tok::identifier, tok::kw_requires,
+                            tok::coloncolon)) {

So what do I do?

And what's the regression suite?

It turned out this patch does change behavior.

-  while (
-      FormatTok->isOneOf(tok::identifier, tok::kw_requires, tok::coloncolon)) {
+  while (FormatTok->isOneOf(tok::identifier, tok::kw_requires,
+                            tok::coloncolon)) {

So what do I do?

And what's the regression suite?

I like to run clang-format over all the files in LLVM that were previously clang-formatted, (now be careful because some people don't keep their files formatted), but you can run this over the ~8000 files in LLVM that are clang-format clean, this should give you a good idea of if you change is breaking stuff.

clang-format -verbose -n -files ./clang/docs/tools/clang-formatted-files.txt

It turned out this patch does change behavior.

i.e. now your function is considering more things, the formatted code is different, do you think the new formatting is more correct? (I mean it feels like it should be right? assuming we want to consider whiles in the same way we consider if and for (which seems fairly reasonable, i guess)

You have to capture that in the unit tests for sure, but also can you work out what it was not doing? what it that it wasn't considering the while? should it have?

This is kind of the problem about refactoring without a github issue to justify why you are making the changes in the first place. If you had/can find a bug saying "While loop doesn't format correctly" then it would be easier to justify.

FWIW I'm not a fan of the while( \n case, so assuming this fix, fixes that then that would be good I think.

-  while (
-      FormatTok->isOneOf(tok::identifier, tok::kw_requires, tok::coloncolon)) {
+  while (FormatTok->isOneOf(tok::identifier, tok::kw_requires,
+                            tok::coloncolon)) {
owenpan requested changes to this revision.Mar 22 2022, 2:57 AM
owenpan added inline comments.
clang/lib/Format/ContinuationIndenter.cpp
749 ↗(On Diff #416052)

We only checked for and if before. Now you are also checking while and switch?

clang/lib/Format/FormatToken.h
531
536–541

We prefer early returns and shorter conditionals.

clang/lib/Format/UnwrappedLineParser.cpp
2425–2426
2719–2721

You can move the comment to above line 2729 below if you like.

2727
HazardyKnusperkeks requested changes to this revision.Mar 22 2022, 12:51 PM

Just a formality.

sstwcw updated this revision to Diff 418071.Mar 24 2022, 4:18 PM
sstwcw marked 5 inline comments as done.Mar 24 2022, 4:24 PM
sstwcw added inline comments.
clang/lib/Format/ContinuationIndenter.cpp
749 ↗(On Diff #416052)

Yes, I am. Please see the diff and tell me whether while and switch should be checked here.

clang/lib/Format/FormatToken.h
540

What's that?

clang/lib/Format/TokenAnnotator.cpp
3018

I added a test.

sstwcw updated this revision to Diff 418073.Mar 24 2022, 4:30 PM
sstwcw marked an inline comment as done.
sstwcw marked 2 inline comments as done.Mar 24 2022, 4:31 PM

This is how checking for while changes behavior.

diff --git a/flang/lib/Evaluate/tools.cpp b/flang/lib/Evaluate/tools.cpp
index 68b2a40d48c5..3f811fee9bad 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -868,8 +868,8 @@ bool HasVectorSubscript(const Expr<SomeType> &expr) {
 parser::Message *AttachDeclaration(
     parser::Message &message, const Symbol &symbol) {
   const Symbol *unhosted{&symbol};
-  while (
-      const auto *assoc{unhosted->detailsIf<semantics::HostAssocDetails>()}) {
+  while (const auto *assoc{
+             unhosted->detailsIf<semantics::HostAssocDetails>()}) {
     unhosted = &assoc->symbol();
   }
   if (const auto *binding{
diff --git a/flang/lib/Semantics/data-to-inits.cpp b/flang/lib/Semantics/data-to-inits.cpp
index 6bdbf5f6549f..aa8188656bcb 100644
--- a/flang/lib/Semantics/data-to-inits.cpp
+++ b/flang/lib/Semantics/data-to-inits.cpp
@@ -407,8 +407,8 @@ bool DataInitializationCompiler<DSV>::InitElement(
             DescribeElement(), designatorType->AsFortran());
       }
       auto folded{evaluate::Fold(context, std::move(converted->first))};
-      switch (GetImage().Add(
-          offsetSymbol.offset(), offsetSymbol.size(), folded, context)) {
+      switch (GetImage().Add(offsetSymbol.offset(), offsetSymbol.size(), folded,
+                  context)) {
       case evaluate::InitialImage::Ok:
         return true;
       case evaluate::InitialImage::NotAConstant:
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 3391665786d5..f1b86a133f1c 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -526,7 +526,7 @@ void SBDebugger::HandleCommand(const char *command) {
         EventSP event_sp;
         ListenerSP lldb_listener_sp = m_opaque_sp->GetListener();
         while (lldb_listener_sp->GetEventForBroadcaster(
-            process_sp.get(), event_sp, std::chrono::seconds(0))) {
+                   process_sp.get(), event_sp, std::chrono::seconds(0))) {
           SBEvent event(event_sp);
           HandleProcessEvent(process, event, GetOutputFile(), GetErrorFile());
         }
diff --git a/openmp/runtime/src/kmp_alloc.cpp b/openmp/runtime/src/kmp_alloc.cpp
index 0f76906714b1..319c3e779fda 100644
--- a/openmp/runtime/src/kmp_alloc.cpp
+++ b/openmp/runtime/src/kmp_alloc.cpp
@@ -2051,7 +2051,8 @@ void *___kmp_fast_allocate(kmp_info_t *this_thr, size_t size KMP_SRC_LOC_DECL) {
     // threads only)
     // pop the head of the sync free list, push NULL instead
     while (!KMP_COMPARE_AND_STORE_PTR(
-        &this_thr->th.th_free_lists[index].th_free_list_sync, ptr, nullptr)) {
+               &this_thr->th.th_free_lists[index].th_free_list_sync, ptr,
+               nullptr)) {
       KMP_CPU_PAUSE();
       ptr = TCR_SYNC_PTR(this_thr->th.th_free_lists[index].th_free_list_sync);
     }
@@ -2178,7 +2179,8 @@ void ___kmp_fast_free(kmp_info_t *this_thr, void *ptr KMP_SRC_LOC_DECL) {
         *((void **)tail) = old_ptr;
 
         while (!KMP_COMPARE_AND_STORE_PTR(
-            &q_th->th.th_free_lists[index].th_free_list_sync, old_ptr, head)) {
+                   &q_th->th.th_free_lists[index].th_free_list_sync, old_ptr,
+                   head)) {
           KMP_CPU_PAUSE();
           old_ptr = TCR_PTR(q_th->th.th_free_lists[index].th_free_list_sync);
           *((void **)tail) = old_ptr;
diff --git a/openmp/runtime/src/kmp_atomic.cpp b/openmp/runtime/src/kmp_atomic.cpp
index 21c2c60bfb60..79b8373319e4 100644
--- a/openmp/runtime/src/kmp_atomic.cpp
+++ b/openmp/runtime/src/kmp_atomic.cpp
@@ -793,8 +793,9 @@ static inline kmp_cmplx128_a16_t operator/(kmp_cmplx128_a16_t &lhs,
     old_value = *(TYPE volatile *)lhs;                                         \
     new_value = (TYPE)(old_value OP rhs);                                      \
     while (!KMP_COMPARE_AND_STORE_ACQ##BITS(                                   \
-        (kmp_int##BITS *)lhs, *VOLATILE_CAST(kmp_int##BITS *) & old_value,     \
-        *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                        \
+               (kmp_int##BITS *)lhs,                                           \
+               *VOLATILE_CAST(kmp_int##BITS *) & old_value,                    \
+               *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                 \
       KMP_DO_PAUSE;                                                            \
                                                                                \
       old_value = *(TYPE volatile *)lhs;                                       \
@@ -821,8 +822,9 @@ static inline kmp_cmplx128_a16_t operator/(kmp_cmplx128_a16_t &lhs,
     *old_value.vvv = *(volatile kmp_int##BITS *)lhs;                           \
     new_value.cmp = (TYPE)(old_value.cmp OP rhs);                              \
     while (!KMP_COMPARE_AND_STORE_ACQ##BITS(                                   \
-        (kmp_int##BITS *)lhs, *VOLATILE_CAST(kmp_int##BITS *) old_value.vvv,   \
-        *VOLATILE_CAST(kmp_int##BITS *) new_value.vvv)) {                      \
+               (kmp_int##BITS *)lhs,                                           \
+               *VOLATILE_CAST(kmp_int##BITS *) old_value.vvv,                  \
+               *VOLATILE_CAST(kmp_int##BITS *) new_value.vvv)) {               \
       KMP_DO_PAUSE;                                                            \
                                                                                \
       *old_value.vvv = *(volatile kmp_int##BITS *)lhs;                         \
@@ -848,8 +850,9 @@ static inline kmp_cmplx128_a16_t operator/(kmp_cmplx128_a16_t &lhs,
     *old_value.vvv = *(volatile kmp_int##BITS *)lhs;                           \
     new_value.cmp = old_value.cmp OP rhs;                                      \
     while (!KMP_COMPARE_AND_STORE_ACQ##BITS(                                   \
-        (kmp_int##BITS *)lhs, *VOLATILE_CAST(kmp_int##BITS *) old_value.vvv,   \
-        *VOLATILE_CAST(kmp_int##BITS *) new_value.vvv)) {                      \
+               (kmp_int##BITS *)lhs,                                           \
+               *VOLATILE_CAST(kmp_int##BITS *) old_value.vvv,                  \
+               *VOLATILE_CAST(kmp_int##BITS *) new_value.vvv)) {               \
       KMP_DO_PAUSE;                                                            \
                                                                                \
       *old_value.vvv = *(volatile kmp_int##BITS *)lhs;                         \
@@ -1461,8 +1464,9 @@ ATOMIC_CRITICAL(cmplx16, div_a16, kmp_cmplx128_a16_t, /, 32c,
     old_value = temp_val;                                                      \
     new_value = (TYPE)(rhs OP old_value);                                      \
     while (!KMP_COMPARE_AND_STORE_ACQ##BITS(                                   \
-        (kmp_int##BITS *)lhs, *VOLATILE_CAST(kmp_int##BITS *) & old_value,     \
-        *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                        \
+               (kmp_int##BITS *)lhs,                                           \
+               *VOLATILE_CAST(kmp_int##BITS *) & old_value,                    \
+               *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                 \
       KMP_DO_PAUSE;                                                            \
                                                                                \
       temp_val = *lhs;                                                         \
@@ -2127,8 +2131,9 @@ ATOMIC_CRITICAL_READ(cmplx16, a16_rd, kmp_cmplx128_a16_t, +, 32c,
     old_value = temp_val;                                                      \
     new_value = rhs;                                                           \
     while (!KMP_COMPARE_AND_STORE_ACQ##BITS(                                   \
-        (kmp_int##BITS *)lhs, *VOLATILE_CAST(kmp_int##BITS *) & old_value,     \
-        *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                        \
+               (kmp_int##BITS *)lhs,                                           \
+               *VOLATILE_CAST(kmp_int##BITS *) & old_value,                    \
+               *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                 \
       temp_val = *lhs;                                                         \
       old_value = temp_val;                                                    \
       new_value = rhs;                                                         \
@@ -2275,8 +2280,9 @@ ATOMIC_CRITICAL_WR(cmplx16, a16_wr, kmp_cmplx128_a16_t, =, 32c,
     old_value = temp_val;                                                      \
     new_value = (TYPE)(old_value OP rhs);                                      \
     while (!KMP_COMPARE_AND_STORE_ACQ##BITS(                                   \
-        (kmp_int##BITS *)lhs, *VOLATILE_CAST(kmp_int##BITS *) & old_value,     \
-        *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                        \
+               (kmp_int##BITS *)lhs,                                           \
+               *VOLATILE_CAST(kmp_int##BITS *) & old_value,                    \
+               *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                 \
       temp_val = *lhs;                                                         \
       old_value = temp_val;                                                    \
       new_value = (TYPE)(old_value OP rhs);                                    \
@@ -2971,8 +2977,9 @@ ATOMIC_CRITICAL_CPT(cmplx16, div_a16_cpt, kmp_cmplx128_a16_t, /, 32c,
     old_value = temp_val;                                                      \
     new_value = (TYPE)(rhs OP old_value);                                      \
     while (!KMP_COMPARE_AND_STORE_ACQ##BITS(                                   \
-        (kmp_int##BITS *)lhs, *VOLATILE_CAST(kmp_int##BITS *) & old_value,     \
-        *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                        \
+               (kmp_int##BITS *)lhs,                                           \
+               *VOLATILE_CAST(kmp_int##BITS *) & old_value,                    \
+               *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                 \
       temp_val = *lhs;                                                         \
       old_value = temp_val;                                                    \
       new_value = (TYPE)(rhs OP old_value);                                    \
@@ -3295,8 +3302,9 @@ ATOMIC_CRITICAL_CPT_REV_MIX(float10, long double, div_cpt_rev, /, fp, _Quad,
     old_value = temp_val;                                                      \
     new_value = rhs;                                                           \
     while (!KMP_COMPARE_AND_STORE_ACQ##BITS(                                   \
-        (kmp_int##BITS *)lhs, *VOLATILE_CAST(kmp_int##BITS *) & old_value,     \
-        *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                        \
+               (kmp_int##BITS *)lhs,                                           \
+               *VOLATILE_CAST(kmp_int##BITS *) & old_value,                    \
+               *VOLATILE_CAST(kmp_int##BITS *) & new_value)) {                 \
       temp_val = *lhs;                                                         \
       old_value = temp_val;                                                    \
       new_value = rhs;                                                         \
@@ -3476,8 +3484,9 @@ void __kmpc_atomic_2(ident_t *id_ref, int gtid, void *lhs, void *rhs,
     (*f)(&new_value, &old_value, rhs);
 
     /* TODO: Should this be acquire or release? */
-    while (!KMP_COMPARE_AND_STORE_ACQ16(
-        (kmp_int16 *)lhs, *(kmp_int16 *)&old_value, *(kmp_int16 *)&new_value)) {
+    while (!KMP_COMPARE_AND_STORE_ACQ16((kmp_int16 *)lhs,
+                                        *(kmp_int16 *)&old_value,
+                                        *(kmp_int16 *)&new_value)) {
       KMP_CPU_PAUSE();
 
       old_value = *(kmp_int16 *)lhs;
@@ -3525,8 +3534,9 @@ void __kmpc_atomic_4(ident_t *id_ref, int gtid, void *lhs, void *rhs,
     (*f)(&new_value, &old_value, rhs);
 
     /* TODO: Should this be acquire or release? */
-    while (!KMP_COMPARE_AND_STORE_ACQ32(
-        (kmp_int32 *)lhs, *(kmp_int32 *)&old_value, *(kmp_int32 *)&new_value)) {
+    while (!KMP_COMPARE_AND_STORE_ACQ32((kmp_int32 *)lhs,
+                                        *(kmp_int32 *)&old_value,
+                                        *(kmp_int32 *)&new_value)) {
       KMP_CPU_PAUSE();
 
       old_value = *(kmp_int32 *)lhs;
@@ -3574,8 +3584,9 @@ void __kmpc_atomic_8(ident_t *id_ref, int gtid, void *lhs, void *rhs,
     old_value = *(kmp_int64 *)lhs;
     (*f)(&new_value, &old_value, rhs);
     /* TODO: Should this be acquire or release? */
-    while (!KMP_COMPARE_AND_STORE_ACQ64(
-        (kmp_int64 *)lhs, *(kmp_int64 *)&old_value, *(kmp_int64 *)&new_value)) {
+    while (!KMP_COMPARE_AND_STORE_ACQ64((kmp_int64 *)lhs,
+                                        *(kmp_int64 *)&old_value,
+                                        *(kmp_int64 *)&new_value)) {
       KMP_CPU_PAUSE();
 
       old_value = *(kmp_int64 *)lhs;
diff --git a/openmp/runtime/src/kmp_dispatch.cpp b/openmp/runtime/src/kmp_dispatch.cpp
index 648332109dbb..d3dd63f57401 100644
--- a/openmp/runtime/src/kmp_dispatch.cpp
+++ b/openmp/runtime/src/kmp_dispatch.cpp
@@ -1359,9 +1359,9 @@ int __kmp_dispatch_next_algorithm(int gtid,
         vnew.b = vold.b;
         vnew.p.count++; // get chunk from head of self range
         while (!KMP_COMPARE_AND_STORE_REL64(
-            (volatile kmp_int64 *)&pr->u.p.count,
-            *VOLATILE_CAST(kmp_int64 *) & vold.b,
-            *VOLATILE_CAST(kmp_int64 *) & vnew.b)) {
+                   (volatile kmp_int64 *)&pr->u.p.count,
+                   *VOLATILE_CAST(kmp_int64 *) & vold.b,
+                   *VOLATILE_CAST(kmp_int64 *) & vnew.b)) {
           KMP_CPU_PAUSE();
           vold.b = *(volatile kmp_int64 *)(&pr->u.p.count);
           vnew.b = vold.b;

It turned out this patch does change behavior.

-  while (
-      FormatTok->isOneOf(tok::identifier, tok::kw_requires, tok::coloncolon)) {
+  while (FormatTok->isOneOf(tok::identifier, tok::kw_requires,
+                            tok::coloncolon)) {

So what do I do?

IMO, refactoring and cleaning up code should be NFC. You can first add while, switch, etc (if it makes sense) in one patch and then refactor in another patch (or vice versa). It would be much easier to review.

It turned out this patch does change behavior.

-  while (
-      FormatTok->isOneOf(tok::identifier, tok::kw_requires, tok::coloncolon)) {
+  while (FormatTok->isOneOf(tok::identifier, tok::kw_requires,
+                            tok::coloncolon)) {

So what do I do?

IMO, refactoring and cleaning up code should be NFC. You can first add while, switch, etc (if it makes sense) in one patch and then refactor in another patch (or vice versa). It would be much easier to review.

+1

sstwcw updated this revision to Diff 419006.Mar 29 2022, 4:20 PM
sstwcw retitled this revision from [clang-format] Clean up code looking for if statements to [clang-format] Clean up code looking for if statements NFC.
owenpan requested changes to this revision.Mar 31 2022, 2:59 AM
owenpan added inline comments.
clang/lib/Format/TokenAnnotator.cpp
252–257

I don't think this is NFC.
Before:

} else if (OpeningParen.Previous &&
           (OpeningParen.Previous->isOneOf(tok::kw_static_assert,
                                           tok::kw_while, tok::l_paren,
                                           tok::comma, tok::kw_if,
                                           TT_BinaryOperator) ||
            OpeningParen.Previous->endsSequence(tok::kw_constexpr,
                                                tok::kw_if) ||
            OpeningParen.Previous->endsSequence(tok::identifier,
                                                tok::kw_if))) {

After:

} else if ((OpeningParen.is(tok::l_paren) &&
            OpeningParen.is(TT_ConditionLParen)) ||
           // PreviousNonComment = OpeningParen.getPreviousNonComment()
           (PreviousNonComment &&
            PreviousNonComment->isOneOf(tok::kw_if, tok::kw_while,
                                        tok::kw_switch, tok::kw_case,
                                        tok::kw_constexpr)) ||
           (OpeningParen.Previous &&
            OpeningParen.Previous->isOneOf(tok::kw_static_assert,
                                           tok::l_paren, tok::comma,
                                           TT_BinaryOperator))) {
This revision now requires changes to proceed.Mar 31 2022, 2:59 AM
owenpan added inline comments.Apr 1 2022, 5:26 PM
clang/lib/Format/TokenAnnotator.cpp
252–257

After:

} else if ((OpeningParen.is(tok::l_paren) &&
            OpeningParen.is(TT_ConditionLParen)) ||
            ...
                                        tok::kw_constexpr)) ||
           ...

After:

} else if ((OpeningParen.is(tok::l_paren) &&
            (OpeningParen.is(TT_ConditionLParen) ||
            ...
                                         tok::kw_constexpr)))) ||
           ...
sstwcw updated this revision to Diff 420031.Apr 3 2022, 1:52 AM
sstwcw retitled this revision from [clang-format] Clean up code looking for if statements NFC to [clang-format] Clean up code looking for if statements.
sstwcw edited the summary of this revision. (Show Details)
sstwcw marked 2 inline comments as done.Apr 3 2022, 1:56 AM
sstwcw added inline comments.
clang/lib/Format/TokenAnnotator.cpp
252–257

I removed NFC from the title. It would affect things like this:

new:
switch (x * x)
old:
switch (x *x)

However the entire llvm codebase doesn't seem to have such things.

Please see D123741. It should make it easier to see if you are making NFC changes at other places as well.

clang/lib/Format/TokenAnnotator.cpp
252–257

Nevertheless, your example is valid C++ and may exists in other codebases.

owenpan added inline comments.Apr 13 2022, 3:31 PM
clang/lib/Format/TokenAnnotator.cpp
133–136

Can you split it to a separate patch? Unlike the rest of this patch, it's likely to get accepted.