This is an archive of the discontinued LLVM Phabricator instance.

[clang-format][NFC] Insert/remove braces in clang/lib/Format/
ClosedPublic

Authored by owenpan on May 22 2022, 12:33 AM.

Details

Summary

This patch is the result of running clang-format -style="{InsertBraces: true, RemoveBracesLLVM: true}" (version 09865ae) on clang/lib/Format/.

Diff Detail

Event Timeline

owenpan created this revision.May 22 2022, 12:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 22 2022, 12:33 AM
owenpan requested review of this revision.May 22 2022, 12:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 22 2022, 12:33 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
curdeius accepted this revision.May 22 2022, 2:08 AM

Ok. So we mainly missed braces on complex conditionals. LGTM.

This revision is now accepted and ready to land.May 22 2022, 2:08 AM
MyDeveloperDay accepted this revision.May 23 2022, 8:51 AM

LGTM too.. @owenpan this is one of my favourite features!!

owenpan added inline comments.
clang/lib/Format/UnwrappedLineParser.cpp
2769

From https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements, it's unclear whether the braces should be removed when a block has a do-while loop as its single statement. Should I put the braces back and add handling of do-while for RemoveBracesLLVM?

clang/lib/Format/WhitespaceManager.cpp
231–239

It would be nice to remove the braces of the else here. See https://github.com/llvm/llvm-project/issues/55663.

curdeius added inline comments.May 23 2022, 2:37 PM
clang/lib/Format/UnwrappedLineParser.cpp
2769

It's indeed not clear, I remember having paused over this code for a moment.
I believe that the best course of action is to grep for both cases in the codebase and see what's more frequent (and whether there's some sort of consensus).

clang/lib/Format/WhitespaceManager.cpp
231–239

+1. Would you fix it manually here then and fix it later?

owenpan added inline comments.May 23 2022, 3:23 PM
clang/lib/Format/UnwrappedLineParser.cpp
2769

Good idea. I don't think there's any consensus specifically about this but will ask on discourse.llvm.org. To err on the conservative side, I will manually add the braces back for now.

clang/lib/Format/WhitespaceManager.cpp
231–239

I will leave it as is and use it as a test case for later. Implementing this would add more complexity to RemoveBracesLLVM, which is complex enough already. I will look into it in a few months as I will be away for the summer.

LGTM too.. @owenpan this is one of my favourite features!!

I love it too! :D

owenpan added inline comments.Jun 8 2022, 4:02 PM
clang/lib/Format/UnwrappedLineParser.cpp
2769

See D126512.

clang/lib/Format/WhitespaceManager.cpp
231–239

See D127260.