[PATCH] D37813: clang-format: better handle namespace macros

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

> Ok, that's probably where our different opinions come from - I would want a macro to be formatted to reflect how it's implemented, because otherwise I'm going to be surprised when I look at the implementation, and I consider surprises to be something to avoid in programming in general, where possible.

As "author of the coding rules" (and thus writer of clang-format config), I would agree with you.
But I think the tool itself should not decide to enforce this : maybe there are cases where the macro is "conceptually" a namespace, but should be indented as a class ; in this case I think the tool should quite dumb, and simply do what was asked: if asked to format the macro as a namespace, it should do it, whatever the implementation is.

That said, on top of this "user choice" the tool could be smart, and automatically handle all macros based on their actual implementation: if the macro is actually implemented with a namespace (and there is no "explicit" definition of how it should be handled), then indeed the tool could format the whole block as a namespace, and so on... But unfortunately that would imply parsing numerous #include to get the definitions, and is definitely not the goal of this patch: this patch is really about letting the "user" decide how to handle the macro.

> That said, I'm also then a bit confused by your tests - they seem to currently format a mixture of namespace and class formatting, and combine the indentation of a class-scope with namespace end comments.

This should not be the case: the goal here is that the macro is handled exactly like a namespace.
The NamespaceEndCommentFixer tests indeed contain some indentation, but it is not significant and is there only because I copied existing test cases and followed the same "style": but these tests do not actually perform any indentation, they simply handle the "end comment".
Do you have any specific test which confuses you?


https://reviews.llvm.org/D37813