[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

I don't think cases where there is only a semicolon-less macro are particularly frequent/important either. You probably could even add a semicolon in those cases, right? How frequent are such cases in your codebase? Either way, they aren't fixed by this patch, so they aren't a good reason to move forward with it.

I still believe that this patch adds complexity for very little gain. And I am not even sure it is correct. isFunctionDeclarationName/getFunctionParameterList is just yet another heuristic that might go wrong. And it might go wrong in both ways. You might still miss some cases and you might start incorrectly formatting stuff as functions. Fundamentally clang-format just doesn't have enough info to do this correctly.

In https://reviews.llvm.org/D42729#993188, @Typz wrote:

> In https://reviews.llvm.org/D42729#993159, @djasper wrote:
>
> > I think this case is not important enough to fix. Please tell users to just delete the useless semicolon.
>
>
> I would agree if were simple to spot: but often this may manifest itself only with a missing space between the function parameters and the function body, which can easily be overlooked...


".. which can easily be overlooked". If they are overlooked, nobody cares about the space either, so no harm done ;).

> Another option may be to create new pass which "removes" that extra semicolon: this way we would both fix it and get things right on next pass.

I am not sure we can do this reliably enough.

> However, the issue with a function which contains only a macro and which is followed by another function which returns an custom type cannot so easily be fixed:
> 
>   void abort() {
>     FOO()
>   }
>   uint32_t bar() {}
> 
> 
> (note that this case works fine if the body of the function contains a semicolon or reserved keyword, or if the next function returns a base type [int, void...])


Repository:
  rC Clang

https://reviews.llvm.org/D42729