restore compatibility with gcc 6.3.0, libprotobuf 3.0.0, boost v1.62.0 #2953
Conversation
| // corrects geometry and handedness as expected by bg for rings | ||
| bg::correct(new_ring); |
There was a problem hiding this comment.
correct() is not supporting geographic CS at lower versions it seems. but actually, let me look at this again in bit more detail.. it corrects a bunch of things, but the handedness check is probably most useful. we register a valhalla type as ring via a macro, where I guess the expected handedness is clockwise (the default for builtin ring type) and all tests are on clockwise rings as well. I'm not sure what'll happen if I add a counterclockwise test, so I'll do that real quick
There was a problem hiding this comment.
ok, seems to be fine.. I altered the avoids test to also test a counterclockwise ring and still passes
| @@ -424,7 +424,8 @@ void thor_worker_t::path_arrive_by(Api& api, const std::string& costing) { | |||
|
|
|||
| auto route_two_locations = [&, this](auto& origin, auto& destination) -> bool { | |||
| // Get the algorithm type for this location pair | |||
| thor::PathAlgorithm* path_algorithm = get_path_algorithm(costing, *origin, *destination, options); | |||
| thor::PathAlgorithm* path_algorithm = | |||
| this->get_path_algorithm(costing, *origin, *destination, options); | |||
There was a problem hiding this comment.
can you share the compiler messages that are making you use this syntax, is it something about being ambiguous? we dont use this to access members anywhere else in the code base that im aware of so itd be sad to start doing that stylistically if we can instead just rename some other methods that the compiler is complaining about?
There was a problem hiding this comment.
It was smth along the lines of Missing object on those lines where this-> was missing. It's not a general access member thing, rather smth to do with capturing this inside the lambda. I can reproduce tmrw and paste the trace if you want.
There was a problem hiding this comment.
here it is:
/valhalla/src/thor/route_action.cc: In instantiation of ‘valhalla::thor::thor_worker_t::path_arrive_by(valhalla::Api&, const string&)::<lambda(auto:2&, auto:3&)> [with auto:2 = std::reverse_iterator<google::protobuf::internal::RepeatedPtrIterator<valhalla::Location> >; auto:3 = std::reverse_iterator<google::protobuf::internal::RepeatedPtrIterator<valhalla::Location> >]’:
/valhalla/src/thor/route_action.cc:518:49: required from here
/valhalla/src/thor/route_action.cc:441:87: error: cannot call member function ‘std::vector<std::vector<valhalla::thor::PathInfo> > valhalla::thor::thor_worker_t::get_path(valhalla::thor::PathAlgorithm*, valhalla::Location&, valhalla::Location&, const string&, const valhalla::Options&)’ without object
auto temp_paths = get_path(path_algorithm, *origin, *destination, costing, options);
^
/valhalla/src/thor/route_action.cc:445:5: error: unable to deduce ‘auto&&’ from ‘temp_paths’
for (auto& temp_path : temp_paths) {
^~~There was a problem hiding this comment.
In a quick search I couldn't find any other similar error report, so I'd blame the old GCC?! In any case, it seems capturing [&, this] is redundant and could be simply [&], which compiles both without the above fix on a recent GCC (v10.2) and with the above fix on v6.3.0.
There was a problem hiding this comment.
ah man that does suck, very unfortunate old compiler bug
There was a problem hiding this comment.
jeez, you're up early on a sat ;)
| @@ -1582,7 +1582,7 @@ summarize_route_legs(const google::protobuf::RepeatedPtrField<DirectionsRoute>& | |||
| // unique the same leg (leg_idx) between all routes. | |||
| for (size_t route_i = 0; route_i < routes.size(); route_i++) { | |||
|
|
|||
| size_t num_legs_i = routes[route_i].legs_size(); | |||
| size_t num_legs_i = routes.Get(route_i).legs_size(); | |||
| std::vector<ring_bg_t> rings; | ||
| rings.push_back({{node_a.lng() + 0.1 * dx, node_a.lat() - 0.01 * dy}, | ||
| {node_a.lng() + 0.1 * dx, node_a.lat() - 0.1 * dy}, | ||
| {node_a.lng() - 0.1 * dx, node_a.lat() - 0.1 * dy}, | ||
| {node_a.lng() - 0.1 * dx, node_a.lat() - 0.01 * dy}, | ||
| {node_a.lng() + 0.1 * dx, node_a.lat() - 0.01 * dy}}); | ||
|
|
||
| // one counterclockwise ring |
|
no need to merge master and wait for another round of CI changes are localized |
…halla into pedestrian_crossing * 'pedestrian_crossing' of https://github.com/abhi2039/valhalla: build: Bail with error if non-existant build-type specified (valhalla#2965) nit: Enables compiler warnings in part of mjolnir module (valhalla#2922) fix missing comma scripts/valhalla_build_config (valhalla#2963) Remove duplicate names being used across different gurka tests (valhalla#2962) Dont abort bidirectional a* search if only one direction is exhausted (valhalla#2936) penalize uturns at pencil points and short internal edges (valhalla#2944) Remove unused IsNextEdgeInternal function (valhalla#2958) restore compatibility with gcc 6.3.0, libprotobuf 3.0.0, boost v1.62.0 (valhalla#2953) Add ability to build Valhalla modules as STATIC libraries (valhalla#2957) Allow disabling Werror (valhalla#2937) Enhanced logic for IsTurnChannelManeuverCombinable (valhalla#2952) Allow config object to be passed-in to path algorithms (valhalla#2949) Floating-point precision fix in meili::Project() fixes map-match issue (valhalla#2946) Use kServiceRoad edges while reclassifying ferry connections (valhalla#2933) ci: Update cache key to trigger clearing it (valhalla#2947)
I wanted to build python packages on debian 9 (for compatibility with PEP600), which has a bunch of fairly old versions.. Note, the build was with
-DENABLE_SERVICES=OFF -DENABLE_TESTS=OFF -DENABLE_BENCHMARKS=OFF..I would've built at least with SERVICES=ON, but I couldn't get prime_server to be discovered by CMake (v3.18.3), which could be an issue with
pkg-configwhich is onv0.29on debian 9..