* [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light @ 2021-05-27 17:37 François Dumont 2021-05-31 17:17 ` François Dumont 2021-06-03 12:31 ` Jonathan Wakely 0 siblings, 2 replies; 7+ messages in thread From: François Dumont @ 2021-05-27 17:37 UTC (permalink / raw) To: libstdc++; +Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 1034 bytes --] 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 [-- Attachment #2: assertion_mode.patch --] [-- Type: text/x-patch, Size: 4206 bytes --] 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light 2021-05-27 17:37 [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light François Dumont @ 2021-05-31 17:17 ` François Dumont 2021-06-03 12:31 ` Jonathan Wakely 1 sibling, 0 replies; 7+ messages in thread From: François Dumont @ 2021-05-31 17:17 UTC (permalink / raw) To: libstdc++; +Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 2223 bytes --] 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 > [-- Attachment #2: assertion_mode.patch --] [-- Type: text/x-patch, Size: 4634 bytes --] diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h index 8768624b7d1..015b5da1d18 100644 --- a/libstdc++-v3/include/bits/stl_iterator.h +++ b/libstdc++-v3/include/bits/stl_iterator.h @@ -2385,7 +2385,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _GLIBCXX_END_NAMESPACE_VERSION } // namespace -#ifdef _GLIBCXX_DEBUG +#ifdef _GLIBCXX_ASSERTIONS # include <debug/stl_iterator.h> #endif 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light 2021-05-27 17:37 [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light François Dumont 2021-05-31 17:17 ` François Dumont @ 2021-06-03 12:31 ` Jonathan Wakely 2021-06-07 4:25 ` François Dumont 1 sibling, 1 reply; 7+ messages in thread From: Jonathan Wakely @ 2021-06-03 12:31 UTC (permalink / raw) To: François Dumont; +Cc: libstdc++, gcc-patches 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light 2021-06-03 12:31 ` Jonathan Wakely @ 2021-06-07 4:25 ` François Dumont 2021-08-06 14:52 ` François Dumont 0 siblings, 1 reply; 7+ messages in thread From: François Dumont @ 2021-06-07 4:25 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++, gcc-patches 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light 2021-06-07 4:25 ` François Dumont @ 2021-08-06 14:52 ` François Dumont 2021-08-08 19:34 ` François Dumont 0 siblings, 1 reply; 7+ messages in thread From: François Dumont @ 2021-08-06 14:52 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 4532 bytes --] 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 [-- Attachment #2: assertion_mode.patch --] [-- Type: text/x-patch, Size: 22902 bytes --] diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h index d0c49628d7f..d45eec4339b 100644 --- a/libstdc++-v3/include/bits/stl_algobase.h +++ b/libstdc++-v3/include/bits/stl_algobase.h @@ -1551,7 +1551,7 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO __glibcxx_function_requires(_EqualOpConcept< typename iterator_traits<_II1>::value_type, typename iterator_traits<_II2>::value_type>) - __glibcxx_requires_can_increment_range(__first1, __last1, __first2); + __glibcxx_requires_can_increment_range_runtime(__first1, __last1, __first2); return std::__equal_aux(__first1, __last1, __first2); } diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h index 3773d600b8f..37c348adfd3 100644 --- a/libstdc++-v3/include/bits/stl_iterator.h +++ b/libstdc++-v3/include/bits/stl_iterator.h @@ -2465,7 +2465,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _GLIBCXX_END_NAMESPACE_VERSION } // namespace -#ifdef _GLIBCXX_DEBUG +#ifdef _GLIBCXX_ASSERTIONS # include <debug/stl_iterator.h> #endif diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h index 116f2f023e2..d30ef0f25b2 100644 --- a/libstdc++-v3/include/debug/debug.h +++ b/libstdc++-v3/include/debug/debug.h @@ -61,12 +61,13 @@ 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) # define __glibcxx_requires_can_increment(_First,_Size) # define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2) +# define __glibcxx_requires_can_increment_range_runtime(_First1,_Last1,_First2) # define __glibcxx_requires_can_decrement_range(_First1,_Last1,_First2) # define __glibcxx_requires_sorted(_First,_Last) # define __glibcxx_requires_sorted_pred(_First,_Last,_Pred) @@ -91,11 +92,19 @@ namespace __gnu_debug # define __glibcxx_requires_cond(_Cond,_Msg) _GLIBCXX_DEBUG_VERIFY(_Cond,_Msg) # define __glibcxx_requires_valid_range(_First,_Last) \ - __glibcxx_check_valid_range(_First,_Last) + __glibcxx_check_valid_range(_First,_Last,false) # define __glibcxx_requires_can_increment(_First,_Size) \ __glibcxx_check_can_increment(_First,_Size) -# define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2) \ - __glibcxx_check_can_increment_range(_First1,_Last1,_First2) +# define __glibcxx_requires_irreflexive(_First,_Last) \ + __glibcxx_check_irreflexive(_First,_Last) +# define __glibcxx_requires_irreflexive_pred(_First,_Last,_Pred) \ + __glibcxx_check_irreflexive_pred(_First,_Last,_Pred) + +#ifdef _GLIBCXX_DEBUG +# define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2) \ + __glibcxx_check_can_increment_range(_First1,_Last1,_First2,false) +# define __glibcxx_requires_can_increment_range_runtime(_First1,_Last1,_First2) \ + __glibcxx_check_can_increment_range(_First1,_Last1,_First2,true) # define __glibcxx_requires_can_decrement_range(_First1,_Last1,_First2) \ __glibcxx_check_can_decrement_range(_First1,_Last1,_First2) # define __glibcxx_requires_sorted(_First,_Last) \ @@ -121,16 +130,46 @@ namespace __gnu_debug # define __glibcxx_requires_string(_String) __glibcxx_check_string(_String) # define __glibcxx_requires_string_len(_String,_Len) \ __glibcxx_check_string_len(_String,_Len) -# define __glibcxx_requires_irreflexive(_First,_Last) \ - __glibcxx_check_irreflexive(_First,_Last) # define __glibcxx_requires_irreflexive2(_First,_Last) \ __glibcxx_check_irreflexive2(_First,_Last) -# define __glibcxx_requires_irreflexive_pred(_First,_Last,_Pred) \ - __glibcxx_check_irreflexive_pred(_First,_Last,_Pred) # define __glibcxx_requires_irreflexive_pred2(_First,_Last,_Pred) \ __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred) # include <debug/functions.h> +#else /* _GLIBCXX_ASSERTIONS */ +# define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2) \ + __glibcxx_check_valid_range(_First1,_Last1,false) +# define __glibcxx_requires_can_increment_range_runtime(_First1,_Last1,_First2) \ + __glibcxx_check_valid_range(_First1,_Last1,true) +# define __glibcxx_requires_can_decrement_range(_First1,_Last1,_First2) \ + __glibcxx_check_valid_range(_First1,_Last1,false) +# define __glibcxx_requires_sorted(_First,_Last) \ + __glibcxx_check_valid_range(_First,_Last,false); \ + __glibcxx_check_irreflexive(_First,_Last) +# define __glibcxx_requires_sorted_pred(_First,_Last,_Pred) \ + __glibcxx_check_valid_range(_First,_Last,false); \ + __glibcxx_check_irreflexive_pred(_First,_Last,_Pred) +# define __glibcxx_requires_sorted_set(_First1,_Last1,_First2) \ + __glibcxx_check_valid_range(_First1,_Last1,false) +# define __glibcxx_requires_sorted_set_pred(_First1,_Last1,_First2,_Pred) \ + __glibcxx_check_valid_range(_First1,_Last1,false) +# define __glibcxx_requires_partitioned_lower(_First,_Last,_Value) \ + __glibcxx_check_valid_range(_First,_Last,false) +# define __glibcxx_requires_partitioned_upper(_First,_Last,_Value) \ + __glibcxx_check_valid_range(_First,_Last,false) +# define __glibcxx_requires_partitioned_lower_pred(_First,_Last,_Value,_Pred) \ + __glibcxx_check_valid_range(_First,_Last,false) +# define __glibcxx_requires_partitioned_upper_pred(_First,_Last,_Value,_Pred) \ + __glibcxx_check_valid_range(_First,_Last,false) +# define __glibcxx_requires_heap(_First,_Last) +# define __glibcxx_requires_heap_pred(_First,_Last,_Pred) +# define __glibcxx_requires_string(_String) +# define __glibcxx_requires_string_len(_String,_Len) +# define __glibcxx_requires_irreflexive2(_First,_Last) +# define __glibcxx_requires_irreflexive_pred2(_First,_Last,_Pred) + +# include <debug/helper_functions.h> +#endif /* _GLIBCXX_ASSERTIONS */ #endif diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h index c0144ced979..fd7c42ba081 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 @@ -237,8 +237,13 @@ namespace __gnu_debug _GLIBCXX20_CONSTEXPR inline bool __valid_range(_InputIterator __first, _InputIterator __last, - typename _Distance_traits<_InputIterator>::__type& __dist) + typename _Distance_traits<_InputIterator>::__type& __dist, + bool __skip_if_constexpr __attribute__ ((__unused__))) { +#ifdef __cpp_lib_is_constant_evaluated + if (std::is_constant_evaluated() && __skip_if_constexpr) + return true; +#endif typedef typename std::__is_integer<_InputIterator>::__type _Integral; return __valid_range_aux(__first, __last, __dist, _Integral()); } @@ -247,21 +252,28 @@ namespace __gnu_debug bool __valid_range(const _Safe_iterator<_Iterator, _Sequence, _Category>&, const _Safe_iterator<_Iterator, _Sequence, _Category>&, - typename _Distance_traits<_Iterator>::__type&); + typename _Distance_traits<_Iterator>::__type&, + bool __skip_if_constexpr); #if __cplusplus >= 201103L template<typename _Iterator,typename _Sequence> bool __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>&, const _Safe_local_iterator<_Iterator, _Sequence>&, - typename _Distance_traits<_Iterator>::__type&); + typename _Distance_traits<_Iterator>::__type&, + bool __skip_if_constexpr); #endif template<typename _InputIterator> _GLIBCXX14_CONSTEXPR inline bool - __valid_range(_InputIterator __first, _InputIterator __last) + __valid_range(_InputIterator __first, _InputIterator __last, + bool __skip_if_constexpr __attribute__ ((__unused__))) { +#ifdef __cpp_lib_is_constant_evaluated + if (std::is_constant_evaluated() && __skip_if_constexpr) + return true; +#endif typedef typename std::__is_integer<_InputIterator>::__type _Integral; return __valid_range_aux(__first, __last, _Integral()); } @@ -269,13 +281,15 @@ namespace __gnu_debug template<typename _Iterator, typename _Sequence, typename _Category> bool __valid_range(const _Safe_iterator<_Iterator, _Sequence, _Category>&, - const _Safe_iterator<_Iterator, _Sequence, _Category>&); + const _Safe_iterator<_Iterator, _Sequence, _Category>&, + bool __skip_if_constexpr); #if __cplusplus >= 201103L template<typename _Iterator, typename _Sequence> bool __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>&, - const _Safe_local_iterator<_Iterator, _Sequence>&); + const _Safe_local_iterator<_Iterator, _Sequence>&, + bool __skip_if_constexpr); #endif // Fallback method, always ok. diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h index 9e1288cf4d9..18abd22b717 100644 --- a/libstdc++-v3/include/debug/macros.h +++ b/libstdc++-v3/include/debug/macros.h @@ -52,13 +52,17 @@ #define _GLIBCXX_DEBUG_VERIFY_AT(_Cond,_ErrMsg,_File,_Line) \ _GLIBCXX_DEBUG_VERIFY_AT_F(_Cond,_ErrMsg,_File,_Line,__PRETTY_FUNCTION__) -#define _GLIBCXX_DEBUG_VERIFY(_Cond,_ErrMsg) \ +#ifdef _GLIBCXX_DEBUG +# define _GLIBCXX_DEBUG_VERIFY(_Cond,_ErrMsg) \ _GLIBCXX_DEBUG_VERIFY_AT_F(_Cond, _ErrMsg, __FILE__, __LINE__, \ __PRETTY_FUNCTION__) +#else /* _GLIBCXX_ASSERTIONS */ +# define _GLIBCXX_DEBUG_VERIFY(_Cond,_ErrMsg) __glibcxx_assert(_Cond) +#endif // Verify that [_First, _Last) forms a valid iterator range. -#define __glibcxx_check_valid_range(_First,_Last) \ -_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__valid_range(_First, _Last), \ +#define __glibcxx_check_valid_range(_First,_Last,_SkipIfConstexpr) \ +_GLIBCXX_DEBUG_VERIFY(__gnu_debug::__valid_range(_First, _Last,_SkipIfConstexpr), \ _M_message(__gnu_debug::__msg_valid_range) \ ._M_iterator(_First, #_First) \ ._M_iterator(_Last, #_Last)) @@ -100,12 +104,12 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_First, #_First) \ ._M_integer(_Way * _Dist.first, #_Dist)) -#define __glibcxx_check_can_increment_range(_First1,_Last1,_First2) \ +#define __glibcxx_check_can_increment_range(_First1,_Last1,_First2,_SkipIfConstexpr) \ do \ { \ typename __gnu_debug::_Distance_traits<__decltype(_First1)>::__type __dist;\ _GLIBCXX_DEBUG_VERIFY_AT_F( \ - __gnu_debug::__valid_range(_First1, _Last1, __dist),\ + __gnu_debug::__valid_range(_First1, _Last1, __dist, _SkipIfConstexpr), \ _M_message(__gnu_debug::__msg_valid_range) \ ._M_iterator(_First1, #_First1) \ ._M_iterator(_Last1, #_Last1), \ @@ -238,7 +242,7 @@ _GLIBCXX_DEBUG_VERIFY(_Position._M_attached_to(this), \ * valid iterator range within this sequence. */ #define __glibcxx_check_erase_range(_First,_Last) \ -__glibcxx_check_valid_range(_First,_Last); \ +__glibcxx_check_valid_range(_First,_Last,false); \ _GLIBCXX_DEBUG_VERIFY(_First._M_attached_to(this), \ _M_message(__gnu_debug::__msg_erase_different) \ ._M_sequence(*this, "this") \ @@ -337,7 +341,7 @@ _GLIBCXX_DEBUG_VERIFY(! this->empty(), \ // Verify that the iterator range [_First, _Last) is sorted #define __glibcxx_check_sorted(_First,_Last) \ -__glibcxx_check_valid_range(_First,_Last); \ +__glibcxx_check_valid_range(_First,_Last,false); \ __glibcxx_check_irreflexive(_First,_Last); \ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted( \ __gnu_debug::__base(_First), \ @@ -349,7 +353,7 @@ __glibcxx_check_irreflexive(_First,_Last); \ /** Verify that the iterator range [_First, _Last) is sorted by the predicate _Pred. */ #define __glibcxx_check_sorted_pred(_First,_Last,_Pred) \ -__glibcxx_check_valid_range(_First,_Last); \ +__glibcxx_check_valid_range(_First,_Last,false); \ __glibcxx_check_irreflexive_pred(_First,_Last,_Pred); \ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted( \ __gnu_debug::__base(_First), \ @@ -361,7 +365,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_sorted( \ // Special variant for std::merge, std::includes, std::set_* #define __glibcxx_check_sorted_set(_First1,_Last1,_First2) \ -__glibcxx_check_valid_range(_First1,_Last1); \ +__glibcxx_check_valid_range(_First1,_Last1,false); \ _GLIBCXX_DEBUG_VERIFY( \ __gnu_debug::__check_sorted_set(__gnu_debug::__base(_First1), \ __gnu_debug::__base(_Last1), _First2),\ @@ -371,7 +375,7 @@ _GLIBCXX_DEBUG_VERIFY( \ // Likewise with a _Pred. #define __glibcxx_check_sorted_set_pred(_First1,_Last1,_First2,_Pred) \ -__glibcxx_check_valid_range(_First1,_Last1); \ +__glibcxx_check_valid_range(_First1,_Last1,false); \ _GLIBCXX_DEBUG_VERIFY( \ __gnu_debug::__check_sorted_set(__gnu_debug::__base(_First1), \ __gnu_debug::__base(_Last1), \ @@ -384,7 +388,7 @@ _GLIBCXX_DEBUG_VERIFY( \ /** Verify that the iterator range [_First, _Last) is partitioned w.r.t. the value _Value. */ #define __glibcxx_check_partitioned_lower(_First,_Last,_Value) \ -__glibcxx_check_valid_range(_First,_Last); \ +__glibcxx_check_valid_range(_First,_Last,false); \ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower( \ __gnu_debug::__base(_First), \ __gnu_debug::__base(_Last), _Value), \ @@ -394,7 +398,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower( \ ._M_string(#_Value)) #define __glibcxx_check_partitioned_upper(_First,_Last,_Value) \ -__glibcxx_check_valid_range(_First,_Last); \ +__glibcxx_check_valid_range(_First,_Last,false); \ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper( \ __gnu_debug::__base(_First), \ __gnu_debug::__base(_Last), _Value), \ @@ -406,7 +410,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper( \ /** Verify that the iterator range [_First, _Last) is partitioned w.r.t. the value _Value and predicate _Pred. */ #define __glibcxx_check_partitioned_lower_pred(_First,_Last,_Value,_Pred) \ -__glibcxx_check_valid_range(_First,_Last); \ +__glibcxx_check_valid_range(_First,_Last,false); \ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower( \ __gnu_debug::__base(_First), \ __gnu_debug::__base(_Last), _Value, _Pred), \ @@ -419,7 +423,7 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_lower( \ /** Verify that the iterator range [_First, _Last) is partitioned w.r.t. the value _Value and predicate _Pred. */ #define __glibcxx_check_partitioned_upper_pred(_First,_Last,_Value,_Pred) \ -__glibcxx_check_valid_range(_First,_Last); \ +__glibcxx_check_valid_range(_First,_Last,false); \ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__check_partitioned_upper( \ __gnu_debug::__base(_First), \ __gnu_debug::__base(_Last), _Value, _Pred), \ diff --git a/libstdc++-v3/testsuite/24_iterators/istream_iterator/1.cc b/libstdc++-v3/testsuite/24_iterators/istream_iterator/1.cc index 93085aa1471..ecf87a58797 100644 --- a/libstdc++-v3/testsuite/24_iterators/istream_iterator/1.cc +++ b/libstdc++-v3/testsuite/24_iterators/istream_iterator/1.cc @@ -48,7 +48,7 @@ void test01() ss.str("-1 -2 -3"); VERIFY( iter == end ); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS // This is undefined, so aborts under debug mode. // Without debug mode, it should not extract anything from the stream, // and the iterator should remain at end-of-stream. diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc new file mode 100644 index 00000000000..8840f31a1d9 --- /dev/null +++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc @@ -0,0 +1,52 @@ +// Copyright (C) 2021 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++2a -D_GLIBCXX_ASSERTIONS" } +// { dg-do compile { target c++2a xfail *-*-* } } + +#include <algorithm> +#include <array> + +constexpr bool +test1() +{ + constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + + const auto out6 = std::copy(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2); + + return out6 == ma0.begin() + 10; +} + +static_assert(test1()); // { dg-error "non-constant condition" } + +constexpr bool +test2() +{ + constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + + const auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, ma0.begin() + 10); + + return out6 == ma0.begin() + 18; +} + +static_assert(test2()); // { dg-error "is outside the bounds" } + +// { dg-prune-output "in 'constexpr' expansion" } +// { dg-prune-output "builtin_unreachable" } +// { dg-prune-output "non-constant condition" } diff --git a/libstdc++-v3/testsuite/25_algorithms/heap/1.cc b/libstdc++-v3/testsuite/25_algorithms/heap/1.cc index 69793de429f..9be4337729e 100644 --- a/libstdc++-v3/testsuite/25_algorithms/heap/1.cc +++ b/libstdc++-v3/testsuite/25_algorithms/heap/1.cc @@ -83,7 +83,7 @@ test02() { Gt gt; -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS //const int logN = static_cast<int>(std::log(static_cast<double>(N)) + 0.5); const int logN = 3; #endif @@ -95,7 +95,7 @@ test02() for (int i = 2; i <= N; ++i) { std::push_heap(s1, s1 + i, gt); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS VERIFY(gt.count() <= logN); #endif gt.reset(); @@ -104,7 +104,7 @@ test02() for (int i = N; i >= 2; --i) { std::pop_heap(s1, s1 + i, gt); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS VERIFY(gt.count() <= 2 * logN); #endif gt.reset(); @@ -118,13 +118,13 @@ test02() VERIFY(std::equal(s2, s2 + N, A)); std::make_heap(s2, s2 + N, gt); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS VERIFY(gt.count() <= 3 * N); #endif gt.reset(); std::sort_heap(s2, s2 + N, gt); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS VERIFY(gt.count() <= N * logN); #endif diff --git a/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_neg.cc b/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_neg.cc index 3536e64c00c..56d80d61ae2 100644 --- a/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_neg.cc +++ b/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_neg.cc @@ -44,5 +44,5 @@ test() static_assert(test()); // { dg-error "" } -// { dg-prune-output "failed_assertion" } +// { dg-prune-output "builtin_unreachable" } // { dg-prune-output "in 'constexpr'" } diff --git a/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_pred_neg.cc b/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_pred_neg.cc index ecda3f5c5da..ab931bbc731 100644 --- a/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_pred_neg.cc +++ b/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_partitioned_pred_neg.cc @@ -34,5 +34,5 @@ test() static_assert(test()); // { dg-error "" } -// { dg-prune-output "failed_assertion" } +// { dg-prune-output "builtin_unreachable" } // { dg-prune-output "in 'constexpr'" } diff --git a/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_valid_range_neg.cc b/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_valid_range_neg.cc index e7195438552..fee67e52172 100644 --- a/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_valid_range_neg.cc +++ b/libstdc++-v3/testsuite/25_algorithms/lower_bound/debug/constexpr_valid_range_neg.cc @@ -47,6 +47,6 @@ test2() static_assert(test2()); // { dg-error "" } -// { dg-prune-output "failed_assertion" } +// { dg-prune-output "builtin_unreachable" } // { dg-prune-output "in 'constexpr'" } diff --git a/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_neg.cc b/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_neg.cc index 04d6f73a9ac..7937ac8d308 100644 --- a/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_neg.cc +++ b/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_neg.cc @@ -44,5 +44,5 @@ test() static_assert(test()); // { dg-error "" } -// { dg-prune-output "failed_assertion" } +// { dg-prune-output "builtin_unreachable" } // { dg-prune-output "in 'constexpr'" } diff --git a/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_pred_neg.cc b/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_pred_neg.cc index 7d2ef738c83..7d33eb535fa 100644 --- a/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_pred_neg.cc +++ b/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_partitioned_pred_neg.cc @@ -34,5 +34,5 @@ test() static_assert(test()); // { dg-error "" } -// { dg-prune-output "failed_assertion" } +// { dg-prune-output "builtin_unreachable" } // { dg-prune-output "in 'constexpr'" } diff --git a/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_valid_range_neg.cc b/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_valid_range_neg.cc index 33a09b73bca..9c2d0657208 100644 --- a/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_valid_range_neg.cc +++ b/libstdc++-v3/testsuite/25_algorithms/upper_bound/debug/constexpr_valid_range_neg.cc @@ -47,5 +47,5 @@ test2() static_assert(test2()); // { dg-error "" } -// { dg-prune-output "failed_assertion" } +// { dg-prune-output "builtin_unreachable" } // { dg-prune-output "in 'constexpr'" } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light 2021-08-06 14:52 ` François Dumont @ 2021-08-08 19:34 ` François Dumont 2021-08-23 5:01 ` François Dumont 0 siblings, 1 reply; 7+ messages in thread From: François Dumont @ 2021-08-08 19:34 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++, gcc-patches [-- Attachment #1: Type: text/plain, Size: 6554 bytes --] 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 > [-- Attachment #2: assertion_mode.patch --] [-- Type: text/x-patch, Size: 15915 bytes --] diff --git a/libstdc++-v3/include/bits/stl_algobase.h b/libstdc++-v3/include/bits/stl_algobase.h index d0c49628d7f..d45eec4339b 100644 --- a/libstdc++-v3/include/bits/stl_algobase.h +++ b/libstdc++-v3/include/bits/stl_algobase.h @@ -1551,7 +1551,7 @@ _GLIBCXX_BEGIN_NAMESPACE_ALGO __glibcxx_function_requires(_EqualOpConcept< typename iterator_traits<_II1>::value_type, typename iterator_traits<_II2>::value_type>) - __glibcxx_requires_can_increment_range(__first1, __last1, __first2); + __glibcxx_requires_can_increment_range_runtime(__first1, __last1, __first2); return std::__equal_aux(__first1, __last1, __first2); } diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h index c5b02408c1c..a8986d11dfe 100644 --- a/libstdc++-v3/include/bits/stl_iterator.h +++ b/libstdc++-v3/include/bits/stl_iterator.h @@ -2463,7 +2463,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION _GLIBCXX_END_NAMESPACE_VERSION } // namespace -#ifdef _GLIBCXX_DEBUG +#ifdef _GLIBCXX_ASSERTIONS # include <debug/stl_iterator.h> #endif diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h index 116f2f023e2..4ff027eaa74 100644 --- a/libstdc++-v3/include/debug/debug.h +++ b/libstdc++-v3/include/debug/debug.h @@ -61,12 +61,13 @@ 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) # define __glibcxx_requires_can_increment(_First,_Size) # define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2) +# define __glibcxx_requires_can_increment_range_runtime(_First1,_Last1,_First2) # define __glibcxx_requires_can_decrement_range(_First1,_Last1,_First2) # define __glibcxx_requires_sorted(_First,_Last) # define __glibcxx_requires_sorted_pred(_First,_Last,_Pred) @@ -87,6 +88,7 @@ namespace __gnu_debug #else +# ifdef _GLIBCXX_DEBUG # include <debug/macros.h> # define __glibcxx_requires_cond(_Cond,_Msg) _GLIBCXX_DEBUG_VERIFY(_Cond,_Msg) @@ -95,7 +97,9 @@ namespace __gnu_debug # define __glibcxx_requires_can_increment(_First,_Size) \ __glibcxx_check_can_increment(_First,_Size) # define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2) \ - __glibcxx_check_can_increment_range(_First1,_Last1,_First2) + __glibcxx_check_can_increment_range(_First1,_Last1,_First2,false) +# define __glibcxx_requires_can_increment_range_runtime(_First1,_Last1,_First2) \ + __glibcxx_check_can_increment_range(_First1,_Last1,_First2,true) # define __glibcxx_requires_can_decrement_range(_First1,_Last1,_First2) \ __glibcxx_check_can_decrement_range(_First1,_Last1,_First2) # define __glibcxx_requires_sorted(_First,_Last) \ @@ -131,7 +135,50 @@ namespace __gnu_debug __glibcxx_check_irreflexive_pred2(_First,_Last,_Pred) # include <debug/functions.h> +# else +# define __glibcxx_requires_cond(_Cond,_Msg) __glibcxx_assert(_Cond) +# define __glibcxx_requires_valid_range(_First,_Last) \ + __glibcxx_assert(__gnu_debug::__valid_range(_First,_Last)) +# define __glibcxx_requires_can_increment(_First,_Size) \ + __glibcxx_assert(__gnu_debug::__can_advance(_First,_Size)) +# define __glibcxx_requires_can_increment_range(_First1,_Last1,_First2) \ + __glibcxx_requires_valid_range(_First1,_Last1) +# define __glibcxx_requires_can_increment_range_runtime(_First1,_Last1,_First2) \ + __glibcxx_assert(__gnu_debug::__valid_range(_First1,_Last1,true)) +# define __glibcxx_requires_can_decrement_range(_First1,_Last1,_First2) \ + __glibcxx_requires_valid_range(_First1,_Last1) +# define __glibcxx_requires_sorted(_First,_Last) \ + __glibcxx_requires_valid_range(_First,_Last); \ + __glibcxx_requires_irreflexive(_First,_Last) +# define __glibcxx_requires_sorted_pred(_First,_Last,_Pred) \ + __glibcxx_requires_valid_range(_First,_Last); \ + __glibcxx_requires_irreflexive_pred(_First,_Last,_Pred) +# define __glibcxx_requires_sorted_set(_First1,_Last1,_First2) \ + __glibcxx_requires_valid_range(_First1,_Last1) +# define __glibcxx_requires_sorted_set_pred(_First1,_Last1,_First2,_Pred) \ + __glibcxx_requires_valid_range(_First1,_Last1) +# define __glibcxx_requires_partitioned_lower(_First,_Last,_Value) \ + __glibcxx_requires_valid_range(_First,_Last) +# define __glibcxx_requires_partitioned_upper(_First,_Last,_Value) \ + __glibcxx_requires_valid_range(_First,_Last) +# define __glibcxx_requires_partitioned_lower_pred(_First,_Last,_Value,_Pred) \ + __glibcxx_requires_valid_range(_First,_Last) +# define __glibcxx_requires_partitioned_upper_pred(_First,_Last,_Value,_Pred) \ + __glibcxx_requires_valid_range(_First,_Last) +# define __glibcxx_requires_heap(_First,_Last) +# define __glibcxx_requires_heap_pred(_First,_Last,_Pred) +# define __glibcxx_requires_string(_String) +# define __glibcxx_requires_string_len(_String,_Len) +# define __glibcxx_requires_irreflexive(_First,_Last) \ + __glibcxx_assert(_First == _Last || !(*_First < *_First)) +# define __glibcxx_requires_irreflexive2(_First,_Last) +# define __glibcxx_requires_irreflexive_pred(_First,_Last,_Pred) \ + __glibcxx_assert(_First == _Last || !_Pred(*_First < *_First)) +# define __glibcxx_requires_irreflexive_pred2(_First,_Last,_Pred) + +# include <debug/helper_functions.h> +# endif /* _GLIBCXX_DEBUG */ -#endif +#endif /* _GLIBCXX_ASSERTIONS */ #endif // _GLIBCXX_DEBUG_MACRO_SWITCH_H diff --git a/libstdc++-v3/include/debug/helper_functions.h b/libstdc++-v3/include/debug/helper_functions.h index c0144ced979..e4e582aefb9 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 @@ -237,8 +237,13 @@ namespace __gnu_debug _GLIBCXX20_CONSTEXPR inline bool __valid_range(_InputIterator __first, _InputIterator __last, - typename _Distance_traits<_InputIterator>::__type& __dist) + typename _Distance_traits<_InputIterator>::__type& __dist, + bool __skip_if_constexpr __attribute__ ((__unused__)) = false) { +#ifdef __cpp_lib_is_constant_evaluated + if (std::is_constant_evaluated() && __skip_if_constexpr) + return true; +#endif typedef typename std::__is_integer<_InputIterator>::__type _Integral; return __valid_range_aux(__first, __last, __dist, _Integral()); } @@ -247,21 +252,28 @@ namespace __gnu_debug bool __valid_range(const _Safe_iterator<_Iterator, _Sequence, _Category>&, const _Safe_iterator<_Iterator, _Sequence, _Category>&, - typename _Distance_traits<_Iterator>::__type&); + typename _Distance_traits<_Iterator>::__type&, + bool __skip_if_constexpr = false); #if __cplusplus >= 201103L template<typename _Iterator,typename _Sequence> bool __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>&, const _Safe_local_iterator<_Iterator, _Sequence>&, - typename _Distance_traits<_Iterator>::__type&); + typename _Distance_traits<_Iterator>::__type&, + bool __skip_if_constexpr = false); #endif template<typename _InputIterator> _GLIBCXX14_CONSTEXPR inline bool - __valid_range(_InputIterator __first, _InputIterator __last) + __valid_range(_InputIterator __first, _InputIterator __last, + bool __skip_if_constexpr __attribute__ ((__unused__)) = false) { +#ifdef __cpp_lib_is_constant_evaluated + if (std::is_constant_evaluated() && __skip_if_constexpr) + return true; +#endif typedef typename std::__is_integer<_InputIterator>::__type _Integral; return __valid_range_aux(__first, __last, _Integral()); } @@ -269,13 +281,15 @@ namespace __gnu_debug template<typename _Iterator, typename _Sequence, typename _Category> bool __valid_range(const _Safe_iterator<_Iterator, _Sequence, _Category>&, - const _Safe_iterator<_Iterator, _Sequence, _Category>&); + const _Safe_iterator<_Iterator, _Sequence, _Category>&, + bool __skip_if_constexpr = false); #if __cplusplus >= 201103L template<typename _Iterator, typename _Sequence> bool __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>&, - const _Safe_local_iterator<_Iterator, _Sequence>&); + const _Safe_local_iterator<_Iterator, _Sequence>&, + bool __skip_if_constexpr = false); #endif // Fallback method, always ok. diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h index 9e1288cf4d9..5332ad09d22 100644 --- a/libstdc++-v3/include/debug/macros.h +++ b/libstdc++-v3/include/debug/macros.h @@ -100,12 +100,12 @@ _GLIBCXX_DEBUG_VERIFY(__gnu_debug::__can_advance(_First, _Size), \ ._M_iterator(_First, #_First) \ ._M_integer(_Way * _Dist.first, #_Dist)) -#define __glibcxx_check_can_increment_range(_First1,_Last1,_First2) \ +#define __glibcxx_check_can_increment_range(_First1,_Last1,_First2,_SkipIfConstexpr) \ do \ { \ typename __gnu_debug::_Distance_traits<__decltype(_First1)>::__type __dist;\ _GLIBCXX_DEBUG_VERIFY_AT_F( \ - __gnu_debug::__valid_range(_First1, _Last1, __dist),\ + __gnu_debug::__valid_range(_First1, _Last1, __dist, _SkipIfConstexpr), \ _M_message(__gnu_debug::__msg_valid_range) \ ._M_iterator(_First1, #_First1) \ ._M_iterator(_Last1, #_Last1), \ diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h index 5584d06de5a..003557f2e59 100644 --- a/libstdc++-v3/include/debug/safe_iterator.h +++ b/libstdc++-v3/include/debug/safe_iterator.h @@ -965,7 +965,8 @@ namespace __gnu_debug _Category>& __first, const _Safe_iterator<_Iterator, _Sequence, _Category>& __last, - typename _Distance_traits<_Iterator>::__type& __dist) + typename _Distance_traits<_Iterator>::__type& __dist, + bool /* __skip_if_constexpr */) { return __first._M_valid_range(__last, __dist); } template<typename _Iterator, typename _Sequence, typename _Category> @@ -973,7 +974,8 @@ namespace __gnu_debug __valid_range(const _Safe_iterator<_Iterator, _Sequence, _Category>& __first, const _Safe_iterator<_Iterator, _Sequence, - _Category>& __last) + _Category>& __last, + bool /* __skip_if_constexpr */) { typename _Distance_traits<_Iterator>::__type __dist; return __first._M_valid_range(__last, __dist); diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h index 31c48e65a24..732965860c0 100644 --- a/libstdc++-v3/include/debug/safe_local_iterator.h +++ b/libstdc++-v3/include/debug/safe_local_iterator.h @@ -408,13 +408,15 @@ namespace __gnu_debug inline bool __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>& __first, const _Safe_local_iterator<_Iterator, _Sequence>& __last, - typename _Distance_traits<_Iterator>::__type& __dist_info) + typename _Distance_traits<_Iterator>::__type& __dist_info, + bool /* __skip_if_constexpr */) { return __first._M_valid_range(__last, __dist_info); } template<typename _Iterator, typename _Sequence> inline bool __valid_range(const _Safe_local_iterator<_Iterator, _Sequence>& __first, - const _Safe_local_iterator<_Iterator, _Sequence>& __last) + const _Safe_local_iterator<_Iterator, _Sequence>& __last, + bool /* __skip_if_constexpr */) { typename _Distance_traits<_Iterator>::__type __dist_info; return __first._M_valid_range(__last, __dist_info); diff --git a/libstdc++-v3/testsuite/24_iterators/istream_iterator/1.cc b/libstdc++-v3/testsuite/24_iterators/istream_iterator/1.cc index 93085aa1471..ecf87a58797 100644 --- a/libstdc++-v3/testsuite/24_iterators/istream_iterator/1.cc +++ b/libstdc++-v3/testsuite/24_iterators/istream_iterator/1.cc @@ -48,7 +48,7 @@ void test01() ss.str("-1 -2 -3"); VERIFY( iter == end ); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS // This is undefined, so aborts under debug mode. // Without debug mode, it should not extract anything from the stream, // and the iterator should remain at end-of-stream. diff --git a/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc new file mode 100644 index 00000000000..8840f31a1d9 --- /dev/null +++ b/libstdc++-v3/testsuite/25_algorithms/copy/constexpr_neg.cc @@ -0,0 +1,52 @@ +// Copyright (C) 2021 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++2a -D_GLIBCXX_ASSERTIONS" } +// { dg-do compile { target c++2a xfail *-*-* } } + +#include <algorithm> +#include <array> + +constexpr bool +test1() +{ + constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + + const auto out6 = std::copy(ca0.begin() + 8, ca0.begin(), ma0.begin() + 2); + + return out6 == ma0.begin() + 10; +} + +static_assert(test1()); // { dg-error "non-constant condition" } + +constexpr bool +test2() +{ + constexpr std::array<int, 12> ca0{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}; + std::array<int, 12> ma0{{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + + const auto out6 = std::copy(ca0.begin(), ca0.begin() + 8, ma0.begin() + 10); + + return out6 == ma0.begin() + 18; +} + +static_assert(test2()); // { dg-error "is outside the bounds" } + +// { dg-prune-output "in 'constexpr' expansion" } +// { dg-prune-output "builtin_unreachable" } +// { dg-prune-output "non-constant condition" } diff --git a/libstdc++-v3/testsuite/25_algorithms/heap/1.cc b/libstdc++-v3/testsuite/25_algorithms/heap/1.cc index 69793de429f..9be4337729e 100644 --- a/libstdc++-v3/testsuite/25_algorithms/heap/1.cc +++ b/libstdc++-v3/testsuite/25_algorithms/heap/1.cc @@ -83,7 +83,7 @@ test02() { Gt gt; -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS //const int logN = static_cast<int>(std::log(static_cast<double>(N)) + 0.5); const int logN = 3; #endif @@ -95,7 +95,7 @@ test02() for (int i = 2; i <= N; ++i) { std::push_heap(s1, s1 + i, gt); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS VERIFY(gt.count() <= logN); #endif gt.reset(); @@ -104,7 +104,7 @@ test02() for (int i = N; i >= 2; --i) { std::pop_heap(s1, s1 + i, gt); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS VERIFY(gt.count() <= 2 * logN); #endif gt.reset(); @@ -118,13 +118,13 @@ test02() VERIFY(std::equal(s2, s2 + N, A)); std::make_heap(s2, s2 + N, gt); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS VERIFY(gt.count() <= 3 * N); #endif gt.reset(); std::sort_heap(s2, s2 + N, gt); -#ifndef _GLIBCXX_DEBUG +#ifndef _GLIBCXX_ASSERTIONS VERIFY(gt.count() <= N * logN); #endif ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light 2021-08-08 19:34 ` François Dumont @ 2021-08-23 5:01 ` François Dumont 0 siblings, 0 replies; 7+ messages in thread From: François Dumont @ 2021-08-23 5:01 UTC (permalink / raw) To: Jonathan Wakely; +Cc: libstdc++, gcc-patches 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 >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-23 5:01 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-27 17:37 [PATCH] Use _GLIBCXX_ASSERTIONS as _GLIBCXX_DEBUG light François Dumont 2021-05-31 17:17 ` François Dumont 2021-06-03 12:31 ` Jonathan Wakely 2021-06-07 4:25 ` François Dumont 2021-08-06 14:52 ` François Dumont 2021-08-08 19:34 ` François Dumont 2021-08-23 5:01 ` François Dumont
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).