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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__config:1438
+#define _LIBCPP_PRAGMA(text) _Pragma(#text)
+#define _LIBCPP_FORCE_SEMICOLON static_assert(true, "")
+
----------------
Neither `_LIBCPP_PUSH_MACROS` nor `_LIBCPP_BEGIN_NAMESPACE_STD` end with the noise characters `();`, and I don't think `_LIBCPP_DIAGNOSTIC_PUSH` or `_LIBCPP_DIAGNOSTIC_POP` should, either. //Especially// since you have to go out of your way with this `static_assert` hack in order to make the `();` syntactically valid C++ code.
Just do
```
#define _LIBCPP_TOSTRING2(w) #w
#define _LIBCPP_DIAGNOSTIC_PUSH _Pragma("clang diagnostic push")
#define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED(w) _Pragma(_LIBCPP_TOSTRING2(clang diagnostic ignored w))
#define _LIBCPP_DIAGNOSTIC_POP _Pragma("clang diagnostic pop")
```
(Notice that in this branch we //know// we're on Clang, so we know `_Pragma` is supported. I initially agreed with Louis that the extra indirection of `_LIBCPP_PRAGMA` didn't seem to be buying us anything... but then I realized that what it was actually buying us was stringification! Which is important in this case, but `_LIBCPP_PRAGMA` is the wrong name for it, and in fact `<__config>` already provides `_LIBCPP_TOSTRING2` ready-made.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119295