[PATCH] D33440: clang-format: better handle statement and namespace macros

Francois Ferrand via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 18 06:18:35 PDT 2017


Typz added a comment.

t>>! In https://reviews.llvm.org/D33440#812645, @djasper wrote:

> So, there are two things in this patch: Statement macros and namespace macros. Lets break this out and handle them individually. They really aren't related that much.

Indeed, the only "relation" is the implementation, which now uses the exact same list (internally) to match all macros. Phabricator makes it very difficult to work with related changes pushed as multiple reviews, so I ended up merging the two features.

> Statement macros:
>  I think clang-format's handling here is good enough. clang-format does not insert the line break, but it also doesn't remove it. I am not 100% sure here, so I an be convinced. But I want to understand the use cases better. Do you expect people to run into this frequently? I am essentially trying to understand whether the cost of an extra option is worth the benefit it is giving.

This happens relatively often, for example when fixing "unused parameter warning" on an inlined function: ``int f(int a) { return 0; }`` often gets fixed to ``int f(int a) { Q_UNUSED(a) return 0; }`` and clang-format does not fix the formatting...

> Namespace macros:
>  How important are the automatic closing comments to you? I'd say that we should punt on that and leave it to the user to fix comments of these. And then, we could try to make the things we already have in MacroBlockBegin detect whether it ends with an opening brace and not need an extra list here. What do you think?

This is not just about automatic closing comments, there are may differences: indentation of namespaces, 'compacting' of namespaces when `CompactNamespaces` is used, detection of inline functions (for putting on a single line with SFS_Inline), handling of empty blocks (i.e. use `BraceWrappingFlags.SplitEmptyNamespace`)...


https://reviews.llvm.org/D33440





More information about the cfe-commits mailing list