[PATCH] D121756: [clang-format] Clean up code looking for if statements

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Please see D123741 <https://reviews.llvm.org/D123741>. It should make it easier to see if you are making NFC changes at other places as well.



================
Comment at: clang/lib/Format/TokenAnnotator.cpp:252-256
+    } else if (OpeningParen.isConditionLParen(/*IncludeFor=*/false) ||
+               (OpeningParen.Previous &&
+                OpeningParen.Previous->isOneOf(TT_BinaryOperator, tok::l_paren,
+                                               tok::comma,
+                                               tok::kw_static_assert))) {
----------------
sstwcw wrote:
> owenpan wrote:
> > owenpan wrote:
> > > 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))) {
> > > ```
> > > 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)))) ||
> >                ...
> > ```
> 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.
Nevertheless, your example is valid C++ and may exists in other codebases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756