>> We shouldn't include <cassert> in the std::lib, the uses of assert >> should be changed to __glibcxx_assert instead (which is enabled by >> defining _GLIBCXX_ASSERTIONS). >> > > This has to come through one of the PSTL library configuration > macros because the "right assert" upstream won't be __glibcxx_assert.
There are a number of locations in the upstream PSTL sources where <cassert> is included and assert() is used. Additionally, the TBB backend uses __TBB_ASSERT(). I'm going to follow up with a separate patch that introduces __PSTL_ASSERT() and makes everything consistent, with __PSTL_ASSERT expanding to __glibcxx_assert in libstdc++. Tom. Thomas Rodgers writes: > Jonathan Wakely writes: > >> On 29/03/19 12:12 -0700, Thomas Rodgers wrote: >>>Prevent ADL related weirdness. >>> >>> * include/pstl/algorithm_impl.h: Add namespace qualification. >>> * include/pstl/execution_defs.h: Add namespace qualification. >>> * include/pstl/execution_impl.h: Add namespace qualification. >>> * include/pstl/numeric_impl.h: Add namespace qualification. >>> * include/pstl/parallel_backend_tbb.h: Add namespace qualification. >>> * include/pstl/unseq_backend_simd.h: Add namespace qualification. >>> * include/pstl/parallel_backend_utils.h: Include <cassert>. >> >> We shouldn't include <cassert> in the std::lib, the uses of assert >> should be changed to __glibcxx_assert instead (which is enabled by >> defining _GLIBCXX_ASSERTIONS). >> > > This has to come through one of the PSTL library configuration > macros because the "right assert" upstream won't be __glibcxx_assert. > >>>@@ -285,7 +286,7 @@ _ForwardIterator2 >>> __pattern_walk2_n(_ExecutionPolicy&&, _ForwardIterator1 __first1, _Size n, >>> _ForwardIterator2 __first2, _Function f, >>> _IsVector is_vector, /*parallel=*/std::false_type) >>> noexcept >> >> I missed these before, but we have non-uglified 'n' and 'f' and >> 'is_vector' here. Almost all uses of "is_vector" are inside a comment >> like /*is_vector=*/ but here it's a real parameter name. >> >> In practice if users do something stupid like >> #define n 123 >> #define f 456 >> then they won't be able to include these headers anyway, because >> TBB uses those as parameter names (so users that are using these >> headers with TBB can't defines such dumb macros). But when we get a >> non-TBB backend that won't be the case, and we should be uglifying all >> names declared in our own headers on principle. >> >>> { >>>- return __brick_walk2_n(__first1, n, __first2, f, is_vector); >>>+ return __internal::__brick_walk2_n(__first1, n, __first2, f, is_vector); >>> } >>> >>> template <class _ExecutionPolicy, class _RandomAccessIterator1, class >>> _Size, class _RandomAccessIterator2, >>>@@ -294,7 +295,7 @@ _RandomAccessIterator2 >>> __pattern_walk2_n(_ExecutionPolicy&& __exec, _RandomAccessIterator1 >>> __first1, _Size n, _RandomAccessIterator2 __first2, >>> _Function f, _IsVector is_vector, >>> /*parallel=*/std::true_type) >> >> And 'n' and 'f' here. >> >>> { >>>- return __pattern_walk2(std::forward<_ExecutionPolicy>(__exec), >>>__first1, __first1 + n, __first2, f, is_vector, >>>+ return >>>__internal::__pattern_walk2(std::forward<_ExecutionPolicy>(__exec), >>>__first1, __first1 + n, __first2, f, is_vector, >>> std::true_type()); >>> } >>> >> >> We can fix the 'n' and 'f' and 'is_vector' names in another patch, for >> this one please just remove <cassert> again and change assert(...) to >> __glibcxx_assert(...).