[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:766
+  assert(!Tokens.empty());
+  const auto *LastToken = Tokens.back().Tok;
+  assert(LastToken);
----------------
curdeius wrote:
> It might be a matter of taste but adding this variable makes the code harder to read to me.
The last token of `ParsedLine` is of interest here. We want the annotator to compute its `TotalLength` to determine whether the line might fit on a single line. I'm open to renaming the variable if you have a better suggestion.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:777
+    SavedToken.Tok->copyFrom(*Token.Tok);
+    SavedToken.Children = std::move(Token.Children);
+  }
----------------
curdeius wrote:
> So the token's children are modified (moved)? Is it done so that children be not considered by the annotator?
Both the constructor and the destructor of `AnnotatedLine` clear the children of `UnwrappedLineNode`, so we save them beforehand and restore them afterward.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:787
+
+  const int Length = LastToken->TotalLength;
+
----------------
curdeius wrote:
> Why not like this?
> Why not like this?

See the assertion on line 781 above. We are computing the `TotalLength` of `LastToken` via `Line`. Either way works, but I prefer the simpler expression. I can change it though if you insist.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125137/new/

https://reviews.llvm.org/D125137