djasper added inline comments.
================
Comment at: lib/Format/ContinuationIndenter.cpp:44
+ int MatchingStackIndex = Stack.size() - 1;
+ while (MatchingStackIndex >= 0 && Stack[MatchingStackIndex].Tok != &Tok)
+ --MatchingStackIndex;
----------------
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).
================
Comment at: lib/Format/ContinuationIndenter.cpp:49
+ break;
+ while (MatchingStackIndex >= 0 &&
+ Stack[MatchingStackIndex].Tok != End->Next->MatchingParen)
----------------
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)
================
Comment at: lib/Format/ContinuationIndenter.h:215
+ /// \brief The token opening this parenthesis level, or nullptr if this level
+ /// is opened by fake parenthesis.
+ const FormatToken *Tok;
----------------
The comment should include that this is not considered for memoization as the
same state will always be represented by the same token (similar to other
below). I wonder whether we should actually move the fields that don't affect
memoization out to a different structure at some point.
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
