Switch to reliable non-existent handle for QueryHeap items#4396
Switch to reliable non-existent handle for QueryHeap items#4396TheMarex merged 1 commit intoProject-OSRM:masterfrom mloskot:ml/queryheap-none-handle
Conversation
daniel-j-h
left a comment
daniel-j-h
left a comment
There was a problem hiding this comment.
Urgh thanks for chasing this one down.
Could you add your findings, this ticket, and the boost ml / stackoverflow links as comment to the code please.
TheMarex
left a comment
TheMarex
left a comment
There was a problem hiding this comment.
Urg this is an ugly one. Good catch. 👍 Could you run ./scripts/format.sh? Our formatting check on CI is complaining, otherwise this is good to merge.
include/util/query_heap.hpp
Outdated
| BOOST_ASSERT(WasInserted(node)); | ||
| const Key index = node_index.peek_index(node); | ||
| return inserted_nodes[index].handle == HeapHandle{}; | ||
| auto const end_it = const_cast<HeapContainer&>(heap).end(); // non-const iterator |
There was a problem hiding this comment.
interestingly this method is used only in DecreaseKey in assertion.
I guess it can be removed completely,
and in DecreaseKey added a line
BOOST_ASSERT(reference.handle != HeapContainer::s_handle_from_iterator(heap.end())); // was not removed
There was a problem hiding this comment.
@daniel-j-h & @TheMarex shall I update the PR as you suggest or simply remove the method completely as @oxidase suggests?
There was a problem hiding this comment.
@mloskot I would like to keep the interface change out of this PR. :-)
|
I'll be more than happy to improve the PR but after I'm back from short trip in a week or two. No access to any compiler enabled computer ATM. |
|
while waiting for response to my question in #4396 (comment), I did apply:
PR updated with both changes. |
Default-constructed objects of (boost::heap) handle_type are singular, including the wrapped handle_type::iterator. Apparently, MSVC iterator debug facilities strictly require that one singular instance is compared to another singular instance. It is not possible to get check-comparabe iterators of non-singular and singular instances as owning container will always mismatch.
|
@mloskot looks good to me, I restarted the travis build. I think the failure was unrelated. 👍 |
|
@TheMarex Thanks |
4 participants
Issue
Default-constructed objects of (
boost::heap)handle_typeare singular, including the wrappedhandle_type::iterator.Apparently, MSVC iterator debug facilities strictly require that one singular instance is compared to another singular instance.
It is not possible to get check-comparabe iterators of non-singular and singular instances as owning container will always mismatch.
Initially investigated at Boost ML (https://lists.boost.org/boost-users/2017/08/87787.php), eventually received confirmation via SO (https://stackoverflow.com/a/45622940/151641)
Tasklist