On 26/11/19 20:07 +0100, François Dumont wrote:
>On 11/26/19 1:21 PM, Jonathan Wakely wrote:
>>On 26/11/19 12:33 +0100, Stephan Bergmann wrote:
>>>On 22/11/2019 18:59, Jonathan Wakely wrote:
>>>>On 22/11/19 18:38 +0100, François Dumont wrote:
>>>>> I noticed that we are not checking that iterators are not
>>>>>singular in valid_range. Moreover __check_singular signature
>>>>>for pointers is not intercepting all kind of pointers in terms
>>>>>of qualification.
>>>>>
>>>>> I'd like to commit it next week but considering we are in
>>>>>stage 3 I need proper acceptance.
>>>>>
>>>>> * include/debug/functions.h: Remove <bits/move.h> include.
>>>>> (__check_singular_aux, __check_singular): Move...
>>>>> * include/debug/helper_functions.h:
>>>>> (__check_singular_aux, __check_singular): ...here.
>>>>> (__valid_range_aux): Adapt to use latter.
>>>>> * testsuite/25_algorithms/copy/debug/2_neg.cc: New.
>>>>>
>>>>>Tested under Linux x86_64 normal and debug modes.
>>>>
>>>>OK for trunk, thanks.
>>>
>>>The curly braces...
>>>
>>>>diff --git a/libstdc++-v3/include/debug/helper_functions.h
>>>>b/libstdc++-v3/include/debug/helper_functions.h
>>>>index c3e7478f649..5a858754875 100644
>>>>--- a/libstdc++-v3/include/debug/helper_functions.h
>>>>+++ b/libstdc++-v3/include/debug/helper_functions.h
>>>[...]
>>>>@@ -138,14 +156,23 @@ namespace __gnu_debug
>>>> inline bool
>>>> __valid_range_aux(_InputIterator __first, _InputIterator __last,
>>>> std::input_iterator_tag)
>>>>- { return true; }
>>>>+ {
>>>>+ if (__first != __last)
>>>>+ return !__check_singular(__first) && !__check_singular(__last);
>>>>+
>>>>+ return true;
>>>>+ }
>>>> template<typename _InputIterator>
>>>> _GLIBCXX_CONSTEXPR
>>>> inline bool
>>>> __valid_range_aux(_InputIterator __first, _InputIterator __last,
>>>> std::random_access_iterator_tag)
>>>>- { return __first <= __last; }
>>>>+ {
>>>>+ return
>>>>+ __valid_range_aux(__first, __last, std::input_iterator_tag{})
>>>
>>>...^^^ here...
>>>
>>>>+ && __first <= __last;
>>>>+ }
>>>> /** We have iterators, so figure out what kind of iterators they are
>>>> * to see if we can check the range ahead of time.
>>>>@@ -167,6 +194,9 @@ namespace __gnu_debug
>>>> typename _Distance_traits<_InputIterator>::__type&
>>>>__dist,
>>>> std::__false_type)
>>>> {
>>>>+ if (!__valid_range_aux(__first, __last,
>>>>std::input_iterator_tag{}))
>>>
>>>...and ^^^ here are not allowed pre C++11. Replacing those with
>>>
>>> std::input_iterator_tag()
>>>
>>>should fix it.
>>
>>Indeed. We should also have tests that use "-std=gnu++98
>>-D_GLIBCXX_DEBUG" so they'd catch this.
>>
>>François, can you take care of the fix please?
>>
>>
>>
>Sure, I am about to do so.
>
>However I wasn't sure about this syntax before the commit so I had run
>the new 2_neg.cc with:
>
>make CXXFLAGS=-std=c++98 check-debug
>
>and it worked fine and still is !
The testsuite doesn't use CXXFLAGS.
>I also try -std=gnu++98 and made sure that pch had been updated by
>re-building libstdc++ first. I just can't get the expected compilation
>error.
>
>Maybe I need to rebuild the whole stuff to get an error...
No, you need to pass the right flags so they are used by the
testsuite. This will do it:
make -C testsuite/ check-debug debug_flags=unix/-D_GLIBCXX_DEBUG/-std=gnu++98/-Wsystem-headers
But since it only shows up with -Wsystem-headers, there's no point
trying to test for it.