Re: [PATCH] Add PSTL internal namespace qualifications

[Jonathan Wakely <jwakely at redhat dot com>]

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).

> @@ -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(...).