refactor: replace Boost.Iterator with Boost.STLInterfaces

[sdebionne]

Description

Contribute to remove dependencies to older c++03 boost libraries.

References

C++11 Modernization

Tasklist

• Add test case(s)
• Ensure all CI builds pass
• Review and approve

[sdebionne]

@mloskot Boost.Iterator provides it's own Concept checking classes in #include <boost/iterator/iterator_concepts.hpp> that matche the (new for the time) Boost.Iterator concepts that never made it to the standard. Since we want to cut the ties with Boost.Iterator, I suppose we need to move to Boost.Concept Iterator concept, what do you think?

Honestly I wonder if it is worth it, and if, for concept checking only, we could require c++20....

[mloskot]

@sdebionne

Honestly I wonder if it is worth it, and if, for concept checking only, we could require c++20...

I don't think it is worth it ...

to move to Boost.Concept Iterator concept,

One of my "next big thing" to do for GIL is to:

1. Freeze current use of Boost.Concept
2. Introduce C++20 concepts (compile-time disabled for older compilation modes)
3. Ensure we CI-test at C++20 level thoroughly (core + extensions) with those concepts enabled.
4. Slowly deprecated use of Boost.Concept, and eventually remove it.

@sdebionne Does the above seem sensible to you?

[mloskot]

This is included in the planning towards C++14/17 discussion here #676

[mloskot]

I guess we may run out of time to squeeze it into Boost 1.80. What do you think @sdebionne ?

[sdebionne]

I guess we may run out of time to squeeze it into Boost 1.80. What do you think @sdebionne ?

Indeed, I underestimate the work and overestimate the time I could spend on the project. Let's move it to 1.81.

[codecov]

Codecov Report

❌ Patch coverage is 97.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.05%. Comparing base (8a8d969) to head (b409769).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #669      +/-   ##
===========================================
+ Coverage    81.97%   82.05%   +0.08%     
===========================================
  Files          117      117              
  Lines         5353     5383      +30     
===========================================
+ Hits          4388     4417      +29     
- Misses         965      966       +1     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sdebionne]

@mloskot there is still a bit of cleaning to do but otherwise this PR is ready for review. Beside the switch to Boost.StlInferface, there is a fix for the CI (code coverage), and a fix for std::is_floating_point<> specialization.

I don't known why some of the CI GitHub actions timeout -I have just try to rerun them.

[mloskot]

I don't known why some of the CI GitHub actions timeout -I have just try to rerun them.

It looks like GHA runners availability issue which I'd ignore:

The Ubuntu 18.04 has also been deprecated, so I have just tried to update the GHA:

• ci: ubuntu-18.04 environment is deprecated #738

Let's see if this improves anything.

[mloskot]

Thank you so much for pulling this off @sdebionne

[sdebionne]

I'll rebase on the fixed CI (thank you for that) and cleanup.

[sdebionne]

I wonder if iterator should be removed from .ci/get-boost.sh?

[mloskot]

Yes, I think it should.

It may be required as transitive dependency, but the CI jobs will let us know.

[sdebionne]

Concept checking errors with stdcxx=20 to be dealt with:

./boost/concept_check.hpp:209:13: error: conversion from ‘std::input_iterator_tag’ to non-scalar type ‘std::bidirectional_iterator_tag’ requested

[sdebionne]

The test in legacy/image.cpp segfault in c++20 when using Boost.STLInterfaces.

It does not crash with the old Boost.Iterator implementation. I compiled the develop branch's legacy/image.cpp (still using iterator_facade) at the exact same -std=c++20 with the same compiler, and it runs to completion cleanly: EXIT: 0, all ~120 checksums pass, including the planarrgb8_* cases and everything after — no segfault at all.

So this is a genuine regression introduced by the Boost.Iterator → Boost.STLInterfaces conversion, specifically its C++20 concepts-enabled code path when handling nested step iterators (memory_based_step_iterator<memory_based_step_iterator<pixel*>>), and it's unrelated to my iterator_category fix (which only affects 's legacy tag-dispatch and compile-time concept checks, not this crash). It's a real bug to chase down before this branch can be considered done.

[sdebionne]

Root cause
step_iterator_adaptor (used by memory_based_step_iterator) defines its own scaled operator- as a member function:

constexpr auto operator-(step_iterator_adaptor other) const noexcept { return -distance_to(other); }

Boost.STLInterfaces' C++20 "concepts" code path (only active at cxxstd≥20) also auto-generates a fallback operator- as a free/hidden-friend template, unconditionally enabled whenever the raw underlying iterator supports subtraction:

