[libcxx-commits] [PATCH] D103983: [libc++][rfc] Improve atomic_fetch_(add|sub).*.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/atomic:2196-2212
+#ifdef DO_WE_WANT_TO_KEEP_THIS_COMPILER_EXTENSION
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY
 _Tp*
 atomic_fetch_add(volatile atomic<_Tp*>* __o, typename atomic<_Tp*>::difference_type __op) _NOEXCEPT
 {
     return __o->fetch_add(__op);
----------------
Quuxplusone wrote:
> This is not a compiler extension; I mean,
> ```
> atomic<int*> a;
> atomic_fetch_add(&a, 1);
> ```
> must keep working no matter what. https://godbolt.org/z/ajdEYPhfK
> I suggest that you keep it working — and simplify the implementation — by removing the bogus constraints on line 2187, and then delete lines 2196-2212.
> (I'm willing to commandeer, if what I'm saying isn't making sense to you.)
> This is not a compiler extension; I mean,
> ```
> atomic<int*> a;
> atomic_fetch_add(&a, 1);
> ```
> must keep working no matter what. https://godbolt.org/z/ajdEYPhfK

Correct, but that's not an extension. These two are https://godbolt.org/z/rs1oxYEMv
```
	atomic<int*> a;
	atomic_fetch_add<int>(&a, 1);

	atomic<int(*)()> f;
	atomic_fetch_add(&f, 1);
```

> I suggest that you keep it working — and simplify the implementation — by removing the bogus constraints on line 2187, and then delete lines 2196-2212.
> (I'm willing to commandeer, if what I'm saying isn't making sense to you.)

What you say makes sense, but I just want to know whether we want to keep these 2 extensions (bugs?).
There's no need to commandeer, I just want to decide which direction to take before implementing the changes and the tests.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103983