[libcxx-commits] [PATCH] D118800: [libc++] Normalize all our '#pragma GCC system_header', and regression-test.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.

LGTM with spaces consistently on all pragmas.



================
Comment at: libcxx/include/__format/formatter_pointer.h:27
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
-#  pragma GCC system_header
 #endif
----------------
Mordante wrote:
> Quuxplusone wrote:
> > Mordante wrote:
> > > The spaces in the `#if` is something we recently changed in the `.clang-format` file on @ldionne's request https://reviews.llvm.org/D103368#inline-1043939. So I would prefer to keep it that way. If we agree to remove the spaces, please also update the `.clang-format`. I know we don't use clang-format, but I've been using it in the formatter code since its inception and I prefer to keep using it.
> > @ldionne, do you want me to change just these few files, or do you want me to change all the rest of the files? I can go either way; it's a simple `sed` command to change all of them. Personally I'd //rather// touch the fewest files, because to me the extra couple of whitespaces cost 2 bytes per file and buy literally nothing; but I can `sed` them all if we want.
> > 
> > What I //won't// do in this PR is introduce inconsistency. For example, I'm not going to start adding `#  pragma` directives to some files while other files still contain ordinary `#pragma` directives.
> I agree, I too don't want inconsistencies! When we want to remove the spaces, then lets also revert the `.clang-format` patch that adds these spaces. I don't have a strong opinion on whether or not to add spaces.
Let's add the spaces in this PR too.

> because to me the extra couple of whitespaces cost 2 bytes per file and buy literally nothing

Do you indent your `if`s? Of course, you do. Why?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118800