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

baldr: Adjusting code to C++17 #4011

Merged
merged 35 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
18db226
[admin] handling for loops and using std::array
cvvergara Mar 7, 2023
279cd8a
[admin] explicit attribute: fallthrough
cvvergara Mar 7, 2023
bf82af3
[connectivity_map] wraping unused code with if 0 & minor c++ improvem…
cvvergara Mar 7, 2023
8b08502
[datetime] using range loop
cvvergara Mar 7, 2023
66902ef
running the pre-commit
cvvergara Mar 7, 2023
c2f92c4
[connectivity_map]:broom: removing unused code
cvvergara Mar 7, 2023
025fc2c
[admin] adding missing header file
cvvergara Mar 7, 2023
1e8e1ee
[edgeinfo] using tuple for GetNamesAndTypes
cvvergara Mar 7, 2023
6272ed2
[edgeinfo] Reminder for doing tests on new function
cvvergara Mar 7, 2023
86bb2dc
[edgeinfo] fixing typo
cvvergara Mar 8, 2023
59ccd61
[odin/enhancedtrippath.cc] removing clang warnings
cvvergara Mar 9, 2023
0bd7583
[graphtile] removing clang warnings
cvvergara Mar 9, 2023
40ac860
[graphreadeer] removing clang warnings
cvvergara Mar 9, 2023
c002de5
[graphreader] using max value to indicate invalid
cvvergara Mar 9, 2023
11b5472
[thrid_party/date] added as a system include to avoid deprecated warning
cvvergara Mar 9, 2023
e183562
[enhancedtrippath] Compile static fucntions when needed for LOGGING_L…
cvvergara Mar 9, 2023
e747d34
[graphtileheader] fixing constructor TODO when tests pass delete unus…
cvvergara Mar 9, 2023
e20bede
[baldr] No more warnings on Clang or g++
cvvergara Mar 9, 2023
ff80ea7
[graphtileheader] fixing specified bound 16 equals destination size […
cvvergara Mar 9, 2023
2f3aa76
[graphtileheader] adding missing include file
cvvergara Mar 9, 2023
478b190
[graphtileheader] fixing specified bound 16 equals destination size […
cvvergara Mar 9, 2023
f2fb8ec
[graphtileheader] using std::array on version_ & removing unused code
cvvergara Mar 9, 2023
14f0eca
[graphtileheader] using std::array on empty_slots_
cvvergara Mar 9, 2023
39e6c24
[graphtileheader] using std::array on bin_offsets_
cvvergara Mar 9, 2023
91d8531
[baldr] Removing shrink_to_fit, adjusting some text
cvvergara Mar 15, 2023
50b2d71
[baldr] requested changes: documentation and static_cast
cvvergara Mar 17, 2023
d7be34d
[baldr] requested changes: static_cast & version.size
cvvergara Mar 17, 2023
efd24d1
[baldr] fixing typo
cvvergara Mar 17, 2023
083d8fd
Removing unused code and using sentinel
cvvergara Mar 19, 2023
55744a9
[build] Using the module for system includes
cvvergara Mar 22, 2023
f889c5d
[utils] adding more functionality around invalid & using
cvvergara Mar 22, 2023
81d45a5
[lint] running the pre-commit
cvvergara Mar 22, 2023
9b297e5
[edgeinfo] removing try/catch from original function and new function
cvvergara Apr 4, 2023
6a8c6ae
[edgeinfo] Adjusting docstrings
cvvergara Apr 5, 2023
e88d80e
docstring valhalla/baldr/edgeinfo.h
kevinkreiser Apr 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
* CHANGED: Updated url for just_gtfs library [#3994](https://github.com/valhalla/valhalla/pull/3995)
* ADDED: Docker image pushes to Github's docker registry [#4033](https://github.com/valhalla/valhalla/pull/4033)
* ADDED: `disable_hierarchy_pruning` costing option to find the actual optimal route for motorized costing modes, i.e `auto`, `motorcycle`, `motor_scooter`, `bus`, `truck` & `taxi`. [#4000](https://github.com/valhalla/valhalla/pull/4000)
* CHANGED: baldr directory: remove warnings and C++17 adjustments [#4011](https://github.com/valhalla/valhalla/pull/4011)

## Release Date: 2023-01-03 Valhalla 3.3.0
* **Removed**
Expand Down
9 changes: 6 additions & 3 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ endfunction()

function(valhalla_module)
set(oneValueArgs NAME)
set(multiValueArgs SOURCES SOURCES_WITH_WARNINGS HEADERS DEPENDS INCLUDE_DIRECTORIES)
set(multiValueArgs SOURCES SOURCES_WITH_WARNINGS HEADERS DEPENDS INCLUDE_DIRECTORIES SYSTEM_INCLUDE_DIRECTORIES)
cmake_parse_arguments(MODULE "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})

if (UNIX AND ENABLE_SINGLE_FILES_WERROR)
Expand Down Expand Up @@ -107,7 +107,10 @@ function(valhalla_module)
PRIVATE
${VALHALLA_BINARY_DIR}
${VALHALLA_BINARY_DIR}/valhalla)

if (MODULE_SYSTEM_INCLUDE_DIRECTORIES)
target_include_directories(${library} SYSTEM ${MODULE_SYSTEM_INCLUDE_DIRECTORIES})
endif()

target_link_libraries(${library} Boost::boost)

if(ENABLE_COVERAGE)
Expand Down Expand Up @@ -263,7 +266,7 @@ if(BUILD_SHARED_LIBS)
install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${LIBVALHALLA_SO_LINK}
DESTINATION "${CMAKE_INSTALL_LIBDIR}"
COMPONENT development)

if(UNIX)
add_custom_target(${LIBVALHALLA_SO_LINK} ALL
COMMAND ${CMAKE_COMMAND} -E create_symlink "${LIBVALHALLA_SO_LINK}.${VALHALLA_VERSION_MAJOR}" ${LIBVALHALLA_SO_LINK})
Expand Down
20 changes: 13 additions & 7 deletions src/baldr/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,15 @@ set(includes
${VALHALLA_SOURCE_DIR}/valhalla
$<$<BOOL:${WIN32}>:${VALHALLA_SOURCE_DIR}/third_party/dirent/include>
${VALHALLA_SOURCE_DIR}/third_party/rapidjson/include
${VALHALLA_SOURCE_DIR}/third_party/date/include
${CMAKE_CURRENT_BINARY_DIR}/src/baldr
)

# treat date library as system
set(system_includes ${VALHALLA_SOURCE_DIR}/third_party/date/include)
if(APPLE)
list(APPEND system_includes ${VALHALLA_SOURCE_DIR}/third_party/date/include/date)
endif()

set(sources
accessrestriction.cc
admin.cc
Expand All @@ -45,6 +50,9 @@ set(sources
directededge.cc
edgeinfo.cc
graphid.cc
graphreader.cc
graphtile.cc
graphtileheader.cc
incident_singleton.h
edgetracker.cc
nodeinfo.cc
Expand All @@ -64,18 +72,14 @@ set(sources
transitroute.cc
transitschedule.cc
transittransfer.cc
tz_alt.cpp
laneconnectivity.cc
verbal_text_formatter.cc
verbal_text_formatter_us.cc
verbal_text_formatter_us_co.cc
verbal_text_formatter_us_tx.cc
verbal_text_formatter_factory.cc)

set (sources_with_warnings
graphreader.cc
graphtile.cc
graphtileheader.cc)

list(APPEND sources
#basic timezone stuff
${CMAKE_CURRENT_BINARY_DIR}/date_time_africa.h
Expand All @@ -97,7 +101,6 @@ list(APPEND sources_with_warnings

#ios we have more work to make use of system tzdb
if(APPLE)
list(APPEND includes ${VALHALLA_SOURCE_DIR}/third_party/date/include/date)
list(APPEND sources ${VALHALLA_SOURCE_DIR}/third_party/date/src/ios.mm)
endif()

Expand All @@ -114,6 +117,9 @@ valhalla_module(NAME baldr
${includes}
PRIVATE
${CMAKE_CURRENT_BINARY_DIR}
SYSTEM_INCLUDE_DIRECTORIES
PUBLIC
${system_includes}

DEPENDS
valhalla::midgard
Expand Down
45 changes: 17 additions & 28 deletions src/baldr/admin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,24 +65,24 @@ Admin::Admin(const uint32_t country_offset,
const std::string& state_iso)
: country_offset_(country_offset), state_offset_(state_offset) {

std::size_t length = 0;
// Example: GB or US
if (country_iso.size() == kCountryIso) {
length = country_iso.copy(country_iso_, kCountryIso);
std::copy(country_iso.begin(), country_iso.end(), country_iso_.begin());
} else {
country_iso_[0] = '\0';
}

// Example: PA
if (state_iso.size() == kStateIso - 1) {
length = state_iso.copy(state_iso_, kStateIso - 1);
state_iso_[length] = '\0';
}
// Example: WLS
else if (state_iso.size() == kStateIso) {
length = state_iso.copy(state_iso_, kStateIso);
} else {
state_iso_[0] = '\0';
switch (state_iso.size()) {
case kStateIso - 1:
// Example: PA
state_iso_[kStateIso - 1] = '\0';
[[fallthrough]];
case kStateIso:
// Example: WLS
std::copy(state_iso.begin(), state_iso.end(), state_iso_.begin());
break;
default:
state_iso_[0] = '\0';
}
}

Expand All @@ -98,26 +98,15 @@ uint32_t Admin::country_offset() const {

// country ISO3166-1
std::string Admin::country_iso() const {
std::string str;
for (size_t i = 0; i < kCountryIso; i++) {
if (country_iso_[i] == '\0') {
break;
}
str.append(1, country_iso_[i]);
}
return str;
return country_iso_[0] == '\0' ? std::string()
: std::string(country_iso_.begin(), country_iso_.end());
}

// country ISO + dash + state ISO will give you ISO3166-2 for state.
std::string Admin::state_iso() const {
std::string str;
for (size_t i = 0; i < kStateIso; i++) {
if (state_iso_[i] == '\0') {
break;
}
str.append(1, state_iso_[i]);
}
return str;
return state_iso_[0] == '\0'
? std::string()
: std::string(state_iso_.begin(), std::find(state_iso_.begin(), state_iso_.end(), '\0'));
}

} // namespace baldr
Expand Down
11 changes: 2 additions & 9 deletions src/baldr/connectivity_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,10 @@ json::MapPtr to_geometry(const polygon_t& polygon) {
for (const auto& ring : polygon) {
auto ring_coords = json::array({});
for (const auto& coord : ring) {
// if (outer) {
ring_coords->emplace_back(
json::array({json::fixed_t{coord.first, 6}, json::fixed_t{coord.second, 6}}));
/*} else {
ring_coords->emplace_front(
json::array({json::fixed_t{coord.first, 6}, json::fixed_t{coord.second, 6}}));
}*/
}
coords->emplace_back(ring_coords);
// outer = false;
}
return json::map({{"type", std::string("Polygon")}, {"coordinates", coords}});
}
Expand Down Expand Up @@ -189,7 +183,7 @@ std::unordered_set<size_t> connectivity_map_t::get_colors(uint32_t hierarchy_lev
AABB2<PointLL> bbox(Point2(ll.lng() - lngdeg, ll.lat() - latdeg),
Point2(ll.lng() + lngdeg, ll.lat() + latdeg));
std::vector<int32_t> tilelist = tiles.TileList(bbox);
for (auto& id : tilelist) {
for (const auto& id : tilelist) {
auto color = level->second.find(id);
if (color != level->second.cend()) {
result.emplace(color->second);
Expand Down Expand Up @@ -248,8 +242,7 @@ std::vector<size_t> connectivity_map_t::to_image(const uint32_t hierarchy_level)
}
const auto& level_tiles = TileHierarchy::levels()[tile_level];

std::vector<size_t> tiles(level_tiles.tiles.nrows() * level_tiles.tiles.ncolumns(),
static_cast<uint32_t>(0));
std::vector<size_t> tiles(level_tiles.tiles.nrows() * level_tiles.tiles.ncolumns(), 0);
auto level = colors.find(hierarchy_level);
if (level != colors.cend()) {
for (size_t i = 0; i < tiles.size(); ++i) {
Expand Down
5 changes: 3 additions & 2 deletions src/baldr/datetime.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ namespace DateTime {
tz_db_t::tz_db_t() : db(date::get_tzdb()) {
// NOTE: outside of this class 0 is reserved for invalid timezone
// so we offset each index by 1 to get into the valid range 1-300 or so
for (size_t i = 0; i < db.zones.size(); ++i) {
names.emplace(db.zones[i].name(), i + 1);
size_t idx{0};
cvvergara marked this conversation as resolved.
Show resolved Hide resolved
for (const auto& zone : db.zones) {
names.emplace(zone.name(), ++idx);
}
}

Expand Down
46 changes: 34 additions & 12 deletions src/baldr/edgeinfo.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,34 @@ std::vector<std::string> EdgeInfo::GetNames() const {
return names;
}

// Get a list of names tagged and/or untagged with tag status
std::vector<std::pair<std::string, bool>> EdgeInfo::GetNames(bool include_tagged_values) const {
// Get each name
std::vector<std::pair<std::string, bool>> name_type_pairs;
name_type_pairs.reserve(name_count());
const NameInfo* ni = name_info_list_;
for (uint32_t i = 0; i < name_count(); i++, ni++) {
// Skip any tagged names (FUTURE code may make use of them)
if (ni->tagged_ && !include_tagged_values) {
continue;
}
if (ni->tagged_) {
if (ni->name_offset_ < names_list_length_) {
std::string name = names_list_ + ni->name_offset_;
if (IsNameTag(name[0])) {
name_type_pairs.push_back({name.substr(1), false});
}
} else
throw std::runtime_error("GetNames: offset exceeds size of text list");
} else if (ni->name_offset_ < names_list_length_) {
name_type_pairs.push_back({names_list_ + ni->name_offset_, ni->is_route_num_});
} else {
throw std::runtime_error("GetNames: offset exceeds size of text list");
}
}
return name_type_pairs;
}

// Get a list of tagged names
std::vector<std::string> EdgeInfo::GetTaggedValues(bool only_pronunciations) const {
// Get each name
Expand Down Expand Up @@ -138,11 +166,11 @@ std::vector<std::string> EdgeInfo::GetTaggedValues(bool only_pronunciations) con
}

// Get a list of names
std::vector<std::pair<std::string, bool>>
EdgeInfo::GetNamesAndTypes(std::vector<uint8_t>& types, bool include_tagged_values) const {
std::vector<std::tuple<std::string, bool, uint8_t>>
EdgeInfo::GetNamesAndTypes(bool include_tagged_values) const {

// Get each name
std::vector<std::pair<std::string, bool>> name_type_pairs;
std::vector<std::tuple<std::string, bool, uint8_t>> name_type_pairs;
name_type_pairs.reserve(name_count());
const NameInfo* ni = name_info_list_;
for (uint32_t i = 0; i < name_count(); i++, ni++) {
Expand All @@ -153,19 +181,13 @@ EdgeInfo::GetNamesAndTypes(std::vector<uint8_t>& types, bool include_tagged_valu
if (ni->tagged_) {
if (ni->name_offset_ < names_list_length_) {
std::string name = names_list_ + ni->name_offset_;
try {
if (IsNameTag(name[0])) {
name_type_pairs.push_back({name.substr(1), false});
types.push_back(static_cast<uint8_t>(name.at(0)));
}
} catch (const std::invalid_argument& arg) {
LOG_DEBUG("invalid_argument thrown for name: " + name);
if (IsNameTag(name[0])) {
name_type_pairs.push_back({name.substr(1), false, static_cast<uint8_t>(name.at(0))});
}
} else
throw std::runtime_error("GetNamesAndTypes: offset exceeds size of text list");
} else if (ni->name_offset_ < names_list_length_) {
name_type_pairs.push_back({names_list_ + ni->name_offset_, ni->is_route_num_});
types.push_back(0);
name_type_pairs.push_back({names_list_ + ni->name_offset_, ni->is_route_num_, 0});
} else {
throw std::runtime_error("GetNamesAndTypes: offset exceeds size of text list");
}
Expand Down
6 changes: 3 additions & 3 deletions src/baldr/graphreader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ void FlatTileCache::Reserve(size_t tile_size) {

// Checks if tile exists in the cache.
bool FlatTileCache::Contains(const GraphId& graphid) const {
return get_index(graphid) != -1;
return is_valid(get_index(graphid));
}

// Lets you know if the cache is too large.
Expand All @@ -206,7 +206,7 @@ void FlatTileCache::Clear() {
// Get a pointer to a graph tile object given a GraphId.
graph_tile_ptr FlatTileCache::Get(const GraphId& graphid) const {
auto index = get_index(graphid);
if (index == -1)
if (is_invalid(index))
return nullptr;
return cache_[index];
}
Expand Down Expand Up @@ -1014,7 +1014,7 @@ IncidentResult GraphReader::GetIncidents(const GraphId& edge_id, graph_tile_ptr&
const valhalla::IncidentsTile::Metadata&
getIncidentMetadata(const std::shared_ptr<const valhalla::IncidentsTile>& tile,
const valhalla::IncidentsTile::Location& incident_location) {
const auto metadata_index = incident_location.metadata_index();
const int64_t metadata_index = incident_location.metadata_index();
if (metadata_index >= tile->metadata_size()) {
throw std::runtime_error(std::string("Invalid incident tile with an incident_index of ") +
std::to_string(metadata_index) + " but total incident metadata of " +
Expand Down
Loading