[WIP] replace mbgl::variant with std::variant

[bencsikandrei]

This is part 1/n of a series of commits (or PRs?) towards solving: #893

My idea for this PR is to not aim at fully resolving this issue, as I believe it would be too much for one PR, but rather make small increments towards that goal (solve the simple things first, etc...)

My decision was to split into at a couple commits since I have some concerns regarding replacing some of the functionality of mbgl::variant with std::variant equivalents (.match and some of its operator== semantics come to mind)

Please let me know if my decision suits what you have envisioned for this task

Thanks!

[bencsikandrei]

Looks great overall but I would like to know why the Overload helper is needed.

the API for std::visit is quite different from the one in mapbox::variant::match, in that it favors applying the same visitor to multiple variant objects (via variadic template arguments) as opposed to multiple callables on the same variant object

the API is as follows: template <class Visitor, class... Variants> constexpr auto visit( Visitor&& vis, Variants&&... vars );

which means we've got to pass 1 visitor for N (which could also be 0) variants

in order to obtain the same behavior as mapbox::variant::match (which AFAIK takes a variadic template argument pack for multiple function like objects) we need to pass some sort of function like object which contains all the visit functions we need

one way to do so is to manually create a visitor with multiple overloads for operator()(...)

e.g.: struct Visitor { void operator()(int); void operator()(double); ..}; which we could apply to a variant<int, double>, but that's tedious and we already have the lambdas from the old calls to match so here's where the Overloaded thing comes in

it effectively inherits from a bunch of lambdas (so it has all their operator()(...)) in a single place (it, unfortunately, also needs a deduction guide for c++17 to allow for {..} initialization from the lambdas)

maybe I made the explanation way too long sorry for that!

here's where I took it from, by the way: https://en.cppreference.com/w/cpp/utility/variant/visit

[louwers]

Please click re-request review when you are ready! Thanks 🙂

[github-actions[bot]]

Test results at https://maplibre-native-test-artifacts.s3.eu-central-1.amazonaws.com/4824605626-linux-gcc8-release-style.html

[github-actions[bot]]

Test results at https://maplibre-native-test-artifacts.s3.eu-central-1.amazonaws.com/4840695425-linux-gcc8-release-style.html

[bencsikandrei]

Hi!

I've been working on this on and off but have struggled with a couple things in the public headers (notably changes involving some stuff that mbgl::variant and std::variant handle differently - mostly related to operator== and co.).

In any case since this has been a long PR and has been lingering I've had to rebase several times.

I believe the changes that exist now (that don't touch public headers) could be merged in a first step.

Then we can move to the other (fewer) changes to the public headers and a couple platform things. Those will, hopefully, be faster to do than this one.

I'd rather have everything done in one PR but I've not had the time to figure out what happens to a couple headers when I changed to std::variant (the compiler cries for help with errors).

Keeping this up to date will become increasingly harder if it lingers and I'd rather at least make a small step.

What do you think?

[bencsikandrei]

I also have 0 idea what this wants from me:

`/platform/macos/src/MLNMapViewDelegate.h:88:63: error: expected a type

• (BOOL)mapView:(MLNMapView *)mapView shouldChangeFromCamera:(MLNMapCamera *)oldCamera toCamera:(MLNMapCamera *)newCamera; ^ ./platform/macos/src/MLNMapViewDelegate.h:88:98: error: expected a type
• (BOOL)mapView:(MLNMapView *)mapView shouldChangeFromCamera:(MLNMapCamera *)oldCamera toCamera:(MLNMapCamera *)newCamera; ^ ./platform/macos/src/MLNMapViewDelegate.h:182:62: error: expected a type
• (void)mapView:(MLNMapView *)mapView didFinishLoadingStyle:(MLNStyle *)style; ^ ./platform/macos/src/MLNMapViewDelegate.h:212:88: error: no type or protocol named 'MLNAnnotation'
• (nullable MLNAnnotationImage *)mapView:(MLNMapView *)mapView imageForAnnotation:(id <MLNAnnotation>)annotation; ^ ./platform/macos/src/MLNMapViewDelegate.h:244:4: error: expected a type
• (NSColor *)mapView:(MLNMapView *)mapView strokeColorForShapeAnnotation:(MLNShape *)annotation; ^ ./platform/macos/src/MLNMapViewDelegate.h:259:4: error: expected a type
• (NSColor *)mapView:(MLNMapView *)mapView fillColorForPolygonAnnotation:(MLNPolygon *)annotation; ^ ./platform/macos/src/MLNMapViewDelegate.h:295:64: error: no type or protocol named 'MLNAnnotation'
• (void)mapView:(MLNMapView *)mapView didSelectAnnotation:(id <MLNAnnotation>)annotation; ^ ./platform/macos/src/MLNMapViewDelegate.h:305:66: error: no type or protocol named 'MLNAnnotation'
• (void)mapView:(MLNMapView *)mapView didDeselectAnnotation:(id <MLNAnnotation>)annotation; ^ ./platform/macos/src/MLNMapViewDelegate.h:330:69: error: no type or protocol named 'MLNAnnotation'
• (BOOL)mapView:(MLNMapView *)mapView annotationCanShowCallout:(id <MLNAnnotation>)annotation; ^ ./platform/macos/src/MLNMapViewDelegate.h:348:13: error: expected a type
• (nullable NSViewController *)mapView:(MLNMapView *)mapView calloutViewControllerForAnnotation:(id <MLNAnnotation>)annotation; ^ ./platform/macos/src/MLNMapViewDelegate.h:348:102: error: no type or protocol named 'MLNAnnotation'
• (nullable NSViewController *)mapView:(MLNMapView *)mapView calloutViewControllerForAnnotation:(id <MLNAnnotation>)annotation; ^ :0: error: failed to import bridging header 'platform/macos/src/MLNMapViewDelegate.h'`

If someone could help me with iOS that would be amazing!

[github-actions[bot]]

Render test results at https://maplibre-native-test-artifacts.s3.eu-central-1.amazonaws.com/4930719458-linux-gcc8-release-style.html

[louwers]

@bencsikandrei Sorry for never replying, I am afraid I don't understand that error either.

Let's see if we can get this in a mergable state.