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 + const unsigned Position = Tokens->getPosition(); + if (Position == 0) ---------------- curdeius wrote: > Please remove unused code. Will do pending D116318. ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:485 + if (FormatTok->isNot(tok::r_brace) || StatementCount != 1 || + IsPrecededByCommentOrPPDirective || // StartsWithBrace || + precededByCommentOrPPDirective()) { ---------------- curdeius wrote: > Please remove unused code. Will do. ================ Comment at: clang/unittests/Format/FormatTest.cpp:23224 + + EXPECT_EQ("if (a)\n" + " if (b)\n" ---------------- curdeius wrote: > 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? Will replace all `EXPECT_EQ`s with `verifyFormat`s. Question: when is the former preferred to the latter? It seems `EXPECT_EQ` is frequently used in FormatTest.cpp. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits