[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:3779
+      return a>b?a:b;                            return a>b?a:b;
+    };                                         };
+
----------------
owenpan wrote:
> 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.
I wonder if I didn't do a final regeneration, lets check when I update


================
Comment at: clang/include/clang/Format/Format.h:3076
+  /// \version 16
+  bool RemoveSemiColons;
+
----------------
owenpan wrote:
> 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`.
I feel if we called it `RemoveSemicolonAfterFunctionBody` then adding extra use cases would then require a new option.. in the same way, `RemoveBracesLLVM` doesn't remove all Braces.. can we go with @owenpan suggestion of `RemoveSemicolon` I could go for

`RemoveExtraSemicolon`
`RemoveExtraSemicolons`

But I don't feel `Extra` adds anything extra, if you pardon the pune.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:843
+    bool CanContainBracedList, TokenType NextLBracesType,
+    bool IsFunctionBlock) {
   auto HandleVerilogBlockLabel = [this]() {
----------------
owenpan wrote:
> We don't need to add the parameter. See below.
Oh! thank goodness, I hated having to add that..


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:923
   }
 
   auto RemoveBraces = [=]() mutable {
----------------
owenpan wrote:
> So that we don't need to add a parameter to `parseBlock`.
I don't know why I didn't see this! much better approach


================
Comment at: clang/unittests/Format/FormatTest.cpp:26755
+               "class Foo {\n"
+               "  int getSomething() const { return something; };\n"
+               "};",
----------------
HazardyKnusperkeks wrote:
> What happens with 2 (or more) semicolons?
I'll add the test but with @owenpan suggestion it handles it


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135466/new/

https://reviews.llvm.org/D135466