owenpan added a comment. @MyDeveloperDay Cool!
In D135466#3843792 <https://reviews.llvm.org/D135466#3843792>, @HazardyKnusperkeks wrote: > Maybe even extend the option to other unnecessary semicolons like `int x = > 5;;` or other noop statements, one just has to be careful not to remove the > semicolon when it's the sole if/loop body. IMO we should keep it simple for now. We can always extend it to an `enum` or `struct` down the road, similar to what I plan to do with `RemoveBracesLLVM`. ================ Comment at: clang/docs/ClangFormatStyleOptions.rst:3761-3762 +**RemoveSemiColons** (``Boolean``) :versionbadge:`clang-format 16` + Removes unnecessary semicolons from the function braces + ---------------- Because "semicolon" is a single word. Also, FWIW I prefer the singular form here and would save the plural form for e.g. `InsertBraces` to mean a pair of braces. ================ Comment at: clang/docs/ClangFormatStyleOptions.rst:3770 + NOTE: + Setting this to false will not add `;` where they were missing + ---------------- IMO we don't need the note as it goes without saying. ================ Comment at: clang/docs/ClangFormatStyleOptions.rst:3779 + return a>b?a:b; return a>b?a:b; + }; }; + ---------------- lahwaacz wrote: > There is an extra `;` in the `true` case ;-) > If this file was generated from the Format.h header below, we have a bug in the dump_format_style.py script. ================ Comment at: clang/include/clang/Format/Format.h:3069-3071 + /// int max(int a, int b) int max(int a, int b) + /// { { + /// return a>b?a:b; return a>b?a:b; ---------------- LLVM style. ================ Comment at: clang/include/clang/Format/Format.h:3076 + /// \version 16 + bool RemoveSemiColons; + ---------------- HazardyKnusperkeks wrote: > The name is a bit hard, just from that it's not clear that this option is > harmless (modulo bugs). Maybe something like the said warning > `RemoveExtraSemiColons`? I'm ok with the short name, which makes the user to look it up in the documentation. The alternative would be something like `RemoveSemicolonAfterFunctionBody`. ================ Comment at: clang/lib/Format/Format.cpp:1971 + continue; + if (!Token->is(tok::semi)) + continue; ---------------- Nit. ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:843 + bool CanContainBracedList, TokenType NextLBracesType, + bool IsFunctionBlock) { auto HandleVerilogBlockLabel = [this]() { ---------------- We don't need to add the parameter. See below. ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:923 } auto RemoveBraces = [=]() mutable { ---------------- So that we don't need to add a parameter to `parseBlock`. ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:963 nextToken(/*LevelDifference=*/-AddLevels); HandleVerilogBlockLabel(); ---------------- The `while` loop would address https://reviews.llvm.org/D135466#inline-1306331 but is optional IMO because it wouldn't distinguish `};;` and the following: ``` }; ; ``` ================ Comment at: clang/lib/Format/UnwrappedLineParser.cpp:986-990 + if (IsFunctionBlock) { + FormatToken *Previous = Tokens->getPreviousToken(); + if (Previous && Previous->is(tok::r_brace)) + FormatTok->Optional = true; + } ---------------- This can be deleted. See above. Now we don't have to worry about if `parseParens()` and `parseStructuralElement()` would see a `};` sequence. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135466/new/ https://reviews.llvm.org/D135466 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
