From 3c48e612efef1e8291b4d84ccdc23b72b2e6ba90 Mon Sep 17 00:00:00 2001 From: Matej Ferencevic Date: Mon, 20 May 2019 12:54:56 +0200 Subject: [PATCH] Make move operations noexcept Reviewers: teon.banek Reviewed By: teon.banek Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D1991 --- src/communication/bolt/v1/value.cpp | 6 ++-- src/communication/bolt/v1/value.hpp | 18 +++++------ src/data_structures/concurrent/skiplist.hpp | 2 +- .../concurrent/skiplist_gc.hpp | 12 ++----- .../single_node/graph_db_accessor.cpp | 4 +-- .../single_node/graph_db_accessor.hpp | 4 +-- src/query/typed_value.cpp | 8 ++--- src/query/typed_value.hpp | 31 ++++++++++--------- src/storage/common/types/property_value.cpp | 5 +-- src/storage/common/types/property_value.hpp | 11 ++++--- src/utils/skip_list.hpp | 16 +++++----- src/utils/stack.hpp | 4 +-- 12 files changed, 59 insertions(+), 62 deletions(-) diff --git a/src/communication/bolt/v1/value.cpp b/src/communication/bolt/v1/value.cpp index 608fb0acb..f626d99ca 100644 --- a/src/communication/bolt/v1/value.cpp +++ b/src/communication/bolt/v1/value.cpp @@ -123,7 +123,7 @@ Value &Value::operator=(const Value &other) { return *this; } -Value::Value(Value &&other) : type_(other.type_) { +Value::Value(Value &&other) noexcept : type_(other.type_) { switch (other.type_) { case Type::Null: break; @@ -164,7 +164,7 @@ Value::Value(Value &&other) : type_(other.type_) { other.type_ = Type::Null; } -Value &Value::operator=(Value &&other) { +Value &Value::operator=(Value &&other) noexcept { if (this != &other) { this->~Value(); // set the type of this @@ -380,4 +380,4 @@ std::ostream &operator<<(std::ostream &os, const Value::Type type) { return os << "path"; } } -} +} // namespace communication::bolt diff --git a/src/communication/bolt/v1/value.hpp b/src/communication/bolt/v1/value.hpp index 823272fb6..80dfc797b 100644 --- a/src/communication/bolt/v1/value.hpp +++ b/src/communication/bolt/v1/value.hpp @@ -166,32 +166,32 @@ class Value { Value(const Path &value) : type_(Type::Path) { new (&path_v) Path(value); } // move constructors for non-primitive values - Value(std::string &&value) : type_(Type::String) { + Value(std::string &&value) noexcept : type_(Type::String) { new (&string_v) std::string(std::move(value)); } - Value(std::vector &&value) : type_(Type::List) { + Value(std::vector &&value) noexcept : type_(Type::List) { new (&list_v) std::vector(std::move(value)); } - Value(std::map &&value) : type_(Type::Map) { + Value(std::map &&value) noexcept : type_(Type::Map) { new (&map_v) std::map(std::move(value)); } - Value(Vertex &&value) : type_(Type::Vertex) { + Value(Vertex &&value) noexcept : type_(Type::Vertex) { new (&vertex_v) Vertex(std::move(value)); } - Value(Edge &&value) : type_(Type::Edge) { + Value(Edge &&value) noexcept : type_(Type::Edge) { new (&edge_v) Edge(std::move(value)); } - Value(UnboundedEdge &&value) : type_(Type::UnboundedEdge) { + Value(UnboundedEdge &&value) noexcept : type_(Type::UnboundedEdge) { new (&unbounded_edge_v) UnboundedEdge(std::move(value)); } - Value(Path &&value) : type_(Type::Path) { + Value(Path &&value) noexcept : type_(Type::Path) { new (&path_v) Path(std::move(value)); } Value &operator=(const Value &other); - Value &operator=(Value &&other); + Value &operator=(Value &&other) noexcept; Value(const Value &other); - Value(Value &&other); + Value(Value &&other) noexcept; ~Value(); Type type() const { return type_; } diff --git a/src/data_structures/concurrent/skiplist.hpp b/src/data_structures/concurrent/skiplist.hpp index 19392de2f..3f01e4196 100644 --- a/src/data_structures/concurrent/skiplist.hpp +++ b/src/data_structures/concurrent/skiplist.hpp @@ -470,7 +470,7 @@ class SkipList : private utils::Lockable { Accessor(const Accessor &) = delete; // cppcheck-suppress uninitMemberVar - Accessor(Accessor &&other) + Accessor(Accessor &&other) noexcept : skiplist(other.skiplist), status_(other.status_) { other.skiplist = nullptr; } diff --git a/src/data_structures/concurrent/skiplist_gc.hpp b/src/data_structures/concurrent/skiplist_gc.hpp index e1756a8a9..40b882ccd 100644 --- a/src/data_structures/concurrent/skiplist_gc.hpp +++ b/src/data_structures/concurrent/skiplist_gc.hpp @@ -73,15 +73,7 @@ class SkipListGC { * ones are alive and which ones are dead. */ struct AccessorStatus { - AccessorStatus(const int64_t id, bool alive) : id_(id), alive_(alive) {} - - AccessorStatus(AccessorStatus &&other) = default; - - AccessorStatus(const AccessorStatus &other) = delete; - AccessorStatus operator=(const AccessorStatus &other) = delete; - AccessorStatus operator=(AccessorStatus &&other) = delete; - - const int64_t id_{-1}; + int64_t id_{-1}; bool alive_{false}; }; @@ -91,7 +83,7 @@ class SkipListGC { */ AccessorStatus &CreateNewAccessor() { std::unique_lock lock(mutex_); - accessors_.emplace_back(++last_accessor_id_, true); + accessors_.push_back({++last_accessor_id_, true}); return accessors_.back(); } diff --git a/src/database/single_node/graph_db_accessor.cpp b/src/database/single_node/graph_db_accessor.cpp index 16e0115d9..e3f7e1bb6 100644 --- a/src/database/single_node/graph_db_accessor.cpp +++ b/src/database/single_node/graph_db_accessor.cpp @@ -32,7 +32,7 @@ GraphDbAccessor::GraphDbAccessor(GraphDb *db, transaction_(db->tx_engine().BeginBlocking(parent_tx)), transaction_starter_{true} {} -GraphDbAccessor::GraphDbAccessor(GraphDbAccessor &&other) +GraphDbAccessor::GraphDbAccessor(GraphDbAccessor &&other) noexcept : db_(other.db_), transaction_(other.transaction_), transaction_starter_(other.transaction_starter_), @@ -43,7 +43,7 @@ GraphDbAccessor::GraphDbAccessor(GraphDbAccessor &&other) other.transaction_starter_ = false; } -GraphDbAccessor &GraphDbAccessor::operator=(GraphDbAccessor &&other) { +GraphDbAccessor &GraphDbAccessor::operator=(GraphDbAccessor &&other) noexcept { db_ = other.db_; transaction_ = other.transaction_; transaction_starter_ = other.transaction_starter_; diff --git a/src/database/single_node/graph_db_accessor.hpp b/src/database/single_node/graph_db_accessor.hpp index 2f35b00f2..42c017d92 100644 --- a/src/database/single_node/graph_db_accessor.hpp +++ b/src/database/single_node/graph_db_accessor.hpp @@ -54,8 +54,8 @@ class GraphDbAccessor { GraphDbAccessor(const GraphDbAccessor &other) = delete; GraphDbAccessor &operator=(const GraphDbAccessor &other) = delete; - GraphDbAccessor(GraphDbAccessor &&other); - GraphDbAccessor &operator=(GraphDbAccessor &&other); + GraphDbAccessor(GraphDbAccessor &&other) noexcept; + GraphDbAccessor &operator=(GraphDbAccessor &&other) noexcept; /** * Creates a new Vertex and returns an accessor to it. If the ID is diff --git a/src/query/typed_value.cpp b/src/query/typed_value.cpp index 2c96c7a13..889a161dc 100644 --- a/src/query/typed_value.cpp +++ b/src/query/typed_value.cpp @@ -51,7 +51,7 @@ TypedValue::TypedValue(const PropertyValue &value) { LOG(FATAL) << "Unsupported type"; } -TypedValue::TypedValue(PropertyValue &&value) { +TypedValue::TypedValue(PropertyValue &&value) noexcept { switch (value.type()) { case PropertyValue::Type::Null: type_ = Type::Null; @@ -128,7 +128,7 @@ TypedValue::TypedValue(const TypedValue &other) : type_(other.type_) { LOG(FATAL) << "Unsupported TypedValue::Type"; } -TypedValue::TypedValue(TypedValue &&other) : type_(other.type_) { +TypedValue::TypedValue(TypedValue &&other) noexcept : type_(other.type_) { switch (other.type_) { case TypedValue::Type::Null: break; @@ -335,7 +335,7 @@ DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const Path &, Path, path_v) #define DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(type_param, typed_value_type, \ member) \ - TypedValue &TypedValue::operator=(type_param &&other) { \ + TypedValue &TypedValue::operator=(type_param &&other) noexcept { \ if (this->type_ == TypedValue::Type::typed_value_type) { \ this->member = std::move(other); \ } else { \ @@ -395,7 +395,7 @@ TypedValue &TypedValue::operator=(const TypedValue &other) { return *this; } -TypedValue &TypedValue::operator=(TypedValue &&other) { +TypedValue &TypedValue::operator=(TypedValue &&other) noexcept { if (this != &other) { DestroyValue(); type_ = other.type_; diff --git a/src/query/typed_value.hpp b/src/query/typed_value.hpp index 5597212fc..7cbacce4e 100644 --- a/src/query/typed_value.hpp +++ b/src/query/typed_value.hpp @@ -78,7 +78,7 @@ class TypedValue { static const TypedValue Null; TypedValue(const TypedValue &other); - TypedValue(TypedValue &&other); + TypedValue(TypedValue &&other) noexcept; // constructors for primitive types TypedValue(bool value) : type_(Type::Bool) { bool_v = value; } @@ -113,25 +113,26 @@ class TypedValue { TypedValue(const PropertyValue &value); // move constructors for non-primitive types - TypedValue(std::string &&value) : type_(Type::String) { + TypedValue(std::string &&value) noexcept : type_(Type::String) { new (&string_v) std::string(std::move(value)); } - TypedValue(std::vector &&value) : type_(Type::List) { + TypedValue(std::vector &&value) noexcept : type_(Type::List) { new (&list_v) std::vector(std::move(value)); } - TypedValue(std::map &&value) : type_(Type::Map) { + TypedValue(std::map &&value) noexcept + : type_(Type::Map) { new (&map_v) std::map(std::move(value)); } - TypedValue(VertexAccessor &&vertex) : type_(Type::Vertex) { + TypedValue(VertexAccessor &&vertex) noexcept : type_(Type::Vertex) { new (&vertex_v) VertexAccessor(std::move(vertex)); } - TypedValue(EdgeAccessor &&edge) : type_(Type::Edge) { + TypedValue(EdgeAccessor &&edge) noexcept : type_(Type::Edge) { new (&edge_v) EdgeAccessor(std::move(edge)); } - TypedValue(Path &&path) : type_(Type::Path) { + TypedValue(Path &&path) noexcept : type_(Type::Path) { new (&path_v) Path(std::move(path)); } - TypedValue(PropertyValue &&value); + TypedValue(PropertyValue &&value) noexcept; // copy assignment operators TypedValue &operator=(const char *); @@ -148,13 +149,13 @@ class TypedValue { TypedValue &operator=(const TypedValue &); // move assignment operators - TypedValue &operator=(std::string &&); - TypedValue &operator=(std::vector &&); - TypedValue &operator=(TypedValue::value_map_t &&); - TypedValue &operator=(VertexAccessor &&); - TypedValue &operator=(EdgeAccessor &&); - TypedValue &operator=(Path &&); - TypedValue &operator=(TypedValue &&); + TypedValue &operator=(std::string &&) noexcept; + TypedValue &operator=(std::vector &&) noexcept; + TypedValue &operator=(TypedValue::value_map_t &&) noexcept; + TypedValue &operator=(VertexAccessor &&) noexcept; + TypedValue &operator=(EdgeAccessor &&) noexcept; + TypedValue &operator=(Path &&) noexcept; + TypedValue &operator=(TypedValue &&) noexcept; ~TypedValue(); diff --git a/src/storage/common/types/property_value.cpp b/src/storage/common/types/property_value.cpp index 3cb706047..52b545555 100644 --- a/src/storage/common/types/property_value.cpp +++ b/src/storage/common/types/property_value.cpp @@ -127,7 +127,8 @@ PropertyValue::PropertyValue(const PropertyValue &other) : type_(other.type_) { } } -PropertyValue::PropertyValue(PropertyValue &&other) : type_(other.type_) { +PropertyValue::PropertyValue(PropertyValue &&other) noexcept + : type_(other.type_) { switch (other.type_) { case Type::Null: return; @@ -183,7 +184,7 @@ PropertyValue &PropertyValue::operator=(const PropertyValue &other) { return *this; } -PropertyValue &PropertyValue::operator=(PropertyValue &&other) { +PropertyValue &PropertyValue::operator=(PropertyValue &&other) noexcept { if (this != &other) { DestroyValue(); type_ = other.type_; diff --git a/src/storage/common/types/property_value.hpp b/src/storage/common/types/property_value.hpp index 558eee2d3..f5643b517 100644 --- a/src/storage/common/types/property_value.hpp +++ b/src/storage/common/types/property_value.hpp @@ -56,22 +56,23 @@ class PropertyValue { } // move constructors for non-primitive types - PropertyValue(std::string &&value) : type_(Type::String) { + PropertyValue(std::string &&value) noexcept : type_(Type::String) { new (&string_v) std::string(std::move(value)); } - PropertyValue(std::vector &&value) : type_(Type::List) { + PropertyValue(std::vector &&value) noexcept + : type_(Type::List) { new (&list_v) std::vector(std::move(value)); } - PropertyValue(std::map &&value) + PropertyValue(std::map &&value) noexcept : type_(Type::Map) { new (&map_v) std::map(std::move(value)); } PropertyValue &operator=(const PropertyValue &other); - PropertyValue &operator=(PropertyValue &&other); + PropertyValue &operator=(PropertyValue &&other) noexcept; PropertyValue(const PropertyValue &other); - PropertyValue(PropertyValue &&other); + PropertyValue(PropertyValue &&other) noexcept; ~PropertyValue(); Type type() const { return type_; } diff --git a/src/utils/skip_list.hpp b/src/utils/skip_list.hpp index b7e83a4b3..173e7ab08 100644 --- a/src/utils/skip_list.hpp +++ b/src/utils/skip_list.hpp @@ -71,7 +71,7 @@ struct SkipListNode { SkipListNode(const TObj &_obj, uint8_t _height) : obj(_obj), height(_height) {} - SkipListNode(TObj &&_obj, uint8_t _height) + SkipListNode(TObj &&_obj, uint8_t _height) noexcept : obj(std::move(_obj)), height(_height) {} // The items here are carefully placed to minimize padding gaps. @@ -540,14 +540,15 @@ class SkipList final { Accessor(const Accessor &other) : skiplist_(other.skiplist_), id_(skiplist_->gc_.AllocateId()) {} - Accessor(Accessor &&other) : skiplist_(other.skiplist_), id_(other.id_) { + Accessor(Accessor &&other) noexcept + : skiplist_(other.skiplist_), id_(other.id_) { other.skiplist_ = nullptr; } Accessor &operator=(const Accessor &other) { skiplist_ = other.skiplist_; id_ = skiplist_->gc_.AllocateId(); } - Accessor &operator=(Accessor &&other) { + Accessor &operator=(Accessor &&other) noexcept { skiplist_ = other.skiplist_; id_ = other.id_; other.skiplist_ = nullptr; @@ -680,7 +681,7 @@ class SkipList final { ConstAccessor(const ConstAccessor &other) : skiplist_(other.skiplist_), id_(skiplist_->gc_.AllocateId()) {} - ConstAccessor(ConstAccessor &&other) + ConstAccessor(ConstAccessor &&other) noexcept : skiplist_(other.skiplist_), id_(other.id_) { other.skiplist_ = nullptr; } @@ -688,7 +689,7 @@ class SkipList final { skiplist_ = other.skiplist_; id_ = skiplist_->gc_.AllocateId(); } - ConstAccessor &operator=(ConstAccessor &&other) { + ConstAccessor &operator=(ConstAccessor &&other) noexcept { skiplist_ = other.skiplist_; id_ = other.id_; other.skiplist_ = nullptr; @@ -757,10 +758,11 @@ class SkipList final { SkipListNode(kSkipListMaxHeight); } - SkipList(SkipList &&other) : head_(other.head_), size_(other.size_.load()) { + SkipList(SkipList &&other) noexcept + : head_(other.head_), size_(other.size_.load()) { other.head_ = nullptr; } - SkipList &operator=(SkipList &&other) { + SkipList &operator=(SkipList &&other) noexcept { TNode *head = head_; while (head != nullptr) { TNode *succ = head->nexts[0].load(std::memory_order_relaxed); diff --git a/src/utils/stack.hpp b/src/utils/stack.hpp index 2f4b9587a..19207e542 100644 --- a/src/utils/stack.hpp +++ b/src/utils/stack.hpp @@ -46,8 +46,8 @@ class Stack { "the size of Stack::Block is a multiple of the page size."); } - Stack(Stack &&other) : head_(other.head_) { other.head_ = nullptr; } - Stack &operator=(Stack &&other) { + Stack(Stack &&other) noexcept : head_(other.head_) { other.head_ = nullptr; } + Stack &operator=(Stack &&other) noexcept { while (head_ != nullptr) { Block *prev = head_->prev; delete head_;