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

[Owen Pan via Phabricator via cfe-commits]

owenpan planned changes to this revision.
owenpan added inline comments.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:465
+
+bool UnwrappedLineParser::mightFitOnOneLine() const {
+  const auto ColumnLimit = Style.ColumnLimit;
----------------
HazardyKnusperkeks wrote:
> A bit explanation what it means that something //might// fit in one line would be nice.
Will do.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:479
+  if (!LastToken->isOneOf(tok::semi, tok::comment))
+    return true;
+
----------------
HazardyKnusperkeks wrote:
> Especially this one is not clear to me, why do we return true here?
We will remove braces only if the previous line (i.e. the line before the closing brace) ends with a semi or comment. I should move this check to the caller or rename the function.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:481
+
+  SmallVector<FormatToken *> SavedTokens;
+  for (const auto &Token : PreviousLine.Tokens) {
----------------
HazardyKnusperkeks wrote:
> Is a FormatToken "big", or expensive to copy? If not I'd save them directly, otherwise I'd prefer a unique_ptr.
We have to make a copy of the FormatToken here so that we can restore it after the TokenAnnotator modifies it directly.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:484
+    FormatToken *Tok = new FormatToken;
+    Tok->copyFrom(*Token.Tok);
+    SavedTokens.push_back(Tok);
----------------
We must save `Token.Children` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125137