[PATCH] D98214: [clang-format] Fix aligning with linebreaks

Marek Kurdej via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 10 00:20:26 PST 2021


curdeius added inline comments.


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:332-340
       if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
           (ScopeStart > Start + 1 &&
            Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)) ||
+          (ScopeStart > Start + 1 &&
+           Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
+           Changes[ScopeStart - 1].Tok->is(tok::l_paren)) ||
           Changes[i].Tok->is(TT_ConditionalExpr) ||
----------------
HazardyKnusperkeks wrote:
> curdeius wrote:
> > Would it be possible to break up this condition and name it (or name its parts)? It's getting hard to follow.
> > Suggestion according to my understanding, which might be wrong.
> Can do, but then all those are always checked, there is no short circuit anymore. But I really get your point.
> 
> How about a lambda with different returns (and comments), that way we would still short circuit. Or some thing like
> ```
> bool AddShift = /* checks #1 */;
> AddShift = AddShift || /* checks #2 */;
> ...
> AddShift = AddShoft || /* checks #n */;
> 
> if (AddShift)
>   Changes[i].Spaces += Shift;
> ```
Good point on the short circuiting. A lambda may be a good solution here. But the `bool AddShift ...` that you suggested above (with comments too) is ok for me as well. I let you choose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98214



More information about the cfe-commits mailing list