Explicitly copy way.nodes() into vector to have proper table meta functions

[oxidase]

Issue

For #3452 added index, length and call metafunctions to WayNodeList, so it is possible to access node references in lua as

  local nodes = way:get_nodes()
  for index = 1, #nodes do
     print (nodes[index]:id())
  end

or

  for i,node in ipairs(way:get_nodes()()) do
     print (i, node:id())
  end

The return value of call is a table, so for node in way:get_nodes() do does not work. It should be checked what call should return to have an "iterable" object.

EDIT: the original commit defines explicitly meta functions for WayNodeList. Also __pairs and __ipairs functions can not be explicitly defined for a usertype. Just copying NodeRef to vector solves the issue by using implicitly container_usertype_metatable.

Tasklist

• add regression / cucumber cases (see docs/testing.md)
• review
• adjust for comments

[daniel-j-h]

This looks like a good workaround to let users still do what they want to.

Still we break profiles here by not providing a 100% replacement with Luabind .. hrm.

[oxidase]

I guess to have the old behavior WayNodeList should return bogus IteratorX

        sol::meta_function::call,
        [](const osmium::WayNodeList &list) { return IteratorX{list.begin(), list.end()}; }

but i did not find how to make working. https://github.com/Project-OSRM/osrm-backend/blob/master/third_party/sol2/sol2/sol.hpp#L11334 is how it works with sol __pairs/__ipairs

[TheMarex]

Reading the issues around this, I think only supporting stateful iterators for Lua 5.2+ is fine. There is a workaround for old lua versions, and really we don't want to be slowed down by Lua 5.1 that much.

@oxidase can you rebase and merge?

[oxidase]

@TheMarex rebased. But just one note - the change will not allow for node in way:get_nodes() do with a stateful iterator. Possible are for _, node in ipairs(way:get_nodes()) do with a state-less iterator or for i = 1, #way:get_nodes() do with an index.

I simply would rely on sol2 implementation: if the lua table interface for std::vector will implement sol::meta_function::call than for node in way:get_nodes() do will work without changes in OSRM code.

[MoKob]

@daniel-j-h @oxidase from the conversation this looks like it is ready to merge, any objections here?

[oxidase]

@MoKob PR makes a copy of NodeRefList as a vector<NodeRef> that is not good. It should be checked first if a correct metatable for NodeRefList can be created automatically.

[MoKob]

As long as this check is pending, I am re-tagging this as work in progress to avoid confusion. Feel free to directly move to ready-to-merge when checks should turn out fine.

[oxidase]

updated PR to wrap a NodeRefList const reference by sol::as_table. So copying is avoided, but life time of way:get_nodes() returned value must be in scope of way.

Although the original issue #3452 is not completely resolved, because the returned WayNodeList value has sol table interface that supports only stateless iterators, i think PR can be merged now.

[oxidase]

updated PR due to windows build fail: decltype(way.nodes()) is resolved by sol to sol.osmium::WayNodeList * lua type that has no table metafunctions. With in-place lambda type is correctly resolved.