Skip to content

Commit

Permalink
More MSVC debug iterator fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
eliaskosunen committed Oct 27, 2023
1 parent 0314fa2 commit 586e093
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 68 deletions.
5 changes: 2 additions & 3 deletions include/scn/detail/input_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,8 @@ namespace scn {
const CharT*,
std::size_t>)
{
auto b = to_address(ranges::begin(r));
return {b, static_cast<std::size_t>(ranges::distance(
b, to_address(ranges::end(r))))};
return make_string_view_from_pointers<CharT>(
to_address(ranges::begin(r)), to_address(ranges::end(r)));
}

SCN_GCC_POP
Expand Down
25 changes: 22 additions & 3 deletions include/scn/detail/pp_detect.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,26 @@
#define SCN_HAS_INCLUDE(x) 0
#endif

#define SCN_STD_17 201703L
#define SCN_STD_20 202002L

#if SCN_HAS_INCLUDE(<version>)
#include <version>

#ifdef _MSC_VER
// _ITERATOR_DEBUG_LEVEL isn't defined in <version> (needed below)
// <ciso646> is removed in C++20, and including it will cause a warning
// So, include <yvals.h> from the STL directly, if available, or otherwise fall back on a small C header
#if SCN_HAS_INCLUDE(<yvals.h>)
#include <yvals.h>
#else
#include <ciso646>
#include <cstddef>
#endif
#endif // _MSC_VER

#define SCN_STD_17 201703L
#define SCN_STD_20 202002L
#else // has_include(<version>)
#include <ciso646>
#endif

