Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with optional types #28

Open
whytro opened this issue Sep 25, 2023 · 2 comments
Open

Issue with optional types #28

whytro opened this issue Sep 25, 2023 · 2 comments

Comments

@whytro
Copy link
Collaborator

whytro commented Sep 25, 2023

This is a leftover issue from GSoC that I've been thinking about, and one that I think that should be fixed.
In its current state, some bindings for optional types (ie. optional approaches) do not work properly - some cannot be used, or do not return a "tangible" value.

pybind11 offered an optional_caster so you could bind other optional types like boost::optional, but nanobind does not, and only has default bindings for the STL optionals.

After experimenting with overloading the optional bindings, I've started to think that it might be cleaner to knock out two issues at once by looking at converting boost::optional references in osrm-backend to their std::optional counterpart, though an initial test reveals that it might not be so simple due to the referred boost::spirit reliance on boost::optional, and while x3 might support std::optional, going from qi to x3 seems to require some rewriting of the parsing rules/grammar/etc.

@nilsnolde
Copy link
Owner

Hi again:) Great to have you still thinking about this. I didn't find the time yet to test stuff on all platforms, but that's still on my to do, then we also still need to pre-release and see that that works.

Regarding converting osrm-backend to std::optional, see Project-OSRM/osrm-backend#6611. You came to a similar conclusion. If you're set to make this work, it seems to me your first approach might be faster (and less frustrating, albeit less ideal). @SiarheiFedartsou any opinion?

@whytro
Copy link
Collaborator Author

whytro commented Sep 25, 2023

Hello! This issue was bugging me a little, so I've decided to try and fix it, for usability.

I've taken a look at the linked issue, but could not find any file changes related to upgrading to x3. While moving from boost::optional to std::optional is essentially a "slot-in" replacement, qi to x3 requires a bit more of a substantial rewrite because they've made some fairly major changes between the versions (ie. grammars have been replaced by groups of rules, some previously provided parsers have been removed in favor of user defined lambdas).

But, the first approach is something that I'm keeping in mind, since if I'm unable to move osrm-backend to std::optional in time, I figure that it's at least a good band-aid fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants