| Message ID | 6ccc9e9e-af8b-fd06-4ad3-4bc021aa364e@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light | expand |
Completing the tests revealed that this patch was missing a small change in include/bits/stl_iterator.h. Here is the updated patch. libstdc++: [_GLIBCXX_ASSERTIONS] Activate basic debug checks Use _GLIBCXX_ASSERTIONS as a _GLIBCXX_DEBUG light mode. When defined it activates all _GLIBCXX_DEBUG checks but skipping those requiring to loop through the iterator range unless in case of constexpr. libstdc++-v3/ChangeLog: * include/bits/stl_iterator.h [_GLIBCXX_ASSERTIONS]: Include <debug/stl_iterator.h>. * include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define debug macros non-empty. * include/debug/helper_functions.h: Cleanup comment about removed _Iter_base. * include/debug/functions.h (__skip_debug_runtime_check): New, returns false if _GLIBCXX_DEBUG is defined or if constant evaluated. (__check_sorted, __check_partitioned_lower, __check_partitioned_upper): Use latter. Ok to commit ? François On 27/05/21 7:37 pm, François Dumont wrote: > We have been talking for a long time of a debug mode with less impact > on performances. > > I propose to simply use the existing _GLIBCXX_ASSERTIONS macro. > > libstdc++: [_GLIBCXX_ASSERTIONS] Activate basic debug checks > > Use _GLIBCXX_ASSERTIONS as a _GLIBCXX_DEBUG light mode. When > defined it activates > all _GLIBCXX_DEBUG checks but skipping those requiring to loop > through the iterator > range unless in case of constexpr. > > libstdc++-v3/ChangeLog: > > * include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define > debug macros non-empty. > * include/debug/helper_functions.h: Cleanup comment about > removed _Iter_base. > * include/debug/functions.h (__skip_debug_runtime_check): > New, returns false if > _GLIBCXX_DEBUG is defined or if constant evaluated. > (__check_sorted, __check_partitioned_lower, > __check_partitioned_upper): Use latter. > > Tested under Linux x64. > > Ok to commit ? > > François >
On 27/05/21 19:37 +0200, François Dumont via Libstdc++ wrote: >We have been talking for a long time of a debug mode with less impact >on performances. We already have it, that's what _GLIBCXX_ASSERTIONS already is :-) >I propose to simply use the existing _GLIBCXX_ASSERTIONS macro. > > libstdc++: [_GLIBCXX_ASSERTIONS] Activate basic debug checks > > Use _GLIBCXX_ASSERTIONS as a _GLIBCXX_DEBUG light mode. When >defined it activates > all _GLIBCXX_DEBUG checks but skipping those requiring to loop >through the iterator > range unless in case of constexpr. > > libstdc++-v3/ChangeLog: > > * include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define >debug macros non-empty. > * include/debug/helper_functions.h: Cleanup comment about >removed _Iter_base. > * include/debug/functions.h (__skip_debug_runtime_check): >New, returns false if > _GLIBCXX_DEBUG is defined or if constant evaluated. > (__check_sorted, __check_partitioned_lower, >__check_partitioned_upper): Use latter. > >Tested under Linux x64. > >Ok to commit ? > >François > >diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h >index 116f2f023e2..2e6ce1c8a93 100644 >--- a/libstdc++-v3/include/debug/debug.h >+++ b/libstdc++-v3/include/debug/debug.h >@@ -61,7 +61,7 @@ namespace __gnu_debug > struct _Safe_iterator; > } > >-#ifndef _GLIBCXX_DEBUG >+#ifndef _GLIBCXX_ASSERTIONS > > # define __glibcxx_requires_cond(_Cond,_Msg) > # define __glibcxx_requires_valid_range(_First,_Last) >diff --git a/libstdc++-v3/include/debug/functions.h b/libstdc++-v3/include/debug/functions.h >index 6cac11f2abd..ee0eb877568 100644 >--- a/libstdc++-v3/include/debug/functions.h >+++ b/libstdc++-v3/include/debug/functions.h >@@ -48,6 +48,25 @@ namespace __gnu_debug > template<typename _Sequence> > struct _Is_contiguous_sequence : std::__false_type { }; > >+ _GLIBCXX20_CONSTEXPR Should this be simply _GLIBCXX_CONSTEXPR so that it can be constexpr in C++14 mode too? Or are there are never any debug checks in functions that are already constexpr in C++14 or C++17? >+ inline bool >+ __skip_debug_runtime_check() >+ { >+ // We could be here while only _GLIBCXX_ASSERTIONS has been defined. >+ // In this case we skip expensive runtime checks, constexpr will still >+ // be checked. >+ return >+#ifndef _GLIBCXX_DEBUG >+# if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED >+ !__builtin_is_constant_evaluated(); >+# else >+ true; >+# endif >+#else >+ false; >+#endif I think this would be simpler without the nesting, and without the preprocessor checks halfway through the return statement: #ifdef _GLIBCXX_DEBUG return false; #elif _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED return !__builtin_is_constant_evaluated(); #else return true; #endif >+ } >+ > /* 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.
On 03/06/21 2:31 pm, Jonathan Wakely wrote: > On 27/05/21 19:37 +0200, François Dumont via Libstdc++ wrote: >> We have been talking for a long time of a debug mode with less impact >> on performances. > > We already have it, that's what _GLIBCXX_ASSERTIONS already is :-) > >> I propose to simply use the existing _GLIBCXX_ASSERTIONS macro. >> >> libstdc++: [_GLIBCXX_ASSERTIONS] Activate basic debug checks >> >> Use _GLIBCXX_ASSERTIONS as a _GLIBCXX_DEBUG light mode. When >> defined it activates >> all _GLIBCXX_DEBUG checks but skipping those requiring to loop >> through the iterator >> range unless in case of constexpr. >> >> libstdc++-v3/ChangeLog: >> >> * include/debug/debug.h [_GLIBCXX_ASSERTIONS]: Define >> debug macros non-empty. >> * include/debug/helper_functions.h: Cleanup comment about >> removed _Iter_base. >> * include/debug/functions.h (__skip_debug_runtime_check): >> New, returns false if >> _GLIBCXX_DEBUG is defined or if constant evaluated. >> (__check_sorted, __check_partitioned_lower, >> __check_partitioned_upper): Use latter. >> >> Tested under Linux x64. >> >> Ok to commit ? >> >> François >> > >> diff --git a/libstdc++-v3/include/debug/debug.h >> b/libstdc++-v3/include/debug/debug.h >> index 116f2f023e2..2e6ce1c8a93 100644 >> --- a/libstdc++-v3/include/debug/debug.h >> +++ b/libstdc++-v3/include/debug/debug.h >> @@ -61,7 +61,7 @@ namespace __gnu_debug >> struct _Safe_iterator; >> } >> >> -#ifndef _GLIBCXX_DEBUG >> +#ifndef _GLIBCXX_ASSERTIONS >> >> # define __glibcxx_requires_cond(_Cond,_Msg) >> # define __glibcxx_requires_valid_range(_First,_Last) >> diff --git a/libstdc++-v3/include/debug/functions.h >> b/libstdc++-v3/include/debug/functions.h >> index 6cac11f2abd..ee0eb877568 100644 >> --- a/libstdc++-v3/include/debug/functions.h >> +++ b/libstdc++-v3/include/debug/functions.h >> @@ -48,6 +48,25 @@ namespace __gnu_debug >> template<typename _Sequence> >> struct _Is_contiguous_sequence : std::__false_type { }; >> >> + _GLIBCXX20_CONSTEXPR > > Should this be simply _GLIBCXX_CONSTEXPR so that it can be constexpr > in C++14 mode too? Or are there are never any debug checks in > functions that are already constexpr in C++14 or C++17? > >> + inline bool >> + __skip_debug_runtime_check() >> + { >> + // We could be here while only _GLIBCXX_ASSERTIONS has been >> defined. >> + // In this case we skip expensive runtime checks, constexpr will >> still >> + // be checked. >> + return >> +#ifndef _GLIBCXX_DEBUG >> +# if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED >> + !__builtin_is_constant_evaluated(); >> +# else >> + true; >> +# endif >> +#else >> + false; >> +#endif > > I think this would be simpler without the nesting, and without the > preprocessor checks halfway through the return statement: > > #ifdef _GLIBCXX_DEBUG > return false; > #elif _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED > return !__builtin_is_constant_evaluated(); > #else > return true; > #endif > > >> + } >> + >> /* 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. François
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
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 >
Any feedback ? Thanks On 08/08/21 9:34 pm, François Dumont wrote: > 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 >> >
diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h index 116f2f023e2..2e6ce1c8a93 100644 --- a/libstdc++-v3/include/debug/debug.h +++ b/libstdc++-v3/include/debug/debug.h @@ -61,7 +61,7 @@ namespace __gnu_debug struct _Safe_iterator; } -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS # define __glibcxx_requires_cond(_Cond,_Msg) # define __glibcxx_requires_valid_range(_First,_Last) diff --git a/libstdc++-v3/include/debug/functions.h b/libstdc++-v3/include/debug/functions.h index 6cac11f2abd..ee0eb877568 100644 --- a/libstdc++-v3/include/debug/functions.h +++ b/libstdc++-v3/include/debug/functions.h @@ -48,6 +48,25 @@ namespace __gnu_debug template<typename _Sequence> struct _Is_contiguous_sequence : std::__false_type { }; + _GLIBCXX20_CONSTEXPR + inline bool + __skip_debug_runtime_check() + { + // We could be here while only _GLIBCXX_ASSERTIONS has been defined. + // In this case we skip expensive runtime checks, constexpr will still + // be checked. + return +#ifndef _GLIBCXX_DEBUG +# if _GLIBCXX_HAVE_BUILTIN_IS_CONSTANT_EVALUATED + !__builtin_is_constant_evaluated(); +# else + true; +# endif +#else + false; +#endif + } + /* 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)); } template<typename _InputIterator, typename _Predicate> @@ -270,8 +290,9 @@ namespace __gnu_debug __check_sorted(const _InputIterator& __first, const _InputIterator& __last, _Predicate __pred) { - return __check_sorted_aux(__first, __last, __pred, - std::__iterator_category(__first)); + return __skip_debug_runtime_check() + || __check_sorted_aux(__first, __last, __pred, + std::__iterator_category(__first)); } template<typename _InputIterator> @@ -351,6 +372,9 @@ namespace __gnu_debug __check_partitioned_lower(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) { + if (__skip_debug_runtime_check()) + return true; + while (__first != __last && *__first < __value) ++__first; if (__first != __last) @@ -368,6 +392,9 @@ namespace __gnu_debug __check_partitioned_upper(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) { + if (__skip_debug_runtime_check()) + return true; + while (__first != __last && !(__value < *__first)) ++__first; if (__first != __last) @@ -387,6 +414,9 @@ namespace __gnu_debug _ForwardIterator __last, const _Tp& __value, _Pred __pred) { + if (__skip_debug_runtime_check()) + return true; + while (__first != __last && bool(__pred(*__first, __value))) ++__first; if (__first != __last) @@ -405,6 +435,9 @@ namespace __gnu_debug _ForwardIterator __last, const _Tp& __value, _Pred __pred) { + if (__skip_debug_runtime_check()) + return true; + while (__first != __last && !bool(__pred(__value, *__first))) ++__first; if (__first != __last) diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h index c0144ced979..587eba2f3e5 100644 --- a/libstdc++-v3/include/debug/helper_functions.h +++ b/libstdc++-v3/include/debug/helper_functions.h @@ -30,8 +30,8 @@ #define _GLIBCXX_DEBUG_HELPER_FUNCTIONS_H 1 #include <bits/move.h> // for __addressof -#include <bits/stl_iterator_base_types.h> // for iterator_traits, - // categories and _Iter_base +#include <bits/stl_iterator_base_types.h> // for iterator_traits and + // categories #include <bits/cpp_type_traits.h> // for __is_integer #include <bits/stl_pair.h> // for pair