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

Migrate to C++17. Update sol2 to 3.3.0. Fix bug with reading Set values from Lua scripts. #6279

Merged
merged 11 commits into from
Jul 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- Misc:
- FIXED: Fix bug with reading Set values from Lua scripts. [#6285](https://github.com/Project-OSRM/osrm-backend/pull/6285)
- Build:
- CHANGED: Migrate to C++17. Update sol2 to 3.3.0. Fix bug with reading Set values from Lua scripts. [#6279](https://github.com/Project-OSRM/osrm-backend/pull/6279)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I maybe extract bug fix into separate PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to do so. It's not clear to me how https://github.com/Project-OSRM/osrm-backend/pull/6279/files#r920390451 is related to the bug, so would be good to separate it from the other changes to understand.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjjbell please see #6285

- CHANGED: Update macOS CI image to macos-11. [#6286](https://github.com/Project-OSRM/osrm-backend/pull/6286)
- CHANGED: Enable even more clang-tidy checks. [#6273](https://github.com/Project-OSRM/osrm-backend/pull/6273)
- CHANGED: Configure CMake to not build flatbuffers tests and samples. [#6274](https://github.com/Project-OSRM/osrm-backend/pull/6274)
Expand Down
9 changes: 6 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
cmake_minimum_required(VERSION 3.2)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED TRUE)

if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_CURRENT_BINARY_DIR AND NOT MSVC_IDE)
message(FATAL_ERROR "In-source builds are not allowed.
Please create a directory and run cmake from there, passing the path to this source directory as the last argument.
Expand Down Expand Up @@ -145,7 +148,7 @@ endif()

include_directories(BEFORE ${CMAKE_CURRENT_BINARY_DIR}/include/)
include_directories(BEFORE ${CMAKE_CURRENT_SOURCE_DIR}/include/)
include_directories(SYSTEM ${CMAKE_CURRENT_SOURCE_DIR}/third_party/sol2/)
include_directories(SYSTEM ${CMAKE_CURRENT_SOURCE_DIR}/third_party/sol2-3.3.0/include)
include_directories(SYSTEM ${CMAKE_CURRENT_SOURCE_DIR}/third_party/variant/include)

set(BOOST_COMPONENTS date_time chrono filesystem iostreams program_options regex system thread unit_test_framework)
Expand Down Expand Up @@ -413,8 +416,8 @@ set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} ${LINKER_FLAGS}")

# Activate C++1y
if(NOT CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14")
set(OSRM_CXXFLAGS "${OSRM_CXXFLAGS} -std=c++14")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17")
set(OSRM_CXXFLAGS "${OSRM_CXXFLAGS} -std=c++17")
endif()

# Configuring other platform dependencies
Expand Down
2 changes: 1 addition & 1 deletion include/extractor/scripting_environment_lua.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include <mutex>
#include <string>

#include <sol.hpp>
#include <sol/sol.hpp>

namespace osrm
{
Expand Down
32 changes: 12 additions & 20 deletions src/extractor/scripting_environment_lua.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -965,14 +965,10 @@ Sol2ScriptingEnvironment::GetStringListFromTable(const std::string &table_name)
auto &context = GetSol2Context();
BOOST_ASSERT(context.state.lua_state() != nullptr);
std::vector<std::string> strings;
if (!context.profile_table[table_name])
sol::optional<sol::table> table = context.profile_table[table_name];
if (table && table->valid())
{
return strings;
}
sol::table table = context.profile_table[table_name];
if (table.valid())
{
for (auto &&pair : table)
for (auto &&pair : *table)
{
strings.emplace_back(GetSetOrSequenceValue(pair));
}
Expand All @@ -987,17 +983,13 @@ Sol2ScriptingEnvironment::GetStringListsFromTable(const std::string &table_name)

auto &context = GetSol2Context();
BOOST_ASSERT(context.state.lua_state() != nullptr);
if (!context.profile_table[table_name])
{
return string_lists;
}
sol::table table = context.profile_table[table_name];
if (!table.valid())
sol::optional<sol::table> table = context.profile_table[table_name];
if (!table || !table->valid())
{
return string_lists;
}

for (const auto &pair : table)
for (const auto &pair : *table)
{
sol::table inner_table = pair.second;
if (!inner_table.valid())
Expand Down Expand Up @@ -1191,14 +1183,14 @@ void LuaScriptingContext::ProcessNode(const osmium::Node &node,
{
case 4:
case 3:
node_function(profile_table, node, result, relations);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise it complained that Node has no copy constructor and it cannot copy it.

node_function(profile_table, std::cref(node), result, relations);
break;
case 2:
node_function(profile_table, node, result);
node_function(profile_table, std::cref(node), result);
break;
case 1:
case 0:
node_function(node, result);
node_function(std::cref(node), result);
break;
}
}
Expand All @@ -1213,14 +1205,14 @@ void LuaScriptingContext::ProcessWay(const osmium::Way &way,
{
case 4:
case 3:
way_function(profile_table, way, result, relations);
way_function(profile_table, std::cref(way), std::ref(result), std::cref(relations));
break;
case 2:
way_function(profile_table, way, result);
way_function(profile_table, std::cref(way), std::ref(result));
break;
case 1:
case 0:
way_function(way, result);
way_function(std::cref(way), std::ref(result));
break;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// sol2

// The MIT License (MIT)

// Copyright (c) 2013-2018 Rapptz, ThePhD and contributors
// Copyright (c) 2013-2020 Rapptz, ThePhD and contributors

// Permission is hereby granted, free of charge, to any person obtaining a copy of
// this software and associated documentation files (the "Software"), to deal in
Expand All @@ -21,34 +19,35 @@
// IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
// CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

#ifndef SOL_PROTECT_HPP
#define SOL_PROTECT_HPP
// This file was generated with a script.
// Generated 2022-06-25 08:14:19.336233 UTC
// This header was generated with sol v3.3.0 (revision eba86625)
// https://github.com/ThePhD/sol2

#ifndef SOL_SINGLE_CONFIG_HPP
#define SOL_SINGLE_CONFIG_HPP

// beginning of sol/config.hpp

/* Base, empty configuration file!

#include "traits.hpp"
#include <utility>
To override, place a file in your include paths of the form:

namespace sol {
. (your include path here)
| sol (directory, or equivalent)
| config.hpp (your config.hpp file)

template <typename T>
struct protect_t {
T value;
So that when sol2 includes the file

template <typename Arg, typename... Args, meta::disable<std::is_same<protect_t, meta::unqualified_t<Arg>>> = meta::enabler>
protect_t(Arg&& arg, Args&&... args)
: value(std::forward<Arg>(arg), std::forward<Args>(args)...) {
}
#include <sol/config.hpp>

protect_t(const protect_t&) = default;
protect_t(protect_t&&) = default;
protect_t& operator=(const protect_t&) = default;
protect_t& operator=(protect_t&&) = default;
};
it gives you the configuration values you desire. Configuration values can be
seen in the safety.rst of the doc/src, or at
https://sol2.readthedocs.io/en/latest/safety.html ! You can also pass them through
the build system, or the command line options of your compiler.

template <typename T>
auto protect(T&& value) {
return protect_t<std::decay_t<T>>(std::forward<T>(value));
}
*/

} // namespace sol
// end of sol/config.hpp

#endif // SOL_PROTECT_HPP
#endif // SOL_SINGLE_CONFIG_HPP
Loading