The implementation is fine and has the proper increment/decrement
operators defined, but the tests were wrong:
- a typo (`T` instead of `std::atomic_ref<T>`) when ensuring that increment/decrement
operators are not defined in the primary template and specialization for floating point
types, and
- the specialization for pointer types was miscategorized.
Adapted from libcxx/test/std/atomics/atomics.types.generic/atomics.types.float/exchange.pass.cpp
as we did for testing other functionalities. Spotted that lapse in coverage when working on #121414.
This patch adds a large number of missing includes in the libc++ headers
and the test suite. Those were found as part of the effort to move
towards a mostly monolithic top-level std module.
This patch increases the alignment requirement for std::atomic_ref
such that we can guarantee lockfree operations more often. Specifically,
we require types that are 1, 2, 4, 8, or 16 bytes in size to be aligned
to at least their size to be used with std::atomic_ref.
This is the case for most types, however a notable exception is
`long long` on x86, which is 8 bytes in length but has an alignment
of 4.
As a result of this patch, one has to be more careful about the
alignment of objects used with std::atomic_ref. Failure to provide
a properly-aligned object to std::atomic_ref is a precondition
violation and is technically UB. On the flipside, this allows us
to provide an atomic_ref that is actually lockfree more often,
which is an important QOI property.
More information in the discussion at https://github.com/llvm/llvm-project/pull/99570#issuecomment-2237668661.
Co-authored-by: Louis Dionne <ldionne.2@gmail.com>
The builtin __atomic_always_lock_free takes into account the type of the
pointer provided as the second argument. Because we were passing void*,
rather than T*, the calculation failed. This meant that
atomic_ref<T>::is_always_lock_free was only true for char & bool.
This bug exists elsewhere in the atomic library (when using GCC, we fail
to pass a pointer at all, and we fail to correctly align the atomic like
_Atomic would).
This change also attempts to start sorting out testing difficulties with
this function that caused the bug to exist by using the
__GCC_ATOMIC_(CHAR|SHORT|INT|LONG|LLONG|POINTER)_IS_LOCK_FREE predefined
macros to establish an expected value for `is_always_lock_free` and
`is_lock_free` for the respective types, as well as types with matching
sizes and compatible alignment values.
Using these compiler pre-defines we can actually validate that certain
types, like char and int, are actually always lock free like they are on
every platform in the wild.
Note that this patch was actually authored by Eric Fiselier but I picked
up the patch and GitHub won't let me set Eric as the primary author.
Co-authored-by: Eric Fiselier <eric@efcs.ca>
This speeds up the CI a bit (anecdotally ~10%) for those jobs, and it
also helps ensure that we are clean w.r.t. Clang modules when we disable
some of the carve-outs like no-localization or no-threads.
I noticed that these tests had empty `main` functions. Dropping them and
renaming the tests to `MEOW.compile.pass.cpp` will slightly improve test
throughput.
Found while running libc++'s tests with MSVC's STL.
* Avoid MSVC warning C5101: use of preprocessor directive in
function-like macro argument list is undefined behavior.
+ We can easily make this portable by extracting `const bool is_newlib`.
+ Followup to #73440.
+ See #73598.
+ See #73836.
* Avoid MSVC warning C4267: 'return': conversion from 'size_t' to 'int',
possible loss of data.
+ This warning is valid, but harmless for the test, so
`static_cast<int>` will avoid it.
* Avoid MSVC warning C4146: unary minus operator applied to unsigned
type, result still unsigned.
+ This warning is also valid (the scenario is sometimes intentional, but
surprising enough that it's worth warning about). This is a C++17 test,
so we can easily avoid it by testing `is_signed_v` at compile-time
before testing `m < 0` and `n < 0` at run-time.
* Silence MSVC warning C4310: cast truncates constant value.
+ These warnings are being emitted by `T(255)`. Disabling the warning is
simpler than attempting to restructure the code.
+ Followup to #79791.
* MSVC no longer emits warning C4521: multiple copy constructors
specified.
+ This warning was removed from the compiler, since at least 2021-12-09.
