owenpan requested changes to this revision.
owenpan added a comment.
This revision now requires changes to proceed.
Because this patch would impact inserting/removing braces, we must test it
against a large codebase. Before I landed `RemoveBracesLLVM` and
`InsertBraces`, I had tested them with `make check-clang` and compared the
result with that of running the same test without the options. I'm not sure if
this patch would be worth the effort as the new function only refactored code
at two places.
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2716-2717
+void UnwrappedLineParser::parseIndentedBlock(bool BracesAreOptional,
+ bool RBraceOnSeparateLine) {
+ CompoundStatementIndenter Indenter(this, Style, Line->Level);
----------------
The suggested names are merely my preference.
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2718-2723
+ CompoundStatementIndenter Indenter(this, Style, Line->Level);
+
+ keepAncestorBraces();
+
+ if (FormatTok->is(tok::l_brace)) {
+ FormatToken *LeftBrace = FormatTok;
----------------
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2725
+ parseBlock();
+ if (BracesAreOptional && Style.RemoveBracesLLVM) {
+ assert(!NestedTooDeep.empty());
----------------
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2736
+
+ if (Style.RemoveBracesLLVM)
+ NestedTooDeep.pop_back();
----------------
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2758
- keepAncestorBraces();
-
- if (FormatTok->is(tok::l_brace)) {
- FormatToken *LeftBrace = FormatTok;
- CompoundStatementIndenter Indenter(this, Style, Line->Level);
- parseBlock();
- if (Style.RemoveBracesLLVM) {
- assert(!NestedTooDeep.empty());
- if (!NestedTooDeep.back())
- markOptionalBraces(LeftBrace);
- }
- addUnwrappedLine();
- } else {
- parseUnbracedBody();
- }
-
- if (Style.RemoveBracesLLVM)
- NestedTooDeep.pop_back();
+ parseIndentedBlock(/*BracesAreOptional=*/true);
}
----------------
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2765-2766
- keepAncestorBraces();
-
- if (FormatTok->is(tok::l_brace)) {
- CompoundStatementIndenter Indenter(this, Style, Line->Level);
- parseBlock();
- if (Style.BraceWrapping.BeforeWhile)
- addUnwrappedLine();
- } else {
- parseUnbracedBody();
- }
-
- if (Style.RemoveBracesLLVM)
- NestedTooDeep.pop_back();
+ parseIndentedBlock(/*BracesAreoptional=*/false,
+ /*RBraceOnSeparateLine=*/Style.BraceWrapping.BeforeWhile);
----------------
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2839
- keepAncestorBraces();
-
- if (FormatTok->is(tok::l_brace)) {
- CompoundStatementIndenter Indenter(this, Style, Line->Level);
- parseBlock();
- addUnwrappedLine();
- } else {
- addUnwrappedLine();
- ++Line->Level;
- parseStructuralElement();
- --Line->Level;
- }
-
- if (Style.RemoveBracesLLVM)
- NestedTooDeep.pop_back();
+ parseIndentedBlock();
}
----------------
We can't refactor here as `parseSwitch()` doesn't call `parseUnbracedBody()`,
which handles option `InsertBraces`.
================
Comment at: clang/lib/Format/UnwrappedLineParser.h:128
void parseTryCatch();
+ void parseIndentedBlock(bool BracesAreOptional = true,
+ bool RBraceOnSeparateLine = true);
----------------
HazardyKnusperkeks wrote:
> I'm no fan of default arguments.
> But we have them all around...
There is no need to have a default for the first parameter now. See line 2839
below.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121757/new/
https://reviews.llvm.org/D121757
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
