From e1cd11d80f5929e916de861f83d672ab72e29046 Mon Sep 17 00:00:00 2001 From: Marshall Clow Date: Fri, 22 Mar 2019 22:32:20 +0000 Subject: [PATCH] Fix a minor bug with std::next and prev not and negative numbers. In particular, std::prev cannot require Bidirectional Iterators, because you might 'go back' -1 places, which goes forward. Thanks to Ville and Jonathan for the bug report. llvm-svn: 356818 --- libcxx/include/iterator | 17 ++++++--- .../test/libcxx/iterators/advance.debug1.pass.cpp | 42 ++++++++++++++++++++++ libcxx/test/libcxx/iterators/next.debug1.pass.cpp | 38 ++++++++++++++++++++ libcxx/test/libcxx/iterators/prev.debug1.pass.cpp | 42 ++++++++++++++++++++++ .../iterator.operations/next.pass.cpp | 20 ++++++----- .../iterator.operations/prev.pass.cpp | 16 ++++++--- 6 files changed, 157 insertions(+), 18 deletions(-) create mode 100644 libcxx/test/libcxx/iterators/advance.debug1.pass.cpp create mode 100644 libcxx/test/libcxx/iterators/next.debug1.pass.cpp create mode 100644 libcxx/test/libcxx/iterators/prev.debug1.pass.cpp diff --git a/libcxx/include/iterator b/libcxx/include/iterator index 16c1bcb..5846c1b4 100644 --- a/libcxx/include/iterator +++ b/libcxx/include/iterator @@ -584,6 +584,8 @@ inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14 void advance(_InputIter& __i, typename iterator_traits<_InputIter>::difference_type __n) { + _LIBCPP_ASSERT(__n >= 0 || __is_bidirectional_iterator<_InputIter>::value, + "Attempt to advance(it, -n) on a non-bidi iterator"); __advance(__i, __n, typename iterator_traits<_InputIter>::iterator_category()); } @@ -624,20 +626,25 @@ typename enable_if next(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n = 1) { + _LIBCPP_ASSERT(__n >= 0 || __is_bidirectional_iterator<_InputIter>::value, + "Attempt to next(it, -n) on a non-bidi iterator"); + _VSTD::advance(__x, __n); return __x; } -template +template inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14 typename enable_if < - __is_bidirectional_iterator<_BidirectionalIter>::value, - _BidirectionalIter + __is_input_iterator<_InputIter>::value, + _InputIter >::type -prev(_BidirectionalIter __x, - typename iterator_traits<_BidirectionalIter>::difference_type __n = 1) +prev(_InputIter __x, + typename iterator_traits<_InputIter>::difference_type __n = 1) { + _LIBCPP_ASSERT(__n <= 0 || __is_bidirectional_iterator<_InputIter>::value, + "Attempt to prev(it, +n) on a non-bidi iterator"); _VSTD::advance(__x, -__n); return __x; } diff --git a/libcxx/test/libcxx/iterators/advance.debug1.pass.cpp b/libcxx/test/libcxx/iterators/advance.debug1.pass.cpp new file mode 100644 index 0000000..37aa3df --- /dev/null +++ b/libcxx/test/libcxx/iterators/advance.debug1.pass.cpp @@ -0,0 +1,42 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// Can't test the system lib because this test enables debug mode +// MODULES_DEFINES: _LIBCPP_DEBUG=1 +// UNSUPPORTED: c++98, c++03 +// UNSUPPORTED: windows +// UNSUPPORTED: with_system_cxx_lib + +// + +// Call advance(non-bidi iterator, -1) + +#define _LIBCPP_DEBUG 0 + +#include +#include "debug_mode_helper.h" + +#include "test_iterators.h" + +int main(int, char**) +{ + int a[] = {1, 2, 3}; + + bidirectional_iterator bidi(a+1); + std::advance(bidi, 1); // should work fine + std::advance(bidi, 0); // should work fine + std::advance(bidi, -1); // should work fine + + forward_iterator it(a+1); + std::advance(it, 1); // should work fine + std::advance(it, 0); // should work fine + EXPECT_DEATH( std::advance(it, -1) ); // can't go backwards on a FwdIter + + return 0; +} + diff --git a/libcxx/test/libcxx/iterators/next.debug1.pass.cpp b/libcxx/test/libcxx/iterators/next.debug1.pass.cpp new file mode 100644 index 0000000..72d9fd4 --- /dev/null +++ b/libcxx/test/libcxx/iterators/next.debug1.pass.cpp @@ -0,0 +1,38 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// Can't test the system lib because this test enables debug mode +// MODULES_DEFINES: _LIBCPP_DEBUG=1 +// UNSUPPORTED: c++98, c++03 +// UNSUPPORTED: windows +// UNSUPPORTED: with_system_cxx_lib + +// + +// Call next(non-bidi iterator, -1) + +#define _LIBCPP_DEBUG 0 + +#include +#include "debug_mode_helper.h" + +#include "test_iterators.h" + +int main(int, char**) +{ + int a[] = {1, 2, 3}; + + + forward_iterator it(a+1); + std::next(it, 1); // should work fine + std::next(it, 0); // should work fine + EXPECT_DEATH( std::next(it, -1) ); // can't go backwards on a FwdIter + + return 0; +} + diff --git a/libcxx/test/libcxx/iterators/prev.debug1.pass.cpp b/libcxx/test/libcxx/iterators/prev.debug1.pass.cpp new file mode 100644 index 0000000..da7c931 --- /dev/null +++ b/libcxx/test/libcxx/iterators/prev.debug1.pass.cpp @@ -0,0 +1,42 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// Can't test the system lib because this test enables debug mode +// MODULES_DEFINES: _LIBCPP_DEBUG=1 +// UNSUPPORTED: with_system_cxx_lib +// UNSUPPORTED: c++98, c++03 +// UNSUPPORTED: windows +// UNSUPPORTED: with_system_cxx_lib + +// + +// Call prev(forward_iterator, -1) + +#define _LIBCPP_DEBUG 0 + +#include +#include "debug_mode_helper.h" + +#include "test_iterators.h" + +int main(int, char**) +{ + int a[] = {1, 2, 3}; + + bidirectional_iterator bidi(a+1); + std::prev(bidi, -1); // should work fine + std::prev(bidi, 0); // should work fine + std::prev(bidi, 1); // should work fine + + forward_iterator it(a+1); + std::prev(it, -1); // should work fine + std::prev(it, 0); // should work fine + EXPECT_DEATH( std::prev(it, 1) ); // can't go backwards on a FwdIter + + return 0; +} diff --git a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp index 87f79fb..26ec32c 100644 --- a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp +++ b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/next.pass.cpp @@ -55,10 +55,12 @@ int main(int, char**) { { const char* s = "1234567890"; - test(input_iterator(s), 10, input_iterator(s+10)); - test(forward_iterator(s), 10, forward_iterator(s+10)); - test(bidirectional_iterator(s), 10, bidirectional_iterator(s+10)); - test(random_access_iterator(s), 10, random_access_iterator(s+10)); + test(input_iterator(s), 10, input_iterator(s+10)); + test(forward_iterator(s), 10, forward_iterator(s+10)); + test(bidirectional_iterator(s), 10, bidirectional_iterator(s+10)); + test(bidirectional_iterator(s+10), -10, bidirectional_iterator(s)); + test(random_access_iterator(s), 10, random_access_iterator(s+10)); + test(random_access_iterator(s+10), -10, random_access_iterator(s)); test(s, 10, s+10); test(input_iterator(s), input_iterator(s+1)); @@ -70,10 +72,12 @@ int main(int, char**) #if TEST_STD_VER > 14 { constexpr const char* s = "1234567890"; - static_assert( constexpr_test(input_iterator(s), 10, input_iterator(s+10)), "" ); - static_assert( constexpr_test(forward_iterator(s), 10, forward_iterator(s+10)), "" ); - static_assert( constexpr_test(bidirectional_iterator(s), 10, bidirectional_iterator(s+10)), "" ); - static_assert( constexpr_test(random_access_iterator(s), 10, random_access_iterator(s+10)), "" ); + static_assert( constexpr_test(input_iterator(s), 10, input_iterator(s+10)), "" ); + static_assert( constexpr_test(forward_iterator(s), 10, forward_iterator(s+10)), "" ); + static_assert( constexpr_test(bidirectional_iterator(s), 10, bidirectional_iterator(s+10)), "" ); + static_assert( constexpr_test(bidirectional_iterator(s+10), -10, bidirectional_iterator(s)), "" ); + static_assert( constexpr_test(random_access_iterator(s), 10, random_access_iterator(s+10)), "" ); + static_assert( constexpr_test(random_access_iterator(s+10), -10, random_access_iterator(s)), "" ); static_assert( constexpr_test(s, 10, s+10), "" ); static_assert( constexpr_test(input_iterator(s), input_iterator(s+1)), "" ); diff --git a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp index 2ee0444..8faaf3d 100644 --- a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp +++ b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp @@ -8,7 +8,7 @@ // -// template +// template // Iter prev(Iter x, Iter::difference_type n = 1); #include @@ -53,8 +53,11 @@ int main(int, char**) { { const char* s = "1234567890"; - test(bidirectional_iterator(s+10), 10, bidirectional_iterator(s)); - test(random_access_iterator(s+10), 10, random_access_iterator(s)); + test(forward_iterator (s), -10, forward_iterator (s+10)); + test(bidirectional_iterator(s+10), 10, bidirectional_iterator(s)); + test(bidirectional_iterator(s), -10, bidirectional_iterator(s+10)); + test(random_access_iterator(s+10), 10, random_access_iterator(s)); + test(random_access_iterator(s), -10, random_access_iterator(s+10)); test(s+10, 10, s); test(bidirectional_iterator(s+1), bidirectional_iterator(s)); @@ -64,8 +67,11 @@ int main(int, char**) #if TEST_STD_VER > 14 { constexpr const char* s = "1234567890"; - static_assert( constexpr_test(bidirectional_iterator(s+10), 10, bidirectional_iterator(s)), "" ); - static_assert( constexpr_test(random_access_iterator(s+10), 10, random_access_iterator(s)), "" ); + static_assert( constexpr_test(forward_iterator (s), -10, forward_iterator (s+10)), "" ); + static_assert( constexpr_test(bidirectional_iterator(s+10), 10, bidirectional_iterator(s)), "" ); + static_assert( constexpr_test(forward_iterator (s), -10, forward_iterator (s+10)), "" ); + static_assert( constexpr_test(random_access_iterator(s+10), 10, random_access_iterator(s)), "" ); + static_assert( constexpr_test(forward_iterator (s), -10, forward_iterator (s+10)), "" ); static_assert( constexpr_test(s+10, 10, s), "" ); static_assert( constexpr_test(bidirectional_iterator(s+1), bidirectional_iterator(s)), "" ); -- 2.7.4