enyquist marked 2 inline comments as done.
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:
> > > 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.
> Fixed that in r300661. We really should not add more ways to parse parens,
> especially not outside of unwrapped lines (e.g. in the whitespace manager).
> That can lead to very bad situations for error recovering when the code is
> incomplete.
Great, thanks for fixing. Your change works.
================
Comment at: lib/Format/WhitespaceManager.cpp:431
+
+ // Special case for AlignTokens: for all other alignment cases,
+ // the current sequence is ended when a comma or a scope change
----------------
djasper wrote:
> I am not yet sure I understand this. How is this different from:
>
> $ clang-format test.cc -style="{AlignConsecutiveDeclarations: true}"
> map<int, int> m;
> map<int> m;
> int a;
I'm not sure exactly what you mean. Do you mean, why do I need this special
case to ignore scope changes and commas? This was the only way I could get it
to work, AlignTokens was bailing out as soon as a paren or a comma inside
parens was seen.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
