djasper 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))
----------------
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.
================
Comment at: lib/Format/WhitespaceManager.cpp:401
+ return Current->is(tok::identifier) && Current->Next &&
+ Current->Next->SpacesRequiredBefore == SpacesRequiredBefore;
+}
----------------
Note that this is comparing an unsigned with a bool, which might not work as
you expect. And especially, there are tokens for which we set
SpacesRequiredBefore to 2. Also I wonder whether we should check the
SpacesRequiredBefore of the r_paren in a function like macro, i.e. in
#define a(x) (x)
We should check that a space is required between "a(x)" and "(x)". Please take
a look at my longer code snippet below where I am proposing an alternative
solution.
https://reviews.llvm.org/D28462
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
