Takes care of proper special member generation globally, fixes #1689

Phew, a lot of classes were affected by this. The rationale for the
changes are as follows:

- When a type X declares any constructor, the default constructor is
  not declared, so there is no need for X() = delete there. In fact,
  there is brutal difference between those two: deleted members
  participate in overload resolution, but not-declared members do not!

- When a type X wants to be non-copyable (e.g. to be only movable, like
  threads, unique_ptrs, and so on), you can either do it by inheriting
  from boost::noncopyable (the old way), or better declare both (!) the
  copy constructor _and_ the copy assignment operator as deleted:

      X(X const&) = delete;
      X& operator=(X const&) = delete;

  We had tons of types with deleted copy constructors that were lacking
  a corresponding deleted copy assignment operator, making them still
  copyable and you wouldn't even notice (read: scary)!

References:

- http://accu.org/content/conf2014/Howard_Hinnant_Accu_2014.pdf
- http://www.boost.org/doc/libs/master/libs/core/doc/html/core/noncopyable.html

Note: I know, I'm quoting Hinnant's extraordinary slides a lot, but
getting the sematic right here is so incredibly important.
This commit is contained in:
Daniel J. Hofmann 2016-01-27 11:20:55 +01:00
parent d189339495
commit a48f02e0da
12 changed files with 55 additions and 43 deletions

View File

@ -46,7 +46,10 @@ class Engine final
public:
Engine(EngineConfig &config_);
Engine(const Engine &) = delete;
Engine &operator=(const Engine &) = delete;
int RunQuery(const RouteParameters &route_parameters, util::json::Object &json_result);
private:

View File

