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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to