[PATCH] Add PSTL internal namespace qualifications

[Thomas Rodgers]

A non-text attachment was scrubbed...
Name: 0001-Replace-direct-PSTL-uses-of-assert-with-a-macro.patch
Type: text/x-patch
Size: 11233 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/libstdc++/attachments/20190405/4218b97d/attachment.bin>
-------------- next part --------------

This patch cleans up the PSTL's use of assert() by replacing those
useages with a couple of macro definitions which c++config now provides.

	    * include/bits/c++config:
	    Add definition for __PSTL_ASSERT.
	    Add definition for __PSTL_ASSERT_MSG.
	    * include/pstl/algorithm_impl.h: Replace use of assert().
	    * include/pstl/numeric_impl.h: Replace use of assert().
	    * include/pstl/parallel_backend_tbb.h:
	    Replace use of assert().
	    Replace use of __TBB_ASSERT().

	    * include/pstl/parallel_backend_utils.h: Replace use of assert().

Thomas Rodgers writes:

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