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

This revision was automatically updated to reflect the committed changes. Closed by commit rG533fbae8d8d8: [clang-format] Add experimental option to remove LLVM braces (authored by owenpan). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM go for it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

owenpan added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3398 +**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14` + Remove optional braces of control statements (``if``, ``else``, ``for``, curdeius wrote: > owenpan

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

curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/docs/ClangFormatStyleOptions.rst:3398 +**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14` + Remove optional braces of control

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

owenpan added a comment. @MyDeveloperDay @curdeius I will land this patch if you have no more comments. :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 ___ cfe-commits mailing list

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

owenpan updated this revision to Diff 399869. owenpan added a comment. - Fixed a bug that missed braces in nested blocks. - Added a test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 Files: clang/docs/ClangFormatStyleOptions.rst

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

owenpan updated this revision to Diff 399632. owenpan added a comment. - Rebased to main. - Addressed review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 Files: clang/docs/ClangFormatStyleOptions.rst

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

HazardyKnusperkeks resigned from this revision. HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:3059 + /// and ``while``) in C++ according to the LLVM coding style. + /// \warning + /// This option will be renamed and expanded to

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

owenpan added inline comments. Comment at: clang/include/clang/Format/Format.h:3059 + /// and ``while``) in C++ according to the LLVM coding style. + /// \warning + /// This option will be renamed and expanded to support other styles! HazardyKnusperkeks

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

HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:3059 + /// and ``while``) in C++ according to the LLVM coding style. + /// \warning + /// This option will be renamed and expanded to support other styles! This should

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

owenpan updated this revision to Diff 398860. owenpan added a comment. This revision is now accepted and ready to land. - Removed unnecessary code and cleaned up. - Tested on `clang` and `check-clang` again. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/

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

owenpan updated this revision to Diff 398419. owenpan added a comment. Fixed a bug and added a test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst

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

owenpan updated this revision to Diff 398415. owenpan added a comment. - Braces should not be removed if the single-statement block might wrap or is already wrapped. - Added test cases. - Did a final round of cleanup. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/

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

owenpan updated this revision to Diff 398308. owenpan added a comment. - Rebased to `main`. - Removed unused code. - Updated release notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 Files: clang/docs/ClangFormatStyleOptions.rst

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

owenpan updated this revision to Diff 398231. owenpan added a comment. - Use `verifyFormat` instead of `EXPECT_EQ`. - Make this option have less impact on the unwrapped line parser by checking `Style.RemoveBracesLLVM`. - Remove some unused code. CHANGES SINCE LAST ACTION

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

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:435-454 +bool UnwrappedLineParser::precededByCommentOrPPDirective() const { + const size_t size = Lines.size(); + if (size > 0 && Lines[size - 1].InPPDirective) +return true; +#if 1 +

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

curdeius accepted this revision. curdeius added a comment. LGTM with some nits. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:435-454 +bool UnwrappedLineParser::precededByCommentOrPPDirective() const { + const size_t size = Lines.size(); + if (size > 0 && Lines[size -

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

owenpan added a comment. In D116316#3224591 , @MyDeveloperDay wrote: > Owen I think we should push ahead with this rather than D95168: > [clang-format] Add Insert/Remove Braces option > as I've looked at what you've

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Owen I think we should push ahead with this rather than D95168: [clang-format] Add Insert/Remove Braces option as I've looked at what you've

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

owenpan updated this revision to Diff 397783. owenpan added a comment. Fixing a couple of major bugs found by running check-clang plus minor bug fixes and cleanup: - In `parseLevel()`, `HasOpeningBrace` and `case tok::r_brace:` don't necessarily mean the tokens are `{` and `}`, respectively. -

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

owenpan updated this revision to Diff 397184. owenpan added a comment. - Fixed a bug found by applying the patch on the entire `clang` source tree. - Added an exception: the braces are not removed if the right one is followed by a comment on the same line. - Updated an existing test case and

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

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2303 + assert(IfRightBrace->MatchingParen == IfLeftBrace); + IfLeftBrace->MatchingParen = nullptr; + IfRightBrace->MatchingParen = nullptr; HazardyKnusperkeks

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

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2303 + assert(IfRightBrace->MatchingParen == IfLeftBrace); + IfLeftBrace->MatchingParen = nullptr; + IfRightBrace->MatchingParen = nullptr; owenpan

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

owenpan added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:23224 + + EXPECT_EQ("if (a)\n" +" if (b)\n" owenpan wrote: > MyDeveloperDay wrote: > > any reason these can't be verifyFormats? > Did you mean to add the expected part

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

owenpan added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2303 + assert(IfRightBrace->MatchingParen == IfLeftBrace); + IfLeftBrace->MatchingParen = nullptr; + IfRightBrace->MatchingParen = nullptr; HazardyKnusperkeks

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

owenpan added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3398 +**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14` + Remove optional braces of control statements (``if``, ``else``, ``for``, MyDeveloperDay wrote: > Can

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

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2303 + assert(IfRightBrace->MatchingParen == IfLeftBrace); + IfLeftBrace->MatchingParen = nullptr; + IfRightBrace->MatchingParen = nullptr; Why null

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

owenpan updated this revision to Diff 396573. owenpan added a comment. Fixed a bug and added a test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h

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

HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2338 + KeepAncestorBraces(); + NestedTooDeep.push_back(false); if (FormatTok->is(tok::l_brace)) { owenpan wrote: > HazardyKnusperkeks wrote: > > I think it is

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

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3398 +**RemoveBracesLLVM** (``Boolean``) :versionbadge:`clang-format 14` + Remove optional braces of control statements (``if``, ``else``, ``for``, Can we agree on one

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

owenpan marked 2 inline comments as done. owenpan added a comment. In D116316#3211269 , @curdeius wrote: > Could you have a look at preceding reviews and see if there wasn't a similar > patch before? I conversed

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

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();

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

owenpan updated this revision to Diff 396493. owenpan added a comment. Addresses review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h

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

owenpan added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3419 + if (isa(D)) { vs. if (isa(D)) { + for (auto *A : D.attrs()) { for (auto *A : D.attrs()) + if (shouldProcessAttr(A)) { if

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

HazardyKnusperkeks added a comment. In D116316#3211269 , @curdeius wrote: > Could you have a look at preceding reviews and see if there wasn't a similar > patch before? > I think that this option is a bit too limited. > Only removing braces doesn't seem

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

MyDeveloperDay added a comment. See D95168: [clang-format] Add Insert/Remove Braces option Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316

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

curdeius added a comment. Could you have a look at preceding reviews and see if there wasn't a similar patch before? I think that this option is a bit too limited. Only removing braces doesn't seem enough. Also, one should probably be able to decide when to add/remove them by e.g. setting the

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

owenpan created this revision. owenpan added reviewers: MyDeveloperDay, HazardyKnusperkeks, curdeius, sammccall, aaron.ballman, krasimir. owenpan added a project: clang-format. owenpan requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. See