Quuxplusone added a comment.
TLDR on `_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS`: I'm neutral. :)
================
Comment at: clang/docs/ReleaseNotes.rst:159-164
+ - The ``ATOMIC_VAR_INIT`` macro from ``<stdatomic.h>`` is now diagnosed as
+ deprecated in both C (C17 and later) and C++ (C++20 and later), and
+ ``ATOMIC_FLAG_INIT`` is now diagnosed as deprecated in C++ (C++20 and
later).
+ The diagnostic can be disabled by defining the
+ ``_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS`` macro prior to including the
+ header.
----------------
IIUC, `<stdatomic.h>` is not relevant to C++20; libc++'s `<atomic>` even
`#error`s if you include both `<stdatomic.h>` and `<atomic>` in the same TU.
Defining `_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS` won't affect the warnings in
C++20; for those you need `_LIBCPP_DISABLE_DEPRECATION_WARNINGS`.
C++20 isn't relevant to this section on "C Language Changes in Clang"; arguably
it could be listed under "C++ Language Changes in Clang," except that D115995
didn't change anything about //Clang//. So I think it's fine not to mention
D115995 anywhere in this file.
================
Comment at: clang/docs/UsersManual.rst:1128-1130
+ #include <stdint.h> // May include Clang CRT deprecation warnings
+ #define _CLANG_DISABLE_CRT_DEPRECATION_WARNINGS
+ #include <stdatomic.h> // Clang CRT deprecation warnings are disabled
----------------
It is //probably// sketchy to advise people to change the setting of
`_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS` over the lifetime of a single TU. If
`<stdint.h>` transitively includes `<stdatomic.h>`, this won't work. (Of course
that's unlikely in this //specific// case, but in general, IMHO we shouldn't
train people to think that this will always work.)
(In fact, `<stdatomic.h>` transitively includes `<stdint.h>`!)
We should train the programmer to think of
`_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS` as all-or-nothing: set it in your
build system, or set it at the very top of the TU.
================
Comment at: clang/lib/Headers/stdatomic.h:42-47
#define ATOMIC_VAR_INIT(value) (value)
+#if __STDC_VERSION__ >= 201710L || __cplusplus >= 202002L
+/* ATOMIC_VAR_INIT was deprecated in C17 and C++20. */
+#pragma clang deprecated(ATOMIC_VAR_INIT)
+#endif
#define atomic_init __c11_atomic_init
----------------
aaron.ballman wrote:
> Quuxplusone wrote:
> > Hmm, I do think there ought to be some way for the C++20 programmer to
> > suppress the deprecation warning for these macros (specifically, not just
> > via `-Wno-deprecated` in their whole project). For deprecations in the C++
> > standard library, libc++ offers an all-or-nothing flag: basically you'd do
> > ```
> > #if (__STDC_VERSION__ >= 201710L || __cplusplus >= 202002L) &&
> > !defined(_LIBCPP_DISABLE_DEPRECATION_WARNINGS)
> > /* ATOMIC_VAR_INIT was deprecated in C17 and C++20. */
> > #pragma clang deprecated(ATOMIC_VAR_INIT)
> > #endif
> > ```
> > (This also makes it easy for libcxx/test/ to suppress the deprecation
> > warning for the purposes of testing.)
> >
> > However, I'm not sure if it's appropriate to mention
> > `_LIBCPP_DISABLE_DEPRECATION_WARNINGS` in this header located in
> > clang/lib/Headers/ instead of libcxx/include/. Someone else will have to
> > make that call.
> >
> > It might be that the only way for the programmer (or libcxx/test/) to work
> > around the warning will be for them to pass `-Wno-deprecated` globally; IMO
> > that is suboptimal but quite far from disastrous.
> I think this is a reasonable idea. Microsoft has macros for a similar purpose
> with `_CRT_SECURE_NO_WARNINGS` (IIRC). I would not want to use `_LIBCPP_`
> for this purpose, but I could imagine it's useful to add something like
> `_CLANG_DISABLE_DEPRECATION_WARNINGS`. (I'm guessing we don't want
> per-deprecation granularity on the macro, but if we needed something like
> that, I think we could add it.)
>
> I went ahead and added `_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS` to the next
> version of the patch, and documented it in the release notes and user's
> manual. We can quibble over the name if you have better suggestions.
Re `_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS`: This naming scheme looks novel —
I don't see any other `_CLANG_{DIS,EN}ABLE.*` macros in `clang/lib/Headers/` —
but that might just be because its purpose is also novel. I'm not saying it's a
good idea, but I don't object to it nor have any particularly better idea.
Btw, where above I said `-Wno-deprecated`, it turns out that
`-Wno-deprecated-pragma` is slightly finer-grained. Also, I'm like 95% sure
that C++ is totally covered by D115995, which means whatever you do here is
relevant only to C17, which means I have no personal stake in its ergonomics. :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112221/new/
https://reviews.llvm.org/D112221
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
