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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D33440: clang-for... Daniel Jasper via Phabricator via cfe-commits
    • [PATCH] D33440: clan... Francois Ferrand via Phabricator via cfe-commits

Reply via email to