[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

[Owen Pan via Phabricator via cfe-commits]

owenpan marked 4 inline comments as done.
owenpan added inline comments.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2173
+void UnwrappedLineParser::KeepAncestorBraces() {
+  const int MaxNestingLevels = 2;
+  const int Size = NestedTooDeep.size();
----------------
HazardyKnusperkeks wrote:
> constexpr or configurable?
It will be configurable when we expand removing braces to other styles.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2338
+  KeepAncestorBraces();
+  NestedTooDeep.push_back(false);
   if (FormatTok->is(tok::l_brace)) {
----------------
HazardyKnusperkeks wrote:
> I think it is worth to create a RAII type for that.
Can you explain why an RAII type would be beneficial here?


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2937
   // Parse the class body after the enum's ";" if any.
-  parseLevel(/*HasOpeningBrace=*/true);
+  parseLevel();
   nextToken();
----------------
HazardyKnusperkeks wrote:
> I would like it better if you insert both arguments.
> I don't really like default arguments.
I'm the opposite, and `parseBlock()`is a good example. However, I restored the previously non-default argument.


================
Comment at: clang/lib/Format/UnwrappedLineParser.h:196
   bool isOnNewLine(const FormatToken &FormatTok);
+  bool precededByCommentOrPPDirective();
+  void KeepAncestorBraces();
----------------
HazardyKnusperkeks wrote:
> I find it nicely if the order of functions in the cpp is the same as in the hpp.
Me too although some of the functions (e.g. `parseCSharpGenericTypeConstraint` and `parseCSharpAttribute`) were already out of order.


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

https://reviews.llvm.org/D116316