[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code
Marek Kurdej via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 6 03:10:02 PST 2022
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 - 1].InPPDirective)
+ return true;
+#if 1
+ const unsigned Position = Tokens->getPosition();
+ if (Position == 0)
----------------
Please remove unused code.
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:485
+ if (FormatTok->isNot(tok::r_brace) || StatementCount != 1 ||
+ IsPrecededByCommentOrPPDirective || // StartsWithBrace ||
+ precededByCommentOrPPDirective()) {
----------------
Please remove unused code.
================
Comment at: clang/unittests/Format/FormatTest.cpp:23224
+
+ EXPECT_EQ("if (a)\n"
+ " if (b)\n"
----------------
owenpan wrote:
> owenpan wrote:
> > MyDeveloperDay wrote:
> > > any reason these can't be verifyFormats?
> > Did you mean to add the expected part as a separate case? I don't think it would add any value if there are no braces to remove in the first place?
> > Did you mean to add the expected part as a separate case? I don't think it would add any value if there are no braces to remove in the first place?
>
> Nvm. I think you wanted something like `verifyFormat(PostformatCode, PreformatCode, Style)`? Yes, I could do that, but I would have to relax the restriction to calling `BracesRemover()` in Format.cpp, i.e. checking `isCpp()` instead.
:+1: you should be able to use the 2-argument version of `verifyFormat`, no?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116316/new/
https://reviews.llvm.org/D116316
More information about the cfe-commits
mailing list