boostorg / gil

Boost.GIL - Generic Image Library | Requires C++14 since Boost 1.80
https://boostorg.github.io/gil
Boost Software License 1.0
178 stars 163 forks source link

refactor: replace Boost.Iterator with Boost.STLInterfaces #669

Open sdebionne opened 2 years ago

sdebionne commented 2 years ago

Description

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

References

C++11 Modernization

Tasklist

sdebionne commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

This is included in the planning towards C++14/17 discussion here https://github.com/boostorg/gil/discussions/676

mloskot commented 2 years ago

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

sdebionne commented 2 years ago

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[bot] commented 11 months ago

Codecov Report

Merging #669 (72d460c) into develop (1df8c24) will increase coverage by 1.03%. Report is 6 commits behind head on develop. The diff coverage is 100.00%.

:exclamation: Current head 72d460c differs from pull request most recent head 4470d11. Consider uploading reports for the commit 4470d11 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #669 +/- ## =========================================== + Coverage 81.10% 82.14% +1.03% =========================================== Files 117 117 Lines 5171 5360 +189 =========================================== + Hits 4194 4403 +209 + Misses 977 957 -20 ```
sdebionne commented 11 months ago

@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 commented 11 months ago

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:

image

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

Let's see if this improves anything.

sdebionne commented 11 months ago

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

sdebionne commented 11 months ago

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

mloskot commented 11 months ago

Yes, I think it should.

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