aaron.ballman added a comment. In D67023#1656453 <https://reviews.llvm.org/D67023#1656453>, @jfb wrote:
> 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++. I don't see why that should preclude us from diagnosing C code. Not everyone shares C and C++ code, and C developers should be alerted to this deprecation independent of what C++ is doing. > 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. Given that C has already made this decision, it seems like the tail wagging the dog to wait for C++ to maybe adopt the same thing as part of an NB comment. >> 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. Let me put it another way: code that does not work with `= 12` will similarly not work with `= ATOMIC_VAR_INIT(12)` because the two are identical (modulo a `ParenExpr`) after preprocessing. So yes, I believe this is "correct" for all implementations of `_Atomic` and `std::atomic` that Clang supports insomuch as it's bug-for-bug-or-lack-thereof compatible. I feel like we're going in circles, so perhaps if I explained my problem a different way, it will resonate more. I am worried about C developers writing C code against a C compiler when the C committee has deprecated `ATOMIC_VAR_INIT` and want to diagnose this situation in C mode. What I hear you being concerned with is: what about C++ developers? I've pointed out that they have several options if they really want full portability to any compiler in C or C++ mode, but beyond that, I am not concerned about C++ developers. They have `ATOMIC_VAR_INIT` and will until such time as WG21 decides to deprecate it, at which point I would like to diagnose in C++ mode as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67023/new/ https://reviews.llvm.org/D67023 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
