[libcxx-commits] [PATCH] D99691: [libcxx] adjusts formatting rules

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/.clang-format:5
 Language: Cpp
-Standard: Cpp03
+Standard: Cpp11
 
----------------
curdeius wrote:
> Mordante wrote:
> > curdeius wrote:
> > > Mordante wrote:
> > > > Quuxplusone wrote:
> > > > > Actually, I'd turn this all the way up to Cpp20 or however high it goes. The only interesting thing controlled by this option AFAIK is whether clang-format will do
> > > > > ```
> > > > > foo<bar<baz> > x;  // we must do this anywhere C++03-portability is required
> > > > > ```
> > > > > or
> > > > > ```
> > > > > foo<bar<baz>> x;  // we prefer this wherever possible
> > > > > ```
> > > > > Either way, blindly applying clang-format to //actual// libc++ code is going to screw it up one way or the other. But given that you're the only person relying on clang-format to format your patches AFAIK, and you're working exclusively on C++20 code (never on C++03 code), I think it's totally reasonable to check in a style file that specifies `Cpp20`.
> > > >  I also rely on clang-format for my patches and I'm not fond that this change allows to break C++98 code. So I prefer `std::vector<std::pair<int, int> >` over `std::vector<std::pair<int, int>>` and my C++03 unit tests break. Ideally I would like to have this option in clang format
> > > > ```
> > > > // clang-format Cpp03
> > > > std::vector<std::pair<int, int> > v3;
> > > > // clang-format Cpp11
> > > > std::vector<std::pair<int, int>> v11;
> > > > ```
> > > > Then we can use a sane default and override it where applicable. I'll reach out to the clang-format developers to see how feasible this would be.
> > > Same for me. Changing to `Standard: Cpp11` will possibly break code with no huge benefit IMO.
> > > 
> > > Now, taking my clang-format dev's hat...
> > > Concerning @Mordante's suggestion, that may be doable (preferably in a more general form e.g. `// clang-format Option: Value` or even `// clang-format {Style...}`), but not sure if worth doing, as it will possibly make the formatting inconsistent between different parts of the code if used injudiciously.
> > > And, the whole purpose of using clang-format is consistency.
> > > Now, taking my clang-format dev's hat...
> > > Concerning @Mordante's suggestion, that may be doable (preferably in a more general form e.g. `// clang-format Option: Value` or even `// clang-format {Style...}`), but not sure if worth doing, as it will possibly make the formatting inconsistent between different parts of the code if used injudiciously.
> > > And, the whole purpose of using clang-format is consistency.
> > I also love consistency but if a file has C++20 and C++03 code I wouldn't mind to have them format differently based on the language features. I think our mixing of C++ version is uncommon, mainly standard libraries and boost so I wouldn't mind if the `// clang-format CppXX` may only be used once in a file and affects the entire file. (Since I'm no clang-format dev I leave the best naming to you.)
> > 
> > Do you have better suggestions how mixing the C++ version on a per file basis can be achieved easier in clang-format?
> > 
> > 
> No, I don't have for what clang-format offers now.
> It's a pity though that there's no option `SpacesInAngles: Keep/Leave` (because we are speaking of this aspect that `Standard` option changes, at least partially as `SpacesInAngles: true` adds space after the opening `<` as well). This might be the best way to go on a longer term (~release 13 at least).
> Personally, I would keep `Standard` option as is for the moment and wait for `SpacesInAngles: Keep/Leave` in clang-format.
> Personally, I would keep Standard option as is for the moment and wait for SpacesInAngles: Keep/Leave in clang-format.

Personally, I would keep this whole .clang-format file as-is, or even `git rm` it entirely, and just remember not to use `clang-format` to format libc++ submissions because `clang-format` can't format libc++ submissions correctly.

I also wouldn't like to see people sprinkling `//clang-format {on,off,Cpp03,Cpp11,whatever}` comments throughout their submissions — that kind of comment is not useful to the reader, and again is useful to the writer only if he's using clang-format blindly. (Which they're probably doing in the name of "consistency," which goal is defeated by sprinkling these comments everywhere.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99691