[PATCH] Fix algo constexpr tests in Debug mode

[François Dumont]

On 9/30/19 11:03 AM, Jonathan Wakely wrote:
> On 28/09/19 23:12 +0200, François Dumont wrote:
>> Here is what I just commited.
>
> Thanks. In my branch I had a lot more _GLIBCXX20_CONSTEXPR additions
> in the debug mode headers. Is it not needed on __check_singular,
> __foreign_iterator etc. ?

Yes, I know, I just limited myself to fixing testsuite tests for the 
moment. I'll check if those need it too.

>
>> I try to use the asm trick in the _GLIBCXX_DEBUG_VERIFY_COND_AT but 
>> didn't notice any enhancement. So for now I kept my solution to just 
>> have a non-constexpr call compiler error.
>
> You won't see any improvement in the testsuite, because the compiler
> flags for the testsuite suppress diagnostic notes. But I'd expect it
> to give better output for users with the default compiler flags.
Ok, I didn't know, I'll give it another try then outside testsuite.
>
>
>> diff --git a/libstdc++-v3/include/bits/stl_algo.h 
>> b/libstdc++-v3/include/bits/stl_algo.h
>> index a672f8b2b39..f25b8b76df6 100644
>> --- a/libstdc++-v3/include/bits/stl_algo.h
>> +++ b/libstdc++-v3/include/bits/stl_algo.h
>> @@ -5054,8 +5054,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
>>    *  @param  __last1   Another iterator.
>>    *  @param  __last2   Another iterator.
>>    *  @param  __result  An iterator pointing to the end of the merged 
>> range.
>> -   *  @return         An iterator pointing to the first element 
>> <em>not less
>> -Â Â  *Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  than</em> @e val.
>> +   *  @return   An output iterator equal to @p __result + (__last1 - 
>> __first1)
>> +Â Â  *Â Â Â Â Â Â Â Â Â Â Â  + (__last2 - __first2).
>> Â Â  *
>> Â Â  *Â  Merges the ranges @p [__first1,__last1) and @p 
>> [__first2,__last2) into
>> Â Â  *Â  the sorted range @p [__result, __result + (__last1-__first1) +
>> @@ -5102,8 +5102,8 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO
>>    *  @param  __last2   Another iterator.
>>    *  @param  __result  An iterator pointing to the end of the merged 
>> range.
>>    *  @param  __comp    A functor to use for comparisons.
>> -   *  @return         An iterator pointing to the first element "not 
>> less
>> -Â Â  *Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  than" @e val.
>> +   *  @return   An output iterator equal to @p __result + (__last1 - 
>> __first1)
>> +Â Â  *Â Â Â Â Â Â Â Â Â Â Â  + (__last2 - __first2).
>> Â Â  *
>> Â Â  *Â  Merges the ranges @p [__first1,__last1) and @p 
>> [__first2,__last2) into
>> Â Â  *Â  the sorted range @p [__result, __result + (__last1-__first1) +
>
> These changes are fine but should have been a separate, unrelated
> commit.

Ok, sorry, I consider that just a comment change was fine.


>
>
>> @@ -157,10 +192,16 @@ namespace __gnu_debug
>> Â Â  *Â  otherwise.
>> Â  */
>> Â  template<typename _InputIterator>
>> +Â Â Â  _GLIBCXX20_CONSTEXPR
>> Â Â Â  inline bool
>> Â Â Â  __valid_range(_InputIterator __first, _InputIterator __last,
>> Â Â Â Â Â Â Â Â Â  typename _Distance_traits<_InputIterator>::__type& __dist)
>> Â Â Â  {
>> +#ifdef __cpp_lib_is_constant_evaluated
>> +Â Â Â Â Â  if (std::is_constant_evaluated())
>> +Â Â Â  // Detected by the compiler directly.
>> +Â Â Â  return true;
>> +#endif
>
> Should this be using the built-in, not the C++20 function?
>
>
> In practice it's probably equivalent, because the function is only
> going to be constant-evaluated when called from C++20 code, and in
> that case the std::is_constant_evaluated() function will be available.


Yes, this is why I did it this way. And moreover it is using std::pair 
move assignment operator which is also C++20 constexpr.

>
> It just seems inconsistent to use the built-in in one place and not in
> the other.

It is also the reason why the simple simple __valid_range is not using 
the other anymore.

Maybe once I'll have check all the algo calls I'll find out that this 
one need _GLIBCXX_CONSTEXPR.

I got the sensation that library is being 'constexpr' decorated only 
when needed and when properly Standardise so are the Debug helpers.

François