Replace Boost.Variant with Boost.Variant2

[sdebionne]

Description

Implements dynamic extension with Boost.Variant2 instead of Boost.Variant.

References

Fixes #232

Tasklist

• Update/Add test case(s)
• Update doc(s)
• Update extension usage in I/O
• Ensure all CI builds pass
• Review and approve

[mloskot]

Thanks, I'm very much in favour of switching to the Boost.Variant2

[sdebionne]

Builds fine with MSVC 2019.

[sdebionne]

@pdimov Any chance that this issue that seems to be specific to MSVC 2017 rings a bell?

boost\mp11\list.hpp(202): error C3200: 'B': invalid template argument for template parameter 'B', expected a class template

[pdimov]

My guess is that it doesn't like

using view_t = mp11::mp_rename<detail::images_get_views_t<any_image>, any_image_view>;

either because it doesn't like any_image or any_image_view. I'll look into why.

[pdimov]

Hm no, it's actually this line:

template <typename ...Views>
class any_image_view : public variant2::variant<Views...>
{
    using parent_t = variant2::variant<Views...>;

public:    
    using const_t = mp11::mp_rename<detail::views_get_const_t<any_image_view>, any_image_view>;

The second any_image_view is interpreted as referring to the class (same as the first one).

[pdimov]

Since the result of detail::view_get_const_t<any_image_view> is already any_image_view, just drop the rename.

And maybe use b2 instead of cmake in order to see the errors.

compile-c-c++ ..\..\bin.v2\libs\gil\test\extension\toolbox\get_num_bits.test\msv
c-14.1\debug\asynch-exceptions-on\threading-multi\get_num_bits.obj
cl : Command line warning D9025 : overriding '/W3' with '/W4'
get_num_bits.cpp
C:\boost-git\develop\boost/mp11/list.hpp(202): error C3200: 'B': invalid templat
e argument for template parameter 'B', expected a class template
        with
        [
            B=boost::gil::any_image_view<Views...>
        ]
C:\boost-git\develop\boost/gil/extension/dynamic_image/any_image_view.hpp(79): n
ote: see reference to alias template instantiation 'boost::mp11::mp_rename<boost
::gil::detail::views_get_const_t<boost::gil::any_image_view<Views...>>,boost::gi
l::any_image_view<Views...>>' being compiled
C:\boost-git\develop\boost/gil/extension/dynamic_image/any_image_view.hpp(122):
note: see reference to class template instantiation 'boost::gil::any_image_view<
Views...>' being compiled

[pdimov]

C:\boost-git\develop\libs\gil>git diff
diff --git a/include/boost/gil/extension/dynamic_image/any_image_view.hpp b/include/boost/gil/extension/dynamic_image/any_image_view.hpp
index 1119b2f7f..f4a2f7351 100644
--- a/include/boost/gil/extension/dynamic_image/any_image_view.hpp
+++ b/include/boost/gil/extension/dynamic_image/any_image_view.hpp
@@ -76,7 +76,7 @@ class any_image_view : public variant2::variant<Views...>
     using parent_t = variant2::variant<Views...>;

 public:
-    using const_t = mp11::mp_rename<detail::views_get_const_t<any_image_view>, any_image_view>;
+    using const_t = detail::views_get_const_t<any_image_view>;
     //using const_t = detail::views_get_const_t<any_image_view>;
     using x_coord_t = std::ptrdiff_t;
     using y_coord_t = std::ptrdiff_t;

[pdimov]

I see that you already had the right thing there but commented out. :-)

[sdebionne]

Thank you so much for your help, I am always amazed by the support you get from this community.

Why on earth did I add this mp_rename ? :-)

If I understand correctly, mp_rename<A<T...>, A> fails on MSVC 2017 or is it just with any_image_view?

[pdimov]

It only fails when you're inside the definition of template<class... T> class A, because there A can refer to both the class itself (A<T...>, "class name injection") and the class template (A), and 2017 doesn't consider the second possibility. You were basically doing something similar to mp_rename<A, A>, where the two As mean different things.

[sdebionne]

One of the Azure jobs failed with:

Cloning into 'boost-root'...
C:/Program Files/Git/mingw64/libexec/git-core\git-submodule: line 457:  1643 Segmentation fault      git submodule--helper ensure-core-worktree "$sm_path"

Any chance that it could be restarted?

[mloskot]

@sdebionne Restarted

[mloskot]

Awesome, thank you for contributions! For me, it's ready to merge. Let's give others a day or two to review.

I think we could also get rid of this bridge now

gil/include/boost/gil/detail/mp11.hpp

Line 13 in a7a2cb2

#include <boost/mp11/mpl.hpp> // required by dynamic_image and boost::variant (?)

[stefanseefeld]

This looks like a great improvement to the dynamic_image extension.
I'm a bit wary, though, seeing that all our I/O extensions (and more !) depend on that. Wouldn't there be a way to split this, so someone can use the PNG reader without having to drag in boost/variant2.hpp, say ?

[mloskot]

@stefanseefeld

Wouldn't there be a way to split this, so someone can use the PNG reader without having to drag in boost/variant2.hpp

Currently, those all drag in boost/variant.hpp, so it's NOT adding a new dependency, but replacing an old A with new B. And, as the new B has less dependencies than the old A, it means removing some transitive dependencies. Unless I'm missing something.

[stefanseefeld]

OK, that's a fair point. So let's move forward with this PR, and try to separate the dynamic_image extension dependency from the other extensions separately.

[mloskot]

@stefanseefeld Yes, I think this PR is completed now and feasibility of decoupling the dynamic_image and io extensions should be a matter of separate discussion and PR.
So, unless you have any other requests for changes, let's mark this as approved.