[libcxx-commits] [PATCH] D129441: [libc++] Update clang-format style.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

For the record, I asked @var-const on Discord and clarified a bit:

> @philnik
>  What exactly is you question on D124789 <https://reviews.llvm.org/D124789>? Do you want to know what the setting does, or are you asking why I set it?
>
> @var-const
>  Sorry for not being clear -- asking why this setting is being set. IIUC, it diverges from LLVM (and from my preferred style 🙂 ), so I'm curious why you decided to do this change
>
> @philnik
>  Well, the short answer is: It's my preferred style (I mostly copied my own clang-format and tweaked it to 120 lines a good as possible and to requests from other people).
>  The answer to why it's my preferred style is that it shows interesting/unusual code. Having a single namespace is normal (`std::__1::{ranges, filesystem, ...}` in the case of libc++). Anything beyond that is something you might want to look at (i.e. why isn't it collapsed to a single `namespace std::__1::ranges::whatever`/what namespace am I actually in). This also discourages complex namespace structures instead of collapsing it to a single namespace declaration and re-opening the namespace a second time if you need to. This of course only works for C++17-and-newer code, but I don't think that's a problem in libc++. i.e.if you need `ranges::__copy::__impl` or whatever you should write `namespace ranges { /*stuff*/ } namespace ranges::__copy { /*stuff*/ } namespace ranges::__copy::__impl { /*stuff*/ }` instead of `namespace ranges { /*stuff*/ namespace __copy { /*stuff*/ namespace __impl { /*stuff*/ } } }`.
>
> @var-const
>  Thanks for explaining (and sorry I missed it in the patch). My preference is to avoid extra indentation. I feel that any extra indentation is mostly useful within a short scope, where a difference in indentation is a great visual aid to separate control structures from the "main" flow of the function, for example. However, typically contents of a namespace would be longer than a single screen, meaning that, in most cases, everything the reader sees will have an extra level of indentation. This way, indentation no longer helps distinguish between anything and effectively just reduces the column limit (which we generally want to extend). I could agree that it makes sense to indent short namespaces where contents fit into a screen, but that spawns the question of where to draw the line (and is it worthwhile to have different rules for these cases).
>
> I could also agree about visually indicating "interesting" namespaces, but I think that having an unnamed namespace in a source file or, similarly, some kind of an impl namespace in a header file is pretty common and not worth calling out. While closing and reopening the current namespace would work, it feels like a workaround.

I think we can agree that it doesn't work very well in libc++ as-is. However, I think it would be a good idea to have non-standard namespaces indicated in some way. I don't know how that should look, so I don't think it's a blocker.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129441/new/

https://reviews.llvm.org/D129441