Replace boost::unordered_{map/set} with std, also remove code duplication by DennisOSRM · Pull Request #6429 · Project-OSRM/osrm-backend

DennisOSRM · 2022-10-29T20:58:36Z

No description provided.

…tion

SiarheiFedartsou · 2022-10-30T10:18:07Z

As we migrated to C++17 we can probably also get rid of boost::optional in favor of std::optional.

SiarheiFedartsou · 2022-10-30T10:23:20Z

Copy-pasting CI errors from Windows(as it was difficult for me to find in CI output):

2022-10-29T21:44:47.7797298Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::BaseParameters * __ptr64 __cdecl boost::proto::detail::proto_get_pointer<struct osrm::engine::api::BaseParameters,struct osrm::engine::api::TripParameters,struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64,struct osrm::engine::api::TripParameters * __ptr64,struct osrm::engine::api::BaseParameters * __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.7929761Z     63>D:/a/osrm-backend/osrm-backend/include/extractor/traffic_signals.hpp(15,58): error C2039: 'hash': is not a member of 'boost' [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.7993508Z        C:/.conan/287e98/1/include\boost/numeric/conversion/cast.hpp(36): message : see declaration of 'boost' [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.7995211Z     55>D:\a\osrm-backend\osrm-backend\third_party\sol2-3.3.0\include\sol\sol.hpp(15585): warning C4711: function 'int __cdecl sol::stack::push<char const * __ptr64,unsigned __int64>(struct lua_State * __ptr64,char const * __ptr64 && __ptr64,unsigned __int64 && __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\EXTRACTOR.vcxproj]
2022-10-29T21:44:47.7998719Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TripParameters & __ptr64 __cdecl boost::proto::detail::lvalue<struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8000715Z     55>D:\a\osrm-backend\osrm-backend\third_party\sol2-3.3.0\include\sol\sol.hpp(16268): warning C4711: function 'int __cdecl sol::stack::push<class std::basic_string_view<char,struct std::char_traits<char> > const & __ptr64>(struct lua_State * __ptr64,class std::basic_string_view<char,struct std::char_traits<char> > const & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\EXTRACTOR.vcxproj]
2022-10-29T21:44:47.8017966Z     63>D:/a/osrm-backend/osrm-backend/include/extractor/traffic_signals.hpp(15,62): error C2065: 'hash': undeclared identifier [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.8252167Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TripParameters * __ptr64 __cdecl boost::proto::detail::get_pointerns::get_pointer<struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8255322Z     55>D:\a\osrm-backend\osrm-backend\third_party\sol2-3.3.0\include\sol\sol.hpp(16269): warning C4711: function 'int __cdecl sol::stack::push<class sol::basic_table_core<0,class sol::basic_reference<0> > & __ptr64>(struct lua_State * __ptr64,class sol::basic_table_core<0,class sol::basic_reference<0> > & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\EXTRACTOR.vcxproj]
2022-10-29T21:44:47.8258752Z     59>C:\.conan\287e98\1\include\boost\spirit\home\qi\detail\expect_function.hpp(45): warning C4711: function 'void __cdecl boost::spirit::traits::clear_queue<class std::_String_iterator<class std::_String_val<struct std::_Simple_types<char> > > >(class std::_String_iterator<class std::_String_val<struct std::_Simple_types<char> > > & __ptr64,enum boost::spirit::traits::clear_mode)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8261746Z     55>D:\a\osrm-backend\osrm-backend\third_party\sol2-3.3.0\include\sol\sol.hpp(12775): warning C4711: function 'bool __cdecl sol::stack::loose_table_check<int (__cdecl*& __ptr64)(struct lua_State * __ptr64,int,enum sol::type,enum sol::type,char const * __ptr64) noexcept>(struct lua_State * __ptr64,int,int (__cdecl*& __ptr64)(struct lua_State * __ptr64,int,enum sol::type,enum sol::type,char const * __ptr64) noexcept,struct sol::stack::record & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\EXTRACTOR.vcxproj]
2022-10-29T21:44:47.8264952Z     59>C:\.conan\287e98\1\include\boost\spirit\home\support\algorithm\any_if.hpp(186): warning C4710: 'bool __cdecl boost::spirit::qi::detail::expect_function<class std::_String_iterator<class std::_String_val<struct std::_Simple_types<char> > >,struct boost::spirit::context<struct boost::fusion::cons<struct boost::spirit::unused_type & __ptr64,struct boost::fusion::cons<struct osrm::engine::api::MatchParameters & __ptr64,struct boost::fusion::nil_> >,struct boost::fusion::vector<> >,struct boost::spirit::unused_type,struct boost::spirit::qi::expectation_failure<class std::_String_iterator<class std::_String_val<struct std::_Simple_types<char> > > > >::operator()<struct boost::spirit::qi::any_int_parser<short,10,1,-1>,short>(struct boost::spirit::qi::any_int_parser<short,10,1,-1> const & __ptr64,short & __ptr64)const __ptr64': function not inlined [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8273921Z     59>C:\.conan\287e98\1\include\boost\spirit\home\support\argument.hpp(177): warning C4711: function 'enum osrm::engine::api::RouteParameters::GeometriesType & __ptr64 __cdecl boost::spirit::get_arg<0,struct boost::fusion::vector<enum osrm::engine::api::RouteParameters::GeometriesType & __ptr64> >(struct boost::fusion::vector<enum osrm::engine::api::RouteParameters::GeometriesType & __ptr64> & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8276079Z     59>C:\.conan\287e98\1\include\boost\phoenix\core\detail\function_eval.hpp(120): warning C4711: function 'struct osrm::engine::api::TripParameters & __ptr64 __cdecl boost::phoenix::detail::help_rvalue_deduction<struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8278587Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TripParameters & __ptr64 __cdecl boost::proto::detail::lvalue<struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8848026Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::RouteParameters * __ptr64 __cdecl boost::proto::detail::proto_get_pointer<struct osrm::engine::api::RouteParameters,struct osrm::engine::api::TripParameters,struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64,struct osrm::engine::api::TripParameters * __ptr64,struct osrm::engine::api::RouteParameters * __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8851979Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TripParameters & __ptr64 __cdecl boost::proto::detail::lvalue<struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8854461Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TripParameters * __ptr64 __cdecl boost::proto::detail::get_pointerns::get_pointer<struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8856005Z     63>D:/a/osrm-backend/osrm-backend/include/extractor/traffic_signals.hpp(15,68): error C2275: 'std::pair<NodeID,NodeID>': expected an expression instead of a type [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.8858234Z     59>C:\.conan\287e98\1\include\boost\spirit\home\qi\detail\expect_function.hpp(45): warning C4711: function 'void __cdecl boost::spirit::traits::clear_queue<class std::_String_iterator<class std::_String_val<struct std::_Simple_types<char> > > >(class std::_String_iterator<class std::_String_val<struct std::_Simple_types<char> > > & __ptr64,enum boost::spirit::traits::clear_mode)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8860768Z     59>C:\.conan\287e98\1\include\boost\spirit\home\support\argument.hpp(177): warning C4711: function 'enum osrm::engine::api::TableParameters::AnnotationsType & __ptr64 __cdecl boost::spirit::get_arg<0,struct boost::fusion::vector<enum osrm::engine::api::TableParameters::AnnotationsType & __ptr64> >(struct boost::fusion::vector<enum osrm::engine::api::TableParameters::AnnotationsType & __ptr64> & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.8862437Z     63>D:/a/osrm-backend/osrm-backend/include/extractor/traffic_signals.hpp(15,10): error C2974: 'std::unordered_set': invalid template argument for '_Hasher', type expected [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.8864069Z        C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.33.31629\include\unordered_set(63): message : see declaration of 'std::unordered_set' [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.8865289Z     63>D:/a/osrm-backend/osrm-backend/include/extractor/traffic_signals.hpp(15,10): error C2976: 'std::unordered_set': too few template arguments [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.8866719Z        C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.33.31629\include\unordered_set(63): message : see declaration of 'std::unordered_set' [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.8867825Z     63>D:/a/osrm-backend/osrm-backend/include/extractor/traffic_signals.hpp(15,89): error C2059: syntax error: '>' [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.8870346Z     59>C:\.conan\287e98\1\include\boost\phoenix\core\detail\function_eval.hpp(120): warning C4711: function 'struct osrm::engine::api::TableParameters & __ptr64 __cdecl boost::phoenix::detail::help_rvalue_deduction<struct osrm::engine::api::TableParameters>(struct osrm::engine::api::TableParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:47.9778100Z     63>D:/a/osrm-backend/osrm-backend/include/extractor/traffic_signals.hpp(16,32): error C2238: unexpected token(s) preceding ';' [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:47.9934623Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TableParameters & __ptr64 __cdecl boost::proto::detail::lvalue<struct osrm::engine::api::TableParameters>(struct osrm::engine::api::TableParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:48.0037769Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TableParameters * __ptr64 __cdecl boost::proto::detail::proto_get_pointer<struct osrm::engine::api::TableParameters,struct osrm::engine::api::TableParameters,struct osrm::engine::api::TableParameters>(struct osrm::engine::api::TableParameters & __ptr64,struct osrm::engine::api::TableParameters * __ptr64,struct osrm::engine::api::TableParameters * __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:48.0040017Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TableParameters & __ptr64 __cdecl boost::proto::detail::lvalue<struct osrm::engine::api::TableParameters>(struct osrm::engine::api::TableParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:48.0041907Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TableParameters * __ptr64 __cdecl boost::proto::detail::get_pointerns::get_pointer<struct osrm::engine::api::TableParameters>(struct osrm::engine::api::TableParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:48.0043960Z     59>C:\.conan\287e98\1\include\boost\spirit\home\qi\detail\expect_function.hpp(45): warning C4711: function 'void __cdecl boost::spirit::traits::clear_queue<class std::_String_iterator<class std::_String_val<struct std::_Simple_types<char> > > >(class std::_String_iterator<class std::_String_val<struct std::_Simple_types<char> > > & __ptr64,enum boost::spirit::traits::clear_mode)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:48.0045361Z     63>D:/a/osrm-backend/osrm-backend/include/extractor/traffic_signals.hpp(20,53): error C2065: 'unidirectional_segments': undeclared identifier [D:\a\osrm-backend\osrm-backend\build\check-headers.vcxproj]
2022-10-29T21:44:48.0132190Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::BaseParameters * __ptr64 __cdecl boost::proto::detail::proto_get_pointer<struct osrm::engine::api::BaseParameters,struct osrm::engine::api::TripParameters,struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64,struct osrm::engine::api::TripParameters * __ptr64,struct osrm::engine::api::BaseParameters * __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]
2022-10-29T21:44:48.0223494Z     59>C:\.conan\287e98\1\include\boost\phoenix\bind\detail\member_variable.hpp(55): warning C4711: function 'struct osrm::engine::api::TripParameters & __ptr64 __cdecl boost::proto::detail::lvalue<struct osrm::engine::api::TripParameters>(struct osrm::engine::api::TripParameters & __ptr64)' selected for automatic inline expansion [D:\a\osrm-backend\osrm-backend\build\SERVER.vcxproj]

At glance it is something about includes: AFAIR check-headers target checks "header sanity"(i.e. that everything needed is included in each header) and we have it enabled for Windows only at the moment.

DennisOSRM · 2022-10-30T10:24:41Z

As we migrated to C++17 we can probably also get rid of boost::optional in favor of std::optional.

Yes, working on removing more boost in an upcoming PR. One sticking point for optional is that the code currently stores optional references which isn't supported by std::optional. Arguably, that should be a pointer.

SiarheiFedartsou · 2022-10-30T10:26:51Z

One sticking point for optional is that the code currently stores optional references which isn't supported by std::optional. Arguably, that should be a pointer.

Hm, was just going to do that(migrate to std::optional) -it seemed to be a low hanging fruit 🤔 Will try if you don't mind :)

DennisOSRM · 2022-10-30T10:34:08Z

One sticking point for optional is that the code currently stores optional references which isn't supported by std::optional. Arguably, that should be a pointer.

Hm, was just going to do that(migrate to std::optional) -it seemed to be a low hanging fruit 🤔 Will try if you don't mind :)

yes, please go ahead. 😏

SiarheiFedartsou · 2022-10-30T10:38:13Z

One sticking point for optional is that the code currently stores optional references which isn't supported by std::optional. Arguably, that should be a pointer.

Hm, was just going to do that(migrate to std::optional) -it seemed to be a low hanging fruit 🤔 Will try if you don't mind :)

yes, please go ahead. 😏

Well, one more pitfall here is that it looks like boost::spirit doesn't support std::optional and works with boost::optional instead(boostorg/spirit_x4#15). We will probably get rid of boost::spirit code one day in the future(if we will migrate osrm-routed to JS or smth else), but now even though we can just convert boost::optional to std::optional after parsing it looks like there are no a lot of reasons to use both boost::optional and std::optional in the same codebase... WDYT?

DennisOSRM · 2022-10-30T10:44:45Z

Copy-pasting CI errors from Windows(as it was difficult for me to find in CI output):
...
At glance it is something about includes: AFAIR check-headers target checks "header sanity"(i.e. that everything needed is included in each header) and we have it enabled for Windows only at the moment.

Thanks for posting this. Should be fixed in 061f0a1.

DennisOSRM · 2022-10-30T10:48:06Z

One sticking point for optional is that the code currently stores optional references which isn't supported by std::optional. Arguably, that should be a pointer.

Hm, was just going to do that(migrate to std::optional) -it seemed to be a low hanging fruit 🤔 Will try if you don't mind :)

yes, please go ahead. 😏

Well, one more pitfall here is that it looks like boost::spirit doesn't support std::optional and works with boost::optional instead(boostorg/spirit_x4#15). We will probably get rid of boost::spirit code one day in the future(if we will migrate osrm-routed to JS or smth else), but now even though we can just convert boost::optional to std::optional after parsing it looks like there are no a lot of reasons to use both boost::optional and std::optional in the same codebase... WDYT?

Agreed, ultimately there shouldn't be a mix of std and boost implementations of the same types. Doesn't spirit X3 work with std::optional/variant?

SiarheiFedartsou · 2022-10-30T10:49:01Z

Agreed, ultimately there shouldn't be a mix of std and boost implementations of the same types. Doesn't spirit X3 work with std::optional/variant?

Tbh I don't know, it is my very first experience with boost::spirit :)

Replace boost::unordered_{map/set} with std, also remove code duplica…

7b73b97

…tion

DennisOSRM requested a review from mjjbell October 29, 2022 20:58

Fix formatting

f97e18d

SiarheiFedartsou approved these changes Oct 30, 2022

View reviewed changes

Fix includes

061f0a1

DennisOSRM merged commit 2f75d6f into master Oct 30, 2022

DennisOSRM deleted the less_boost_unordered branch October 30, 2022 21:17

SiarheiFedartsou mentioned this pull request Feb 23, 2023

Refactor codebase to use std::optional instead of boost::optional #6551

Closed

mugr1x mentioned this pull request Apr 20, 2023

Replace boost::optional with std::optional #6611

Merged

bevanjkay mentioned this pull request Apr 21, 2025

osrm-backend 6.0.0 Homebrew/homebrew-core#220847

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace boost::unordered_{map/set} with std, also remove code duplication#6429

Replace boost::unordered_{map/set} with std, also remove code duplication#6429
DennisOSRM merged 3 commits intomasterfrom
less_boost_unordered

DennisOSRM commented Oct 29, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

DennisOSRM commented Oct 30, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

DennisOSRM commented Oct 30, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

DennisOSRM commented Oct 30, 2022

Uh oh!

DennisOSRM commented Oct 30, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

DennisOSRM commented Oct 29, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

DennisOSRM commented Oct 30, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

DennisOSRM commented Oct 30, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

DennisOSRM commented Oct 30, 2022

Uh oh!

DennisOSRM commented Oct 30, 2022

Uh oh!

SiarheiFedartsou commented Oct 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants