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

[Francois Ferrand via Phabricator via cfe-commits]

Typz 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.

Adding a semicolon works for indentation and behavior, but leads to compiler warning.
So a proper fix would require to change the macro, which can easily be tricky with macros containings branches or loops...

It should not happen that often, but in reality it actually happens often enough that I found the bug while simply testing clang-format...

> 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.

As such, it does not add such complexity I think, but indeed it is not completely correct: it works only in the case where no line breaks are expected around the body (since it is still parsed as a single UnwrappedLine). So it is a slight improvement, but not a complete fix.

`isFunctionDeclarationName()` is an heuristic as well, but provides better result and does not break the existing one (since there are actually quite a few tests, and none get broken by this patch :-) )

I think "overall" clang-format has enough information for this case, it is just not available at the right time: Unwrapping is done first, then token and matching parenthesis are analysed and lines anotated, and these token anotations are needed to improve the unwrapping behavior...
I would really want to call `isFunctionDeclarationName()` from `UnwrappedLineParser::calculateBraceTypes()`, but parenthesis matching, nesting level and operators identification are not available yet.

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

We want to use a formatter to ensure the code is consistent: e.g. to ensure things that may be overlooked by a human are actually formatted according to the rules.
We want people to trust the tool to do the right thing, so it is best to avoid as many errors as possible...


Repository:
  rC Clang

https://reviews.llvm.org/D42729