[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Erik Nyquist via Phabricator via cfe-commits]

enyquist added inline comments.


================
Comment at: lib/Format/WhitespaceManager.cpp:413
+
+  while (Param && !Param->is(tok::l_paren)) {
+    if (!Param->is(tok::identifier) && !Param->is(tok::comma))
----------------
djasper wrote:
> enyquist wrote:
> > djasper wrote:
> > > I think you should be able to use Current.MatchingParen here.
> > Hmm, I couldn't make this work... I just replaced this line with
> > 
> >     while (Param && Param != Current->MatchingParen)
> > 
> > But it must not be doing what I think it's doing, since it made some tests fail.
> > Mind you, my C-brain might be taking over here, please let me know if I'm using MatchingParen incorrectly
> You shouldn't need a while loop. Just:
> 
>   if (!Current->MatchingParen || !Current->MatchingParen->Previous)
>     return false;
>   Param = Param->MatchingParen->Previous;
> 
> And with that, I think you can simplify all these methods a bit:
> 
>   static bool endsPPIdentifier(const FormatToken *Current) {
>     if (!Current->Next || Current->Next->SpacesRequiredBefore == 0)
>       return false;
> 
>     // If Current is a "(", skip past the parameter list.
>     if (Current->is(tok::r_paren)) {
>       if (!Current->MatchingParen || !Current->MatchingParen->Previous)
>         return false;
>       Current = Current->MatchingParen->Previous;
>     }
> 
>     const FormatToken *Keyword = Current->Previous;
>     if (!Keyword || !Keyword->is(tok::pp_define))
>       return false;
> 
>     return Current->is(tok::identifier);
>   }
> 
> Does that work? If so, I'd even suggest inlining this code directly into the lambda instead of pulling out a function.
It seems that MatchingParen does not get set for parens surrounding a macro-function parameter list. So for now, a loop is needed. I was able to clean it up, though, and I've inlined the whole thing in the lambda.


Repository:
  rL LLVM

https://reviews.llvm.org/D28462