@ -42,11 +42,12 @@ template <class DataFacadeT, class Derived> class BasicRoutingInterface
DataFacadeT *facade;
public:
BasicRoutingInterface() = delete;
BasicRoutingInterface(const BasicRoutingInterface &) = delete;
explicit BasicRoutingInterface(DataFacadeT *facade) : facade(facade) {}
~BasicRoutingInterface() {}
BasicRoutingInterface(const BasicRoutingInterface &) = delete;
BasicRoutingInterface &operator=(const BasicRoutingInterface &) = delete;
/*
min_edge_offset is needed in case we use multiple
nodes as start/target nodes with different (even negative) offsets.
@ -98,7 +99,8 @@ template <class DataFacadeT, class Derived> class BasicRoutingInterface
(!force_loop_forward ||
forward_heap.GetData(node).parent !=
node) // if loops are forced, they are so at the source
&& (!force_loop_reverse || reverse_heap.GetData(node).parent != node))
&&
(!force_loop_reverse || reverse_heap.GetData(node).parent != node))
{
middle_node_id = node;
upper_bound = new_distance;
@ -362,8 +364,10 @@ template <class DataFacadeT, class Derived> class BasicRoutingInterface
BOOST_ASSERT(i < id_vector.size());
BOOST_ASSERT(phantom_node_pair.target_phantom.forward_travel_mode > 0);
unpacked_path.emplace_back(
PathData{id_vector[i], phantom_node_pair.target_phantom.name_id,
extractor::TurnInstruction::NoTurn, 0,
PathData{id_vector[i],
phantom_node_pair.target_phantom.name_id,
extractor::TurnInstruction::NoTurn,
0,
phantom_node_pair.target_phantom.forward_travel_mode});
}
}
@ -601,8 +605,8 @@ template <class DataFacadeT, class Derived> class BasicRoutingInterface
// TODO check if unordered_set might be faster
// sort by id and increasing by distance
auto entry_point_comparator = [](const std::pair<NodeID, EdgeWeight> &lhs,
const std::pair<NodeID, EdgeWeight> &rhs)
auto entry_point_comparator =
[](const std::pair<NodeID, EdgeWeight> &lhs, const std::pair<NodeID, EdgeWeight> &rhs)
{
return lhs.first < rhs.first || (lhs.first == rhs.first && lhs.second < rhs.second);
};

View File

@ -36,8 +36,8 @@ namespace extractor
class EdgeBasedGraphFactory
{
public:
EdgeBasedGraphFactory() = delete;
EdgeBasedGraphFactory(const EdgeBasedGraphFactory &) = delete;
EdgeBasedGraphFactory &operator=(const EdgeBasedGraphFactory &) = delete;
explicit EdgeBasedGraphFactory(std::shared_ptr<util::NodeBasedDynamicGraph> node_based_graph,
const CompressedEdgeContainer &compressed_edge_container,
@ -62,7 +62,7 @@ class EdgeBasedGraphFactory
const bool generate_edge_lookup);
#endif
//The following get access functions destroy the content in the factory
// The following get access functions destroy the content in the factory
void GetEdgeBasedEdges(util::DeallocatingVector<EdgeBasedEdge> &edges);
void GetEdgeBasedNodes(std::vector<EdgeBasedNode> &nodes);
void GetStartPointMarkers(std::vector<bool> &node_is_startpoint);

View File

@ -38,10 +38,11 @@ class ExtractorCallbacks
ExtractionContainers &external_memory;
public:
ExtractorCallbacks() = delete;
ExtractorCallbacks(const ExtractorCallbacks &) = delete;
explicit ExtractorCallbacks(ExtractionContainers &extraction_containers);
ExtractorCallbacks(const ExtractorCallbacks &) = delete;
ExtractorCallbacks &operator=(const ExtractorCallbacks &) = delete;
// warning: caller needs to take care of synchronization!
void ProcessNode(const osmium::Node &current_node, const ExtractionNode &result_node);

View File

@ -23,9 +23,11 @@ namespace extractor
class ScriptingEnvironment
{
public:
ScriptingEnvironment() = delete;
explicit ScriptingEnvironment(const std::string &file_name);
ScriptingEnvironment(const ScriptingEnvironment &) = delete;
ScriptingEnvironment &operator=(const ScriptingEnvironment &) = delete;
lua_State *GetLuaState();
private:

View File

@ -39,7 +39,7 @@ class Connection : public std::enable_shared_from_this<Connection>
public:
explicit Connection(boost::asio::io_service &io_service, RequestHandler &handler);
Connection(const Connection &) = delete;
Connection() = delete;
Connection &operator=(const Connection &) = delete;
boost::asio::ip::tcp::socket &socket();

View File

@ -28,6 +28,7 @@ class RequestHandler
RequestHandler();
RequestHandler(const RequestHandler &) = delete;
RequestHandler &operator=(const RequestHandler &) = delete;
void handle_request(const http::request &current_request, http::reply &current_reply);
void RegisterRoutingMachine(OSRM *osrm);

View File

@ -58,7 +58,10 @@ class SharedMemory
}
shm_remove() : m_shmid(INT_MIN), m_initialized(false) {}
shm_remove(const shm_remove &) = delete;
shm_remove &operator=(const shm_remove &) = delete;
~shm_remove()
{
if (m_initialized)
@ -75,7 +78,6 @@ class SharedMemory
public:
void *Ptr() const { return region.get_address(); }
SharedMemory() = delete;
SharedMemory(const SharedMemory &) = delete;
SharedMemory &operator=(const SharedMemory &) = delete;
@ -193,8 +195,6 @@ class SharedMemory
class shm_remove
{
private:
shm_remove(const shm_remove &) = delete;
shm_remove &operator=(const shm_remove &) = delete;
char *m_shmid;
bool m_initialized;
@ -207,6 +207,9 @@ class SharedMemory
shm_remove() : m_shmid("undefined"), m_initialized(false) {}
shm_remove(const shm_remove &) = delete;
shm_remove &operator=(const shm_remove &) = delete;
~shm_remove()
{
if (m_initialized)
@ -324,34 +327,33 @@ class SharedMemory
template <typename IdentifierT, typename LockFileT = OSRMLockFile>
SharedMemory *makeSharedMemory(const IdentifierT &id,
const uint64_t size = 0,
bool read_write = false,
bool remove_prev = true)
const uint64_t size = 0,
bool read_write = false,
bool remove_prev = true)
{
try
{
LockFileT lock_file;
if (!boost::filesystem::exists(lock_file()))
try
{
if (0 == size)
{
throw util::exception("lock file does not exist, exiting");
}
else
{
boost::filesystem::ofstream ofs(lock_file());
}
LockFileT lock_file;
if (!boost::filesystem::exists(lock_file()))
{
if (0 == size)
{
throw util::exception("lock file does not exist, exiting");
}
else
{
boost::filesystem::ofstream ofs(lock_file());
}
}
return new SharedMemory(lock_file(), id, size, read_write, remove_prev);
}
catch (const boost::interprocess::interprocess_exception &e)
{
util::SimpleLogger().Write(logWARNING) << "caught exception: " << e.what() << ", code "
<< e.get_error_code();
throw util::exception(e.what());
}
return new SharedMemory(lock_file(), id, size, read_write, remove_prev);
}
catch (const boost::interprocess::interprocess_exception &e)
{
util::SimpleLogger().Write(logWARNING) << "caught exception: " << e.what() << ", code "
<< e.get_error_code();
throw util::exception(e.what());
}
}
}
}

View File

@ -29,6 +29,7 @@ class LogPolicy
static LogPolicy &GetInstance();
LogPolicy(const LogPolicy &) = delete;
LogPolicy &operator=(const LogPolicy &) = delete;
private:
LogPolicy() : m_is_mute(true) {}

View File

@ -106,8 +106,8 @@ class StaticRTree
boost::filesystem::ifstream leaves_stream;
public:
StaticRTree() = delete;
StaticRTree(const StaticRTree &) = delete;
StaticRTree &operator=(const StaticRTree &) = delete;
template <typename CoordinateT>
// Construct a packed Hilbert-R-Tree with Kamel-Faloutsos algorithm [1]

View File

@ -19,7 +19,6 @@ namespace
{
struct CoordinatePairCalculator
{
CoordinatePairCalculator() = delete;
CoordinatePairCalculator(const util::FixedPointCoordinate coordinate_a,
const util::FixedPointCoordinate coordinate_b)
{

View File

@ -147,7 +147,6 @@ void RequestHandler::handle_request(const http::request &current_request,
catch (const std::exception &e)
{
current_reply = http::reply::stock_reply(http::reply::internal_server_error);
;
util::SimpleLogger().Write(logWARNING) << "[server error] code: " << e.what()
<< ", uri: " << current_request.uri;
}