#define SCN_COMPILER(major, minor, patch) \
((major)*10'000'000 + (minor)*10'000 + (patch))
Expand Down Expand Up @@ -126,6 +138,13 @@
#define SCN_STDLIB_MS_STL 0
#endif

// MSVC debug iterators
#if SCN_STDLIB_MS_STL && defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL != 0
#define SCN_MSVC_DEBUG_ITERATORS 1
#else
#define SCN_MSVC_DEBUG_ITERATORS 0
#endif

// POSIX
#if defined(__unix__) || defined(__APPLE__)
#define SCN_POSIX 1
Expand Down
39 changes: 36 additions & 3 deletions include/scn/util/string_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace scn {
if constexpr (std::is_constructible_v<std::basic_string_view<CharT>,
decltype(first),
decltype(last)> &&
!SCN_STDLIB_MS_STL) {
!SCN_MSVC_DEBUG_ITERATORS) {
return {first, last};
}
else {
Expand All @@ -54,16 +54,49 @@ namespace scn {
}
}

template <typename CharT>
constexpr std::basic_string_view<CharT> make_string_view_from_pointers(
const CharT* first,
const CharT* last)
{
if constexpr (std::is_constructible_v<std::basic_string_view<CharT>,
const CharT*, const CharT*>) {
return {first, last};
}
else {
return {first, static_cast<size_t>(std::distance(first, last))};
}
}

template <typename CharT>
constexpr auto make_string_view_iterator(
std::basic_string_view<CharT> sv,
typename std::basic_string_view<CharT>::iterator it) ->
typename std::basic_string_view<CharT>::iterator
{
if constexpr (std::is_constructible_v<
typename std::basic_string_view<CharT>::iterator,
decltype(it)> &&
!SCN_MSVC_DEBUG_ITERATORS) {
SCN_UNUSED(sv);
return it;
}
else {
return sv.begin() +
std::distance(sv.data(), detail::to_address(it));
}
}

template <typename CharT>
constexpr auto make_string_view_iterator_from_pointer(
std::basic_string_view<CharT> sv,
const CharT* ptr) ->
typename std::basic_string_view<CharT>::iterator
{
if constexpr (std::is_constructible_v<
typename std::basic_string_view<CharT>,
typename std::basic_string_view<CharT>::iterator,
const CharT*> &&
!SCN_STDLIB_MS_STL) {
!SCN_MSVC_DEBUG_ITERATORS) {
SCN_UNUSED(sv);
return ptr;
}
Expand Down
11 changes: 7 additions & 4 deletions src/scn/impl/algorithms/eof_check.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace scn {
SCN_BEGIN_NAMESPACE

namespace impl {
#if !SCN_STD_RANGES && SCN_STDLIB_MS_STL
#if !SCN_STD_RANGES && SCN_MSVC_DEBUG_ITERATORS
#define SCN_NEED_MS_DEBUG_ITERATOR_WORKAROUND 1
#else
#define SCN_NEED_MS_DEBUG_ITERATOR_WORKAROUND 0
Expand All @@ -48,7 +48,7 @@ namespace scn {
constexpr auto range_nocopy_data(R&& r) SCN_NOEXCEPT
{
static_assert(range_supports_nocopy<R>());
#if SCN_STDLIB_MS_STL
#if SCN_NEED_MS_DEBUG_ITERATOR_WORKAROUND
return detail::to_address(ranges::begin(SCN_FWD(r)));
#else
return ranges::data(SCN_FWD(r));
Expand All @@ -59,7 +59,7 @@ namespace scn {
constexpr auto range_nocopy_size(R&& r) SCN_NOEXCEPT
{
static_assert(range_supports_nocopy<R>());
#if SCN_STDLIB_MS_STL
#if SCN_NEED_MS_DEBUG_ITERATOR_WORKAROUND
return static_cast<size_t>(
ranges::distance(detail::to_address(ranges::begin(r)),
detail::to_address(ranges::end(r))));
Expand All @@ -71,12 +71,15 @@ namespace scn {
template <typename I, typename S>
SCN_NODISCARD constexpr bool is_range_eof(I begin, S end)
{
#if SCN_NEED_MS_DEBUG_ITERATOR_WORKAROUND
if constexpr (ranges_std::contiguous_iterator<I> ||
(ranges_std::random_access_iterator<I> &&
detail::can_make_address_from_iterator<I>::value)) {
return detail::to_address(begin) == detail::to_address(end);
}
else {
else
#endif
{
return begin == end;
}
}
Expand Down
41 changes: 18 additions & 23 deletions src/scn/impl/algorithms/find_whitespace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,44 +40,39 @@ namespace scn {
std::string_view::iterator
find_classic_impl(std::string_view source, CuCb cu_cb, CpCb cp_cb)
{
auto it = source.data();
const auto end = source.data() + source.size();
auto it = source.begin();

while (it != end) {
SCN_EXPECT(it < end);
auto sv =
std::string_view{
it, static_cast<size_t>(ranges::distance(
it, detail::to_address(source.end())))}
.substr(0, 8);
while (it != source.end()) {
auto sv = detail::make_string_view_from_iterators<char>(
it, source.end())
.substr(0, 8);

if (!has_nonascii_char_64(sv)) {
auto i = ranges::find_if(sv, cu_cb);
it = detail::to_address(i);
if (i != sv.end()) {
auto tmp_it = ranges::find_if(sv, cu_cb);
it = detail::make_string_view_iterator(source, tmp_it);
if (tmp_it != sv.end()) {
break;
}
continue;
}

for (size_t i = 0; i < sv.size(); ++i) {
auto tmp = std::string_view{
detail::to_address(it),
static_cast<size_t>(ranges::distance(it, end))};
auto tmp =
detail::make_string_view_from_iterators<char>(
it, source.end());
auto res = get_next_code_point(tmp);
if (cp_cb(res.value)) {
return source.begin() +
ranges::distance(source.data(), it);
return it;
}
auto n = ranges::distance(
tmp.data(), detail::to_address(res.iterator));
it += n;
i += n;
SCN_ENSURE(it <= end);
i += ranges::distance(tmp.data(),
detail::to_address(res.iterator));
it = detail::make_string_view_iterator(source,
res.iterator);
SCN_ENSURE(it <= source.end());
}
}

return source.begin() + ranges::distance(source.data(), it);
return detail::make_string_view_iterator(source, it);
}

bool is_decimal_digit(char ch) SCN_NOEXCEPT
Expand Down
16 changes: 7 additions & 9 deletions src/scn/impl/unicode/unicode.h
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ namespace scn {
++it;
continue;
}
if (len > ranges::distance(it, input.end())) {
if (len >
static_cast<size_t>(ranges::distance(it, input.end()))) {
return input.end();
}

Expand Down Expand Up @@ -599,7 +600,7 @@ namespace scn {
auto res = do_transcode(sv, tmp_view);
if (SCN_LIKELY(res.error == simdutf::SUCCESS)) {
dest.append(tmp.data(), std::min(res.count, tmp.size()));
it = sv.end();
it = detail::make_string_view_iterator(source, sv.end());
continue;
}

Expand All @@ -622,9 +623,10 @@ namespace scn {
dest.push_back(DestCharT{0xfffd});
}

it = find_start_of_next_valid_code_point(
auto tmp_it = find_start_of_next_valid_code_point(
detail::make_string_view_from_iterators<SourceCharT>(
it, source.end()));
it = detail::make_string_view_iterator(source, tmp_it);
}
}

Expand All @@ -637,9 +639,7 @@ namespace scn {
detail::make_string_view_from_iterators<CharT>(
it, input.end()));
cb(res.value);

it += ranges::distance(detail::to_address(it),
detail::to_address(res.iterator));
it = detail::make_string_view_iterator(input, res.iterator);
}
}

Expand All @@ -653,9 +653,7 @@ namespace scn {
detail::make_string_view_from_iterators<CharT>(
it, input.end()));
cb(res.value);

it += ranges::distance(detail::to_address(it),
detail::to_address(res.iterator));
it = detail::make_string_view_iterator(input, res.iterator);
}
}
} // namespace impl
Expand Down
56 changes: 33 additions & 23 deletions tests/unittests/impl_tests/find_fast_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,32 +53,32 @@ TEST(FindClassicSpaceNarrowFastTest, WonkyLongInput)

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 24);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 27);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 28);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 44);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 51);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(scn::detail::to_address(it), src.data() + src.size());
}
TEST(FindClassicSpaceNarrowFastTest, WonkyLongInput2)
Expand All @@ -92,32 +92,32 @@ TEST(FindClassicSpaceNarrowFastTest, WonkyLongInput2)

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 24);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 27);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 28);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 44);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(&*it, src.data() + 51);

