We recently noticed that the unwrap_iter.h file was pushing macros, but
it was pushing them again instead of popping them at the end of the
file. This led to libc++ basically swallowing any custom definition of
these macros in user code:
#define min HELLO
#include <algorithm>
// min is not HELLO anymore, it's not defined
While investigating this issue, I noticed that our push/pop pragmas were
actually entirely wrong too. Indeed, instead of pushing macros like
`move`, we'd push `move(int, int)` in the pragma, which is not a valid
macro name. As a result, we would not actually push macros like `move`
-- instead we'd simply undefine them. This led to the following code not
working:
#define move HELLO
#include <algorithm>
// move is not HELLO anymore
Fixing the pragma push/pop incantations led to a cascade of issues
because we use identifiers like `move` in a large number of places, and
all of these headers would now need to do the push/pop dance.
This patch fixes all these issues. First, it adds a check that we don't
swallow important names like min, max, move or refresh as explained
above. This is done by augmenting the existing
system_reserved_names.gen.py test to also check that the macros are what
we expect after including each header.
Second, it fixes the push/pop pragmas to work properly and adds missing
pragmas to all the files I could detect a failure in via the newly added
test.
rdar://121365472
(cherry picked from commit 7b4622514d232ce5f7110dd8b20d90e81127c467)
If a user passes a comparator that doesn't satisfy strict weak ordering
(see https://eel.is/c++draft/algorithms#alg.sorting.general) to
a sorting algorithm, the algorithm can produce an incorrect result or
even lead
to an out-of-bounds access. Unfortunately, comprehensively validating
that a given comparator indeed satisfies the strict weak ordering
requirement is prohibitively expensive (see [the related
RFC](https://discourse.llvm.org/t/rfc-strict-weak-ordering-checks-in-the-debug-libc/70217)).
As a result, we have three independent sets of checks:
- assertions that catch out-of-bounds accesses within the algorithms'
implementation. These are relatively cheap; however, they cannot catch
the underlying cause and cannot prevent the case where an invalid
comparator would result in an incorrectly-sorted sequence without
actually triggering an OOB access;
- debug comparators that wrap a given comparator and on each comparison
check that if `(a < b)`, then `!(b < a)`, where `<` stands for the
user-provided comparator. This performs up to 2x number of comparisons
but doesn't affect the algorithmic complexity. While this approach can
find more issues, it is still a heuristic;
- a comprehensive check of the comparator that validates up to 100
elements in the resulting sorted sequence (see the RFC above for
details). The check is expensive but the 100 element limit can somewhat
compensate for that, especially for large values of `N`.
The first set of checks is enabled in the fast hardening mode while the
other two are only enabled in the debug mode.
This patch also removes the
`_LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK` macro that
previously was used to selectively enable the 100-element check.
Now this check is enabled unconditionally in the debug mode.
Also, introduce a new category
`_LIBCPP_ASSERT_SEMANTIC_REQUIREMENT`. This category is
intended for checking the semantic requirements from the Standard.
Typically, these are hard or impossible to completely validate, so
these checks are expected to be heuristic in nature and potentially
quite expensive.
See https://reviews.llvm.org/D150264 for additional background.
Fixes#71496
Introduce a new `argument-within-domain` category that covers cases
where the given arguments make it impossible to produce a correct result
(or create a valid object in case of constructors). While the incorrect
result doesn't create an immediate problem within the library (like e.g.
a null pointer dereference would), it always indicates a logic error in
user code and is highly likely to lead to a bug in the program once the
value is used.
Also introduce `_LIBCPP_ASSERT_PEDANTIC` for assertions violating which
results in a no-op or other benign behavior, but which may nevertheless
indicate a bug in the invoking code.
Notable things in this commit:
* refactors `__indirect_binary_left_foldable`, making it slightly
different (but equivalent) to _`indirect-binary-left-foldable`_, which
improves readability (a [patch to the Working Paper][patch] was made)
* omits `__cpo` namespace, since it is not required for implementing
niebloids (a cleanup should happen in 2024)
* puts tests ensuring invocable robustness and dangling correctness
inside the correctness testing to ensure that the algorithms' results
are still correct
[patch]: https://github.com/cplusplus/draft/pull/6734
This patch runs clang-format on all of libcxx/include and libcxx/src, in
accordance with the RFC discussed at [1]. Follow-up patches will format
the benchmarks, the test suite and remaining parts of the code. I'm
splitting this one into its own patch so the diff is a bit easier to
review.
This patch was generated with:
find libcxx/include libcxx/src -type f \
| grep -v 'module.modulemap.in' \
| grep -v 'CMakeLists.txt' \
| grep -v 'README.txt' \
| grep -v 'libcxx.imp' \
| grep -v '__config_site.in' \
| xargs clang-format -i
A Git merge driver is available in libcxx/utils/clang-format-merge-driver.sh
to help resolve merge and rebase issues across these formatting changes.
[1]: https://discourse.llvm.org/t/rfc-clang-formatting-all-of-libc-once-and-for-all
Found while running libc++'s tests with MSVC's STL.
`ranges::rotate_copy` takes `forward_iterator`s as this test's comment
banner correctly depicts. However, this test had bogus assertions
expecting that `ranges::rotate_copy` would be constrained away for
not-quite-**bidi** iterators. @philnik777 confirmed that these were
copy-paste relics from the `ranges::reverse_copy` test.
I fixed this by replacing the assertions with the test types that aren't
quite **forward** iterators/ranges. Additionally, I noticed that the
top-level `test()` function was missing coverage with the weakest
possible `forward_iterator<int*>`.
This revealed that the product code in `ranges_rotate_copy.h` was
similarly damaged. In addition to fixing it by taking `forward_iterator`
and `forward_range` as depicted in the Standard, this drops the
inclusion of `<__iterator/reverse_iterator.h>` as this algorithm doesn't
need `std::__reverse_range`.
In preparation for running clang-format on the whole code base, we are
also removing mentions of the legacy _LIBCPP_INLINE_VISIBILITY macro in
favor of the newer _LIBCPP_HIDE_FROM_ABI.
We're still leaving the definition of _LIBCPP_INLINE_VISIBILITY to avoid
creating needless breakage in case some older patches are checked-in
with mentions of the old macro. After we branch for LLVM 18, we can do
another pass to clean up remaining uses of the macro that might have
gotten introduced by mistake (if any) and remove the macro itself at the
same time. This is just a minor convenience to smooth out the transition
as much as possible.
See
https://discourse.llvm.org/t/rfc-clang-formatting-all-of-libc-once-and-for-all
for the clang-format proposal.
I don't know when, but at some point we lost test coverage to ensue that
all the headers are in the modulemap. This adds a test to make sure all
the headers (excluding a few which shouldn't be part of the modulemap)
are at least mentioned. This also fixes a few headers which bit-rotted
while we were missing the coverage.
When working on an OpenMP offloading backend for standard parallel
algorithms (https://github.com/llvm/llvm-project/pull/66968) we noticed
the need of a generalization of `__is_trivial_plus_operation`. This patch
merges `__is_trivial_equality_predicate` and `__is_trivial_plus_operation`
into `__desugars_to`, and in the future we might extend the latter to support
other binary operations as well.
Co-authored-by: Louis Dionne <ldionne.2@gmail.com>
1. Instead of using individual "boolean" macros, have an "enum" macro
`_LIBCPP_HARDENING_MODE`. This avoids issues with macros being
mutually exclusive and makes overriding the hardening mode within a TU
more straightforward.
2. Rename the safe mode to debug-lite.
This brings the code in line with the RFC:
https://discourse.llvm.org/t/rfc-hardening-in-libc/73925Fixes#65101
Before this patch, we would fail to implicitly convert the result of
predicates to bool, which means we'd potentially perform a copy or move
construction of the boolean-testable, which isn't allowed. The same
holds true for comparing iterators against sentinels, which is allowed
to return a boolean-testable type.
We already had tests aiming to ensure correct handling of these types,
but they failed to provide appropriate coverage in several cases due to
guaranteed RVO. This patch fixes the tests, adds tests for missing
algorithms and views, and fixes the actual problems in the code.
Fixes#69074
Resolves Issue #69083
The `_CopySegment` helper for `ranges::copy(join_view, out)` is not
`constexpr` causing rejection in `libc++` in a static assertion context
as in the issue snippet.
This PR addresses a smaller detail discussed in the code review for
https://github.com/llvm/llvm-project/pull/66968. Currently, some
functions in the `libc++` PSTL CPU backend have been appended with a
digit to indicate the number of input iterator arguments. However, there
is no need to change the name for each version as overloading can be
used instead. This PR will make the naming more consistent in the the
CPU and the proposed OpenMP backend.
This makes exception handling a lot simpler, since we don't have to convert any exceptions this way. Is also properly handles all the user-thrown exceptions.
Reviewed By: ldionne, #libc
Spies: arichardson, mstorsjo, libcxx-commits
Differential Revision: https://reviews.llvm.org/D154238
The _LIBCPP_PSTL_CUSTOMIZATION_POINT macro was assuming that the policy
was called _RawPolicy and referencing it by name. It happened to always
work but this was definitely accidental and an oversight in the original
implementation. This patch fixes that by passing the policy to the macro
explicitly. Noticed while reviewing #66968.
This brings most of the enable_ifs in libc++ to the same style. It also has the nice side-effect of reducing the size of names of these symbols, since the depedent return type is shorter.
Reviewed By: #libc, ldionne
Spies: ldionne, libcxx-commits
Differential Revision: https://reviews.llvm.org/D157787
This brings most of the enable_ifs in libc++ to the same style. It also has the nice side-effect of reducing the size of names of these symbols, since the depedent return type is shorter.
Reviewed By: #libc, ldionne
Spies: ldionne, libcxx-commits
Differential Revision: https://reviews.llvm.org/D157736
The current chunking strategy is very bad for sorting, and we don't really know how to chunk in general. This fixes the performance problem for sorting.
Reviewed By: ldionne, #libc
Spies: libcxx-commits, krytarowski
Differential Revision: https://reviews.llvm.org/D155531
P2408 requires this for C++23, but implementing it in C++20 is safe
because the only code impacted would be code that violated a
precondition of the parallel algorithm. It was P2408 intent to
enable implementations to backport this to C++20.
Closes#63447 .
Reviewed By: philnik, #libc
Differential Revision: https://reviews.llvm.org/D154305
I'm getting a few -Wundefined-inline warnings, and a -Wnon-modular-include-in-module too. Fix all of those.
Reviewed By: Mordante, #libc
Differential Revision: https://reviews.llvm.org/D156508
The Apple.cmake cache wasn't set up properly, so we wouldn't enable
the libdispatch backend by default on Apple platforms. This patch
fixes the issue and adds a test.
We also need to make various drive-by fixes:
- Drop the usage of std::vector in libdispatch.h to avoid changing
the transitive includes only on Apple platforms.
- Fix includes
- Use __construct at since construct_at is unavailable in C++17
- Get rid of the (unused) __get_memory_resource function since that
adds a back-deployment requirement and we don't use it right now.
- Fix bugs in the chunking logic around boundary conditions.
Differential Revision: https://reviews.llvm.org/D155649