krasimir marked an inline comment as done and an inline comment as not done.
krasimir added inline comments.
================
Comment at: lib/Format/ContinuationIndenter.cpp:44
+ int MatchingStackIndex = Stack.size() - 1;
+ while (MatchingStackIndex >= 0 && Stack[MatchingStackIndex].Tok != &Tok)
+ --MatchingStackIndex;
----------------
djasper wrote:
> I think this needs a long explanatory comment, possibly with an example of
> the problem it is actually solving.
>
> Also, I am somewhat skeptical as we are using this logic for all paren kinds,
> not just braces. That seems to be unnecessary work and also might be
> unexpected at some point (although I cannot come up with a test case where
> that would be wrong).
Thanks! Added an example and tweaked it a little bit so that it doesn't
traverse the stack unless it is visiting `tok::r_brace`.
================
Comment at: lib/Format/ContinuationIndenter.cpp:49
+ break;
+ while (MatchingStackIndex >= 0 &&
+ Stack[MatchingStackIndex].Tok != End->Next->MatchingParen)
----------------
djasper wrote:
> Maybe pull out a lambda:
>
> auto FindParenState = [&](const FormatToken *Tok) {
> while (MatchingStackIndex >= 0 && Stack[MatchingStackIndex].Tok != &Tok)
> --MatchingStackIndex;
> return MatchingStackIndex >= 0 ? &Stack[MatchingStackIndex] : nullptr;
> };
>
> Then you could write the rest as:
>
> ...
> FindParenState(&Tok);
> const auto* End = Tok.MatchingParen
> for (; End->Next; End = End->Next) {
> if (End->Next->CanBreakBefore || !End->Next->closesScope())
> break;
> auto ParenState = FindParenState(End->Next->MatchingParen);
> if (ParenState && ParenState.BreakBeforeClosingBrace)
> break;
> }
> return End->TotalLength - Tok.TotalLength + 1;
>
>
> (not entirely sure it's better)
Yeah, a lambda is good.
Repository:
rC Clang
https://reviews.llvm.org/D46519
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