++it;
it = scn::impl::find_classic_space_narrow_fast(
{&*it, static_cast<size_t>(src.end() - it)});
scn::detail::make_string_view_from_iterators<char>(it, src.end()));
EXPECT_EQ(scn::detail::to_address(it), src.data() + src.size());
}
TEST(FindClassicSpaceNarrowFastTest, WonkyInput3)
Expand All @@ -135,15 +135,25 @@ TEST(FindClassicNonspaceNarrowFastTest, EmojiInput)
{
auto input = "😂\n"sv;

EXPECT_EQ(scn::impl::find_classic_nonspace_narrow_fast(input.substr(0)),
input.begin() + 0);
EXPECT_EQ(scn::impl::find_classic_nonspace_narrow_fast(input.substr(1)),
input.begin() + 1);
EXPECT_EQ(scn::impl::find_classic_nonspace_narrow_fast(input.substr(2)),
input.begin() + 2);
EXPECT_EQ(scn::impl::find_classic_nonspace_narrow_fast(input.substr(3)),
input.begin() + 3);

EXPECT_EQ(scn::impl::find_classic_nonspace_narrow_fast(input.substr(4)),
input.begin() + 5);
EXPECT_EQ(
scn::detail::to_address(
scn::impl::find_classic_nonspace_narrow_fast(input.substr(0))),
input.data() + 0);
EXPECT_EQ(
scn::detail::to_address(
scn::impl::find_classic_nonspace_narrow_fast(input.substr(1))),
input.data() + 1);
EXPECT_EQ(
scn::detail::to_address(
scn::impl::find_classic_nonspace_narrow_fast(input.substr(2))),
input.data() + 2);
EXPECT_EQ(
scn::detail::to_address(
scn::impl::find_classic_nonspace_narrow_fast(input.substr(3))),
input.data() + 3);

EXPECT_EQ(
scn::detail::to_address(
scn::impl::find_classic_nonspace_narrow_fast(input.substr(4))),
input.data() + 5);
}

0 comments on commit 586e093

Please sign in to comment.