template<typename D1, typename D2>
constexpr auto operator-(D1 lhs, D2 rhs)
  requires ... requires { access::base(lhs) - access::base(rhs); }
    { return access::base(lhs) - access::base(rhs); }

For memory_based_step_iterator<pixel*>, base() returns the raw pixel*, which is directly subtractable — so this fallback is always a candidate. Calling b - a requires converting a/b (type Derived) to invoke GIL's member operator (a derived-to-base conversion for the implicit object parameter), while the library's template fallback deduces D1=D2=Derived with zero conversions. An exact match beats a conversion, so GCC silently picks the library's fallback — returning the raw, unscaled pointer difference instead of the step-divided element count. That wrong (too-large) n then feeds directly into std::__copy_m's for (n = last-first; n>0; --n) loop, which walks the iterator far past the end of the buffer with no bounds re-check. It only shows up at cxxstd=20 because that's exactly the threshold where the library's "concepts" overload set activates.

Fix
Changed operator- from a member into a friend with both parameters typed as the exact Derived (not the base step_iterator_adaptor), in step_iterator.hpp]:

friend constexpr auto operator-(Derived const& lhs, Derived const& rhs) noexcept -> difference_type
{
    return -lhs.distance_to(rhs);
}

Now it's an equally-exact match against the library's template fallback, and the tie-break rule ("prefer a non-template over a function-template specialization") makes GCC choose GIL's correctly-scaled version. This also fixes <, <=, >, >=, == for these iterators, since stl_interfaces derives all of those generically from operator-.

[sdebionne]

Many clang builds fail with

error: use of overloaded operator '++' is ambiguous ...

Root cause
three iterator classes (iterator_from_2d, position_iterator, planar_pixel_iterator) define their own fast prefix operator++()/operator--() and used using parent_t::operator++;/using parent_t::operator--; just to pull in the CRTP base's postfix versions. A using-declaration brings in the entire overload set by that name, including the prefix — colliding with the derived class's own prefix. GCC silently resolves this via ordinary hiding rules; clang-20 treats it as a genuine ambiguity (use of overloaded operator '++' is ambiguous) or, in some instantiations, finds no viable candidate at all (cannot increment value of type ...).

Fix
dropped the using parent_t::operator++/--; lines and added explicit local postfix operators in each of the three headers (iterator_from_2d.hpp, position_iterator.hpp, planar_pixel_iterator.hpp) — no reliance on inheritance ambiguity-resolution rules at all, so it's portable across compilers.

[sdebionne]

CI fixes:

• Dropped the old Ubuntu 18.04 platform, the CI would not event start.
• Remove cxxstd=20 for 11<= clang <= 15 since the implementation of concept is broken:

Older clang versions simply cannot reliably build this GIL code (or, likely, any stl_interfaces-based CRTP iterator) in C++20 mode — this is a compiler limitation, not a code defect. If your CI needs clang-14 coverage, restricting it to cxxstd<=17 would get full, clean passes; C++20 testing would need clang-16+ (where this class of concepts bug was fixed) or gcc.

[sdebionne]

One last issue on macos Sonoma

/bin/sh: line 4: 63660 Abort trap: 6           "bin.v2/libs/gil/test/core/io/path_spec.test/clang-darwin-15/release/arm_64/cxxstd-20-iso/threading-multi/visibility-hidden/path_spec" > "bin.v2/libs/gil/test/core/io/path_spec.test/clang-darwin-15/release/arm_64/cxxstd-20-iso/threading-multi/visibility-hidden/path_spec.output" 2>&1 < /dev/null
====== BEGIN OUTPUT ======
libc++abi: terminating due to uncaught exception of type std::runtime_error: collate_byname<char>::collate_byname failed to construct for C.UTF-8

Root cause
path_spec.cpp hardcoded std::locale("C.UTF-8") as the global locale. C.UTF-8 is a glibc/GNU extension locale name — it doesn't exist in macOS's locale database. On Darwin, libc++'s std::locale constructor throws std::runtime_error for an unrecognized name, and since nothing caught it, it propagated out of main as an uncaught exception, which libc++abi reports as an abort.

Fix
Edited path_spec.cpp to try a short list of common UTF-8 locale names (C.UTF-8, en_US.UTF-8, UTF-8, en_US.utf8) in order, catching std::runtime_error for each and falling through to the next candidate, using whichever one the platform actually has installed. This is a general portability fix, not a macOS-only patch, since it doesn't assume any specific platform's naming convention.