[PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light

[François Dumont]

After further testing here a fixed version which imply less changes.

Moreover I already commit the fixes unrelated with this patch.

     libstdc++: [_GLIBCXX_ASSERTIONS] Activate basic debug checks

     libstdc++-v3/ChangeLog:

             * include/bits/stl_algobase.h (equal): Use runtime-only 
_GLIBCXX_DEBUG check.
             * include/bits/stl_iterator.h [_GLIBCXX_ASSERTIONS]: 
Include <debug/stl_iterator.h>.
             * include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define debug 
macros non-empty. Most of
             the time do a simple valid_range check.
             * include/debug/helper_functions.h: Cleanup comment about 
removed _Iter_base.
             (__gnu_debug::__valid_range): Add __skip_if_constexpr 
parameter and skip check when true
             and in a constexpr context.
             * include/debug/macros.h (_GLIBCXX_DEBUG_VERIFY): Define as 
__glibcxx_assert when only
             _GLIBCXX_ASSERTIONS is defined.
             (__glibcxx_check_valid_range): Add _SkipIfConstexpr parameter.
             (__glibcxx_check_can_increment_range): Likewise.
             * include/debug/safe_iterator.h (__valid_range): Adapt.
             * include/debug/safe_local_iterator.h (__valid_range): Adapt.
             * testsuite/24_iterators/istream_iterator/1.cc (test01): 
Skip iterator increment when
             _GLIBCXX_ASSERTIONS is defined.
             * testsuite/25_algorithms/copy/constexpr_neg.cc: New test.
             * testsuite/25_algorithms/heap/1.cc: Skip operation 
complexity checks when _GLIBCXX_ASSERTIONS
             is defined.

Ok to commit ?

François


On 06/08/21 4:52 pm, François Dumont wrote:
> On 07/06/21 6:25 am, François Dumont wrote:
>> On 03/06/21 2:31 pm, Jonathan Wakely wrote:
>>>
>>>> +  }
>>>> +
>>>>   /* Checks that [first, last) is a valid range, and then returns
>>>>    * __first. This routine is useful when we can't use a separate
>>>>    * assertion statement because, e.g., we are in a constructor.
>>>> @@ -260,8 +279,9 @@ namespace __gnu_debug
>>>>     inline bool
>>>>     __check_sorted(const _InputIterator& __first, const 
>>>> _InputIterator& __last)
>>>>     {
>>>> -      return __check_sorted_aux(__first, __last,
>>>> -                std::__iterator_category(__first));
>>>> +      return __skip_debug_runtime_check()
>>>> +    || __check_sorted_aux(__first, __last,
>>>> +                  std::__iterator_category(__first));
>>>
>>> Currently this function is never called at all ifndef _GLIBCXX_DEBUG.
>>> With this change, it's going to be present for _GLIBCXX_ASSERTIONS,
>>> and if it isn't inlined it's going to explode the code size.
>>>
>>> Some linux distros are already building the entire distro with
>>> _GLIBCXX_ASSERTIONS so I think we need to be quite careful about this
>>> kind of large change affecting every algo.
>>>
>>> So maybe we shouldn't enable these checks via _GLIBCXX_ASSERTIONS, but
>>> a new macro.
>>>
>> _GLIBCXX_DEBUG is already rarely used, so will be yet another mode.
>>
>> So let's forget about all this, thanks.
>>
> I eventually wonder if your feedback was limited to the use of 
> __check_sorted and some other codes perhaps.
>
> So here is another proposal which activate a small subset of the 
> _GLIBCXX_DEBUG checks in _GLIBCXX_ASSERTIONS but with far less code.
>
> First, the _Error_formatter is not used, the injected checks are 
> simply using __glibcxx_assert.
>
> Second I reduced the number of accitaved checks, mostly the 
> __valid_range.
>
> I also enhance the valid_range check for constexpr because sometimes 
> the normal implementation is good enough to let the compiler diagnose 
> a potential issue in this context. This is for example the case of the 
> std::equal implementation whereas the std::copy implementation is too 
> defensive.
>
>     libstdc++: [_GLIBCXX_ASSERTIONS] Activate basic debug checks
>
>     libstdc++-v3/ChangeLog:
>
>             * include/bits/stl_algobase.h (equal): Use runtime-only 
> _GLIBCXX_DEBUG check.
>             * include/bits/stl_iterator.h [_GLIBCXX_ASSERTIONS]: 
> Include <debug/stl_iterator.h>.
>             * include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define 
> debug macros non-empty. Most of
>             the time do a simple valid_range check.
>             * include/debug/helper_functions.h: Cleanup comment about 
> removed _Iter_base.
>             (__valid_range): Add __skip_if_constexpr parameter and 
> skip check when in a constexpr
>             context.
>             * include/debug/macros.h (_GLIBCXX_DEBUG_VERIFY): Define 
> as __glibcxx_assert when only
>             _GLIBCXX_ASSERTIONS is defined.
>             (__glibcxx_check_valid_range): Add _SkipIfConstexpr 
> parameter.
>             (__glibcxx_check_can_increment_range): Likewise.
>             * testsuite/24_iterators/istream_iterator/1.cc (test01): 
> Skip iterator increment when
>             _GLIBCXX_ASSERTIONS is defined.
>             * testsuite/25_algorithms/copy/constexpr_neg.cc: New test.
>             * testsuite/25_algorithms/heap/1.cc: Skip operation 
> complexity checks when _GLIBCXX_ASSERTIONS
>             is defined.
>             * 
> testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_neg.cc: 
> Fix dg-prune-output reason.
>             * 
> testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_pred_neg.cc: 
> Likewise.
>             * 
> testsuite/25_algorithms/lower_bound/debug/constexpr_valid_range_neg.cc: 
> Likewise.
>             * 
> testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_neg.cc: 
> Likewise.
>             * 
> testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_pred_neg.cc: 
> Likewise.
>             * 
> testsuite/25_algorithms/upper_bound/debug/constexpr_valid_range_neg.cc: 
> Likewise.
>
> The last fixes below are due to the recent changes to the 
> __glibcxx_assert macro but it is close to the code I am changing so I 
> prefer to fix those here.
>
> Tested under Linux x86_64 w/o _GLIBCXX_ASSERTIONS.
>
> Ok to commit ?
>
> François
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: assertion_mode.patch
Type: text/x-patch
Size: 15915 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20210808/f64389f9/attachment-0001.bin>