[PATCH] D67023: Diagnose use of ATOMIC_VAR_INIT

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D67023#1654704 <https://reviews.llvm.org/D67023#1654704>, @aaron.ballman wrote:

> In D67023#1654070 <https://reviews.llvm.org/D67023#1654070>, @jfb wrote:
>
> > I refer you to http://wg21.link/p0883
> >  I don’t think this diagnostic should be added to clang until p0883 is fully implemented, even just for C. Otherwise we leave users with no portable way to do the right thing without diagnostic.
>
>
> P0883 has not been adopted, that I can tell (it has strong support, but isn't [expected to be] a part of C++2a)?


I think it'll make it in as an NB comment.

> Also, this functionality is implemented by libc++ and libstdc++, so I'm not certain what you mean by "until p0883 is fully implemented" or why that paper would be implemented "even just for C". Are you suggesting to gate this deprecation for C functionality based on what C++ standard library implementations are doing?

Yes, because atomics are already hard enough to use correctly between C and C++. Once we've fixed C++ I'd like the diagnostic to be the same for both (modulo which standard it's deprecated in). I think we should add the diagnostic for C and C++ at the same time, and we should make sure there's no libc++ nor clang work required before diagnosing.

> I'm sorry if I'm being dense here, but I am still not seeing the issue you're concerned by. C has already deprecated this feature and is planning to remove it shortly. I am diagnosing that fact in C, which seems perfectly appropriate to do regardless of what C++ is doing in this space.
> 
> Users have a portable way to write their code already because `ATOMIC_VAR_INIT` does not do any special magic and no implementation requires a user to use it to achieve portable atomic initialization semantics. If they get the diagnostic (which only triggers in C mode and only if the macro was defined in stdatomic.h), they should remove the use of `ATOMIC_VAR_INIT` from their code, or disable the deprecation diagnostic. If neither of those options appeals to the user, they can write something along the lines of:
> 
>   _Atomic(int) i = 
>   #if defined(__cplusplus)
>   ATOMIC_VAR_INIT(12);
>   #else
>   12;
>   #endif
> 
> 
> (not that I think anyone will want to write that, but strict adherence to a broken part of both standards does not seem like something we want to encourage anyway -- this is deprecated functionality, so the whole point is to discourage its use.)

Is this correct for all implementations of `_Atomic` and `std::atomic` that clang supports, including non-lock-free types? We should make sure before diagnosing.


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

https://reviews.llvm.org/D67023