[libcxx-commits] [PATCH] D119295: [libc++] Guard warning pragmas

Wed Feb 9 11:10:13 PST 2022

philnik added inline comments.

================
Comment at: libcxx/include/__config:1445
+     _LIBCPP_PRAGMA(clang diagnostic ignored warning) _LIBCPP_FORCE_SEMICOLON
+#  define _LIBCPP_GCC_DIAGNOSTIC_IGNORED(warning) _LIBCPP_CLANG_DIAGNOSTIC_IGNORED(warning)
+#elif defined(__GNUG__)
----------------
Mordante wrote:
> I feel this is a bit misleading. This diagnostic is ignored on both GCC and Clang. Clang supports the gcc diagnostics so that works. However I wonder what happens when a GCC only compiler diagnostic is pushed.
> 
> Wouldn't it be better to have
> `_LIBCPP_CLANG_DIAGNOSTIC_IGNORED`, `_LIBCPP_GCC_DIAGNOSTIC_IGNORED`, and `_LIBCPP_COMPILER_DIAGNOSTIC_IGNORED`?
> Where `_LIBCPP_COMPILER_DIAGNOSTIC_IGNORED` uses `GCC diagnostic ignored warning`.
> 
> That name is still wrong when using other compilers, but that feels better than the current naming.
I also thought about the naming. `_LIBCPP_GCC_DIAGNOSTIC_IGNORED` didn't really feel right. Would `_LIBCPP_CLANG_GCC_DIAGNOSTIC_IGNORED` be better? And then `_LIBCPP_GCC_DIAGNOSTIC_IGNORED` only enabled on GCC.
I think `_LIBCPP_COMPILER_DIAGNOSTIC_IGNORED` is to general of a name. That sounds to me to much like some magic attribute that //all// compilers have to support (which would actually be great to have). @ldionne do you have any opinions here?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119295