The Big Cleanup II

CMakeLists.txt

@@ -163,7 +163,6 @@ elseif(${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU")
   # using GCC
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -pedantic -Wuninitialized -Wunreachable-code -Wstrict-overflow=1 -D_FORTIFY_SOURCE=2 ${COLOR_FLAG} -fPIC")
   if(WIN32) # using mingw
-    add_definitions(-D_USE_MATH_DEFINES) # define M_PI, M_1_PI etc.
     add_definitions(-DWIN32)
     set(OPTIONAL_SOCKET_LIBS ws2_32 wsock32)
   endif()
@@ -175,7 +174,6 @@ elseif(${CMAKE_CXX_COMPILER_ID} STREQUAL "MSVC")
   set(BOOST_COMPONENTS ${BOOST_COMPONENTS} date_time chrono zlib)
   add_definitions(-D_CRT_SECURE_NO_WARNINGS)
   add_definitions(-DNOMINMAX) # avoid min and max macros that can break compilation
-  add_definitions(-D_USE_MATH_DEFINES) # define M_PI
   add_definitions(-D_WIN32_WINNT=0x0501)
   add_definitions(-DXML_STATIC)
   find_library(ws2_32_LIBRARY_PATH ws2_32)

include/engine/api_response_generator.hpp

@@ -277,10 +277,10 @@ ApiResponseGenerator<DataFacadeT>::BuildHintData(const InternalRouteResult &raw_
     std::string hint;
     for (const auto i : util::irange<std::size_t>(0, raw_route.segment_end_coordinates.size()))
     {
-        ObjectEncoder::EncodeToBase64(raw_route.segment_end_coordinates[i].source_phantom, hint);
-        json_location_hint_array.values.push_back(hint);
+        hint = encodeBase64(raw_route.segment_end_coordinates[i].source_phantom);
+        json_location_hint_array.values.push_back(std::move(hint));
     }
-    ObjectEncoder::EncodeToBase64(raw_route.segment_end_coordinates.back().target_phantom, hint);
+    hint = encodeBase64(raw_route.segment_end_coordinates.back().target_phantom);
     json_location_hint_array.values.emplace_back(std::move(hint));
     json_hint_object.values["locations"] = json_location_hint_array;
 

include/engine/datafacade/datafacade_base.hpp

@@ -8,7 +8,7 @@
 #include "engine/phantom_node.hpp"
 #include "extractor/turn_instructions.hpp"
 #include "util/integer_range.hpp"
-#include "util/osrm_exception.hpp"
+#include "util/exception.hpp"
 #include "util/string_util.hpp"
 #include "util/typedefs.hpp"
 

include/engine/map_matching/bayes_classifier.hpp

@@ -6,6 +6,8 @@
 #include <vector>
 #include <utility>
 
+#include <boost/math/constants/constants.hpp>
+
 namespace osrm
 {
 namespace engine
@@ -23,8 +25,10 @@ struct NormalDistribution
     // FIXME implement log-probability version since its faster
     double density_function(const double val) const
     {
+        using namespace boost::math::constants;
+
         const double x = val - mean;
-        return 1.0 / (std::sqrt(2. * M_PI) * standard_deviation) *
+        return 1.0 / (std::sqrt(two_pi<double>()) * standard_deviation) *
                std::exp(-x * x / (standard_deviation * standard_deviation));
     }
 

include/engine/map_matching/hidden_markov_model.hpp

@@ -4,6 +4,7 @@
 #include "util/integer_range.hpp"
 
 #include <boost/assert.hpp>
+#include <boost/math/constants/constants.hpp>
 
 #include <cmath>
 
@@ -17,7 +18,7 @@ namespace engine
 namespace map_matching
 {
 
-static const double log_2_pi = std::log(2. * M_PI);
+static const double log_2_pi = std::log(2. * boost::math::constants::pi<double>());
 static const double IMPOSSIBLE_LOG_PROB = -std::numeric_limits<double>::infinity();
 static const double MINIMAL_LOG_PROB = std::numeric_limits<double>::lowest();
 static const std::size_t INVALID_STATE = std::numeric_limits<std::size_t>::max();

include/engine/object_encoder.hpp

@@ -11,59 +11,81 @@
 #include <string>
 #include <vector>
 
+#include <cstdint>
+#include <climits>
+
 namespace osrm
 {
 namespace engine
 {
 
-struct ObjectEncoder
+namespace detail
 {
-    using base64_t = boost::archive::iterators::base64_from_binary<
-        boost::archive::iterators::transform_width<const char *, 6, 8>>;
+static_assert(CHAR_BIT == 8u, "we assume a byte holds 8 bits");
+static_assert(sizeof(char) == 1u, "we assume a char is one byte large");
 
-    using binary_t = boost::archive::iterators::transform_width<
-        boost::archive::iterators::binary_from_base64<std::string::const_iterator>,
-        8,
-        6>;
+using Base64FromBinary = boost::archive::iterators::base64_from_binary<
+    boost::archive::iterators::transform_width<const char *, // sequence of chars
+                                               6,            // get view of 6 bit
+                                               8             // from sequence of 8 bit
+                                               >>;
 
-    template <class ObjectT> static void EncodeToBase64(const ObjectT &object, std::string &encoded)
-    {
-        const char *char_ptr_to_object = reinterpret_cast<const char *>(&object);
-        std::vector<unsigned char> data(sizeof(object));
-        std::copy(char_ptr_to_object, char_ptr_to_object + sizeof(ObjectT), data.begin());
-
-        unsigned char number_of_padded_chars = 0; // is in {0,1,2};
-        while (data.size() % 3 != 0)
-        {
-            ++number_of_padded_chars;
-            data.push_back(0x00);
-        }
-
-        BOOST_ASSERT_MSG(0 == data.size() % 3, "base64 input data size is not a multiple of 3!");
-        encoded.resize(sizeof(ObjectT));
-        encoded.assign(base64_t(&data[0]),
-                       base64_t(&data[0] + (data.size() - number_of_padded_chars)));
-        std::replace(begin(encoded), end(encoded), '+', '-');
-        std::replace(begin(encoded), end(encoded), '/', '_');
-    }
+using BinaryFromBase64 = boost::archive::iterators::transform_width<
+    boost::archive::iterators::binary_from_base64<std::string::const_iterator>,
+    8, // get a view of 8 bit
+    6  // from a sequence of 6 bit
+    >;
+} // ns detail
+
+template <typename T> std::string encodeBase64(const T &x)
+{
+#if not defined __GNUC__ or __GNUC__ > 4
+    static_assert(std::is_trivially_copyable<T>::value, "requires a trivially copyable type");
+#endif
 
-    template <class ObjectT> static void DecodeFromBase64(const std::string &input, ObjectT &object)
+    std::vector<unsigned char> bytes{reinterpret_cast<const char *>(&x),
+                                     reinterpret_cast<const char *>(&x) + sizeof(T)};
+    BOOST_ASSERT(!bytes.empty());
+
+    std::size_t bytes_to_pad{0};
+
+    while (bytes.size() % 3 != 0)
     {
-        try
-        {
-            std::string encoded(input);
-            std::replace(begin(encoded), end(encoded), '-', '+');
-            std::replace(begin(encoded), end(encoded), '_', '/');
-
-            std::copy(binary_t(encoded.begin()), binary_t(encoded.begin() + encoded.length()),
-                      reinterpret_cast<char *>(&object));
-        }
-        catch (...)
-        {
-        }
+        bytes_to_pad += 1;
+        bytes.push_back(0);
     }
-};
+
+    BOOST_ASSERT(bytes_to_pad == 0 || bytes_to_pad == 1 || bytes_to_pad == 2);
+    BOOST_ASSERT_MSG(0 == bytes.size() % 3, "base64 input data size is not a multiple of 3");
+
+    std::string encoded{detail::Base64FromBinary{bytes.data()},
+                        detail::Base64FromBinary{bytes.data() + (bytes.size() - bytes_to_pad)}};
+
+    std::replace(begin(encoded), end(encoded), '+', '-');
+    std::replace(begin(encoded), end(encoded), '/', '_');
+
+    return encoded;
 }
+
+template <typename T> T decodeBase64(std::string encoded)
+{
+#if not defined __GNUC__ or __GNUC__ > 4
+    static_assert(std::is_trivially_copyable<T>::value, "requires a trivially copyable type");
+#endif
+
+    std::replace(begin(encoded), end(encoded), '-', '+');
+    std::replace(begin(encoded), end(encoded), '_', '/');
+
+    T rv;
+
+    std::copy(detail::BinaryFromBase64{begin(encoded)},
+              detail::BinaryFromBase64{begin(encoded) + encoded.length()},
+              reinterpret_cast<char *>(&rv));
+
+    return rv;
 }
 
+} // ns engine
+} // ns osrm
+
 #endif /* OBJECT_ENCODER_HPP */

include/engine/plugins/distance_table.hpp

@@ -94,8 +94,7 @@ template <class DataFacadeT> class DistanceTablePlugin final : public BasePlugin
             if (checksum_OK && i < route_parameters.hints.size() &&
                 !route_parameters.hints[i].empty())
             {
-                PhantomNode current_phantom_node;
-                ObjectEncoder::DecodeFromBase64(route_parameters.hints[i], current_phantom_node);
+                auto current_phantom_node = decodeBase64<PhantomNode>(route_parameters.hints[i]);
                 if (current_phantom_node.IsValid(facade->GetNumberOfNodes()))
                 {
                     if (route_parameters.is_source[i])

include/engine/plugins/trip.hpp

@@ -64,8 +64,7 @@ template <class DataFacadeT> class RoundTripPlugin final : public BasePlugin
             if (checksum_OK && i < route_parameters.hints.size() &&
                 !route_parameters.hints[i].empty())
             {
-                PhantomNode current_phantom_node;
-                ObjectEncoder::DecodeFromBase64(route_parameters.hints[i], current_phantom_node);
+                auto current_phantom_node = decodeBase64<PhantomNode>(route_parameters.hints[i]);
                 if (current_phantom_node.IsValid(facade->GetNumberOfNodes()))
                 {
                     phantom_node_list.push_back(std::move(current_phantom_node));
@@ -317,7 +316,6 @@ template <class DataFacadeT> class RoundTripPlugin final : public BasePlugin
                 //     }
                 //     return s;
                 // }();
-
             }
             else
             {

include/engine/plugins/viaroute.hpp

@@ -83,8 +83,8 @@ template <class DataFacadeT> class ViaRoutePlugin final : public BasePlugin
             if (checksum_OK && i < route_parameters.hints.size() &&
                 !route_parameters.hints[i].empty())
             {
-                ObjectEncoder::DecodeFromBase64(route_parameters.hints[i],
-                                                phantom_node_pair_list[i].first);
+                phantom_node_pair_list[i].first =
+                    decodeBase64<PhantomNode>(route_parameters.hints[i]);
                 if (phantom_node_pair_list[i].first.IsValid(facade->GetNumberOfNodes()))
                 {
                     continue;
@@ -111,8 +111,8 @@ template <class DataFacadeT> class ViaRoutePlugin final : public BasePlugin
         auto snapped_phantoms = snapPhantomNodes(phantom_node_pair_list);
 
         InternalRouteResult raw_route;
-        auto build_phantom_pairs = [&raw_route](const PhantomNode &first_node,
-                                                const PhantomNode &second_node)
+        auto build_phantom_pairs =
+            [&raw_route](const PhantomNode &first_node, const PhantomNode &second_node)
         {
             raw_route.segment_end_coordinates.push_back(PhantomNodes{first_node, second_node});
         };

include/extractor/raster_source.hpp

@@ -1,7 +1,7 @@
 #ifndef RASTER_SOURCE_HPP
 #define RASTER_SOURCE_HPP
 
-#include "util/osrm_exception.hpp"
+#include "util/exception.hpp"
 
 #include <boost/filesystem.hpp>
 #include <boost/filesystem/fstream.hpp>

include/extractor/travel_mode.hpp

@@ -6,6 +6,8 @@ namespace osrm
 namespace extractor
 {
 
+// This is a char instead of a typed enum, so that we can
+// pack it into e.g. a "TravelMode mode : 4" packed bitfield
 using TravelMode = unsigned char;
 }
 }

include/storage/shared_datatype.hpp

@@ -1,7 +1,7 @@
 #ifndef SHARED_DATA_TYPE_HPP
 #define SHARED_DATA_TYPE_HPP
 
-#include "util/osrm_exception.hpp"
+#include "util/exception.hpp"
 #include "util/simple_logger.hpp"
 
 #include <cstdint>

include/storage/shared_memory.hpp

@@ -1,7 +1,7 @@
 #ifndef SHARED_MEMORY_HPP
 #define SHARED_MEMORY_HPP
 
-#include "util/osrm_exception.hpp"
+#include "util/exception.hpp"
 #include "util/simple_logger.hpp"
 
 #include <boost/filesystem.hpp>

include/util/coordinate_calculation.hpp

@@ -62,8 +62,14 @@ double bearing(const FixedPointCoordinate first_coordinate,
 double computeAngle(const FixedPointCoordinate first,
                     const FixedPointCoordinate second,
                     const FixedPointCoordinate third);
-}
-}
-}
+
+namespace mercator
+{
+double yToLat(const double value);
+double latToY(const double latitude);
+} // ns mercator
+} // ns coordinate_calculation
+} // ns util
+} // ns osrm
 
 #endif // COORDINATE_CALCULATION

include/util/fingerprint_impl.hpp.in

@@ -1,4 +1,4 @@
-#include "util/osrm_exception.hpp"
+#include "util/exception.hpp"
 
 #include <boost/uuid/name_generator.hpp>
 #include <boost/uuid/uuid_generators.hpp>

include/util/graph_loader.hpp

@@ -2,7 +2,7 @@
 #define GRAPH_LOADER_HPP
 
 #include "util/fingerprint.hpp"
-#include "util/osrm_exception.hpp"
+#include "util/exception.hpp"
 #include "util/simple_logger.hpp"
 #include "extractor/external_memory_node.hpp"
 #include "extractor/node_based_edge.hpp"

include/util/integer_range.hpp

@@ -12,6 +12,11 @@ namespace osrm
 namespace util
 {
 
+// Warning: do not try to replace this with Boost's irange, as it is broken on Boost 1.55:
+//     auto r = boost::irange<unsigned int>(0, 15);
+//     std::cout << r.size() << std::endl;
+// results in -4294967281. Latest Boost versions fix this, but we still support older ones.
+
 template <typename Integer> class range
 {
   private:

include/util/json_container.hpp

@@ -33,9 +33,9 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <variant/variant.hpp>
 
-#include <iostream>
 #include <vector>
 #include <string>
+#include <utility>
 #include <unordered_map>
 
 namespace osrm
@@ -52,16 +52,16 @@ struct Array;
 
 struct String
 {
-    String() {}
-    String(const char *value) : value(value) {}
-    String(std::string value) : value(std::move(value)) {}
+    String() = default;
+    String(const char *value_) : value{value_} {}
+    String(std::string value_) : value{std::move(value_)} {}
     std::string value;
 };
 
 struct Number
 {
-    Number() {}
-    Number(double value) : value(static_cast<double>(value)) {}
+    Number() = default;
+    Number(double value_) : value{value_} {}
     double value;
 };
 

include/util/json_renderer.hpp

@@ -9,6 +9,11 @@
 
 #include "osrm/json_container.hpp"
 
+#include <ostream>
+#include <vector>
+#include <iterator>
+#include <string>
+
 namespace osrm
 {
 namespace util