diff --git a/docs/dev/code-review.md b/docs/dev/code-review.md index de86c6f10..049a3b9eb 100644 --- a/docs/dev/code-review.md +++ b/docs/dev/code-review.md @@ -47,6 +47,7 @@ You need to watch out for the following. * Is the move assignment done correctly, also it may throw an exception. * Is the copy construction done with the right allocator. * Is the copy assignment done correctly. + * Using `auto` makes allocator propagation rules rather ambiguous. ## Classes & Object Oriented Programming diff --git a/src/query/typed_value.cpp b/src/query/typed_value.cpp index bc8ad19dc..e38e318c8 100644 --- a/src/query/typed_value.cpp +++ b/src/query/typed_value.cpp @@ -14,7 +14,13 @@ namespace query { -TypedValue::TypedValue(const PropertyValue &value) { +TypedValue::TypedValue(const PropertyValue &value) + // TODO: MemoryResource in PropertyValue + : TypedValue(value, utils::NewDeleteResource()) {} + +TypedValue::TypedValue(const PropertyValue &value, + utils::MemoryResource *memory) + : memory_(memory) { switch (value.type()) { case PropertyValue::Type::Null: type_ = Type::Null; @@ -51,30 +57,35 @@ TypedValue::TypedValue(const PropertyValue &value) { LOG(FATAL) << "Unsupported type"; } -TypedValue::TypedValue(PropertyValue &&value) noexcept { - switch (value.type()) { +TypedValue::TypedValue(PropertyValue &&other) /* noexcept */ + // TODO: MemoryResource in PropertyValue, so this can be noexcept + : TypedValue(std::move(other), utils::NewDeleteResource()) {} + +TypedValue::TypedValue(PropertyValue &&other, utils::MemoryResource *memory) + : memory_(memory) { + switch (other.type()) { case PropertyValue::Type::Null: type_ = Type::Null; break; case PropertyValue::Type::Bool: type_ = Type::Bool; - bool_v = value.Value(); + bool_v = other.Value(); break; case PropertyValue::Type::Int: type_ = Type::Int; - int_v = value.Value(); + int_v = other.Value(); break; case PropertyValue::Type::Double: type_ = Type::Double; - double_v = value.Value(); + double_v = other.Value(); break; case PropertyValue::Type::String: type_ = Type::String; - new (&string_v) std::string(std::move(value.Value())); + new (&string_v) std::string(std::move(other.Value())); break; case PropertyValue::Type::List: { type_ = Type::List; - auto &vec = value.Value>(); + auto &vec = other.Value>(); new (&list_v) std::vector(std::make_move_iterator(vec.begin()), std::make_move_iterator(vec.end())); @@ -82,7 +93,7 @@ TypedValue::TypedValue(PropertyValue &&value) noexcept { } case PropertyValue::Type::Map: { type_ = Type::Map; - auto &map = value.Value>(); + auto &map = other.Value>(); new (&map_v) std::map( std::make_move_iterator(map.begin()), std::make_move_iterator(map.end())); @@ -90,10 +101,16 @@ TypedValue::TypedValue(PropertyValue &&value) noexcept { } } - value = PropertyValue::Null; + other = PropertyValue::Null; } -TypedValue::TypedValue(const TypedValue &other) : type_(other.type_) { +TypedValue::TypedValue(const TypedValue &other) + : TypedValue(other, std::allocator_traits>:: + select_on_container_copy_construction(other.memory_) + .GetMemoryResource()) {} + +TypedValue::TypedValue(const TypedValue &other, utils::MemoryResource *memory) + : memory_(memory), type_(other.type_) { switch (other.type_) { case TypedValue::Type::Null: return; @@ -128,7 +145,11 @@ TypedValue::TypedValue(const TypedValue &other) : type_(other.type_) { LOG(FATAL) << "Unsupported TypedValue::Type"; } -TypedValue::TypedValue(TypedValue &&other) noexcept : type_(other.type_) { +TypedValue::TypedValue(TypedValue &&other) noexcept + : TypedValue(std::move(other), other.memory_) {} + +TypedValue::TypedValue(TypedValue &&other, utils::MemoryResource *memory) + : memory_(memory), type_(other.type_) { switch (other.type_) { case TypedValue::Type::Null: break; @@ -247,7 +268,7 @@ bool TypedValue::IsPropertyValue() const { } } -std::ostream &operator<<(std::ostream &os, const TypedValue::Type type) { +std::ostream &operator<<(std::ostream &os, const TypedValue::Type &type) { switch (type) { case TypedValue::Type::Null: return os << "null"; @@ -312,7 +333,7 @@ std::ostream &operator<<(std::ostream &os, const TypedValue &value) { if (this->type_ == TypedValue::Type::typed_value_type) { \ this->member = other; \ } else { \ - *this = TypedValue(other); \ + *this = TypedValue(other, memory_); \ } \ \ return *this; \ @@ -339,7 +360,7 @@ DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const Path &, Path, path_v) if (this->type_ == TypedValue::Type::typed_value_type) { \ this->member = std::move(other); \ } else { \ - *this = TypedValue(std::move(other)); \ + *this = TypedValue(std::move(other), memory_); \ } \ \ return *this; \ @@ -356,6 +377,14 @@ DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(Path, Path, path_v) TypedValue &TypedValue::operator=(const TypedValue &other) { if (this != &other) { + // NOTE: STL uses + // std::allocator_traits<>::propagate_on_container_copy_assignment to + // determine whether to take the allocator from `other`, or use the one in + // `this`. Our utils::Allocator never propagates, so we use the allocator + // from `this`. + static_assert(!std::allocator_traits>:: + propagate_on_container_copy_assignment::value, + "Allocator propagation not implemented"); DestroyValue(); type_ = other.type_; @@ -398,6 +427,14 @@ TypedValue &TypedValue::operator=(const TypedValue &other) { TypedValue &TypedValue::operator=(TypedValue &&other) noexcept { if (this != &other) { DestroyValue(); + // NOTE: STL uses + // std::allocator_traits<>::propagate_on_container_move_assignment to + // determine whether to take the allocator from `other`, or use the one in + // `this`. Our utils::Allocator never propagates, so we use the allocator + // from `this`. + static_assert(!std::allocator_traits>:: + propagate_on_container_move_assignment::value, + "Allocator propagation not implemented"); type_ = other.type_; switch (other.type_) { @@ -518,55 +555,56 @@ TypedValue operator<(const TypedValue &a, const TypedValue &b) { throw TypedValueException("Invalid 'less' operand types({} + {})", a.type(), b.type()); - if (a.IsNull() || b.IsNull()) return TypedValue::Null; + if (a.IsNull() || b.IsNull()) return TypedValue(a.GetMemoryResource()); if (a.IsString() || b.IsString()) { if (a.type() != b.type()) { throw TypedValueException("Invalid 'less' operand types({} + {})", a.type(), b.type()); } else { - return a.Value() < b.Value(); + return TypedValue(a.Value() < b.Value(), + a.GetMemoryResource()); } } // at this point we only have int and double if (a.IsDouble() || b.IsDouble()) { - return ToDouble(a) < ToDouble(b); + return TypedValue(ToDouble(a) < ToDouble(b), a.GetMemoryResource()); } else { - return a.Value() < b.Value(); + return TypedValue(a.Value() < b.Value(), + a.GetMemoryResource()); } } -/** Equality between two typed values that returns either a bool - * or Null TypedValue (never raises an exception). - * - * For the old version of equality that raised an exception - * when comparing incompatible types, see the version of - * this file at 2017-04-12. - */ TypedValue operator==(const TypedValue &a, const TypedValue &b) { - if (a.IsNull() || b.IsNull()) return TypedValue::Null; + if (a.IsNull() || b.IsNull()) return TypedValue(a.GetMemoryResource()); // check we have values that can be compared // this means that either they're the same type, or (int, double) combo - if ((a.type() != b.type() && !(a.IsNumeric() && b.IsNumeric()))) return false; + if ((a.type() != b.type() && !(a.IsNumeric() && b.IsNumeric()))) + return TypedValue(false, a.GetMemoryResource()); switch (a.type()) { case TypedValue::Type::Bool: - return a.Value() == b.Value(); + return TypedValue(a.Value() == b.Value(), + a.GetMemoryResource()); case TypedValue::Type::Int: if (b.IsDouble()) - return ToDouble(a) == ToDouble(b); + return TypedValue(ToDouble(a) == ToDouble(b), a.GetMemoryResource()); else - return a.Value() == b.Value(); + return TypedValue(a.Value() == b.Value(), + a.GetMemoryResource()); case TypedValue::Type::Double: - return ToDouble(a) == ToDouble(b); + return TypedValue(ToDouble(a) == ToDouble(b), a.GetMemoryResource()); case TypedValue::Type::String: - return a.Value() == b.Value(); + return TypedValue(a.Value() == b.Value(), + a.GetMemoryResource()); case TypedValue::Type::Vertex: - return a.Value() == b.Value(); + return TypedValue(a.Value() == b.Value(), + a.GetMemoryResource()); case TypedValue::Type::Edge: - return a.Value() == b.Value(); + return TypedValue(a.Value() == b.Value(), + a.GetMemoryResource()); case TypedValue::Type::List: { // We are not compatible with neo4j at this point. In neo4j 2 = [2] // compares @@ -576,39 +614,44 @@ TypedValue operator==(const TypedValue &a, const TypedValue &b) { // well. Because, why not? // At memgraph we prefer sanity so [1,2] = [1,2] compares to true and // 2 = [2] compares to false. - auto &list_a = a.ValueList(); - auto &list_b = b.ValueList(); - if (list_a.size() != list_b.size()) return false; + const auto &list_a = a.ValueList(); + const auto &list_b = b.ValueList(); + if (list_a.size() != list_b.size()) + return TypedValue(false, a.GetMemoryResource()); // two arrays are considered equal (by neo) if all their // elements are bool-equal. this means that: // [1] == [null] -> false // [null] == [null] -> true // in that sense array-comparison never results in Null - return std::equal(list_a.begin(), list_a.end(), list_b.begin(), - TypedValue::BoolEqual{}); + return TypedValue(std::equal(list_a.begin(), list_a.end(), list_b.begin(), + TypedValue::BoolEqual{}), + a.GetMemoryResource()); } case TypedValue::Type::Map: { - auto &map_a = a.Value>(); - auto &map_b = b.Value>(); - if (map_a.size() != map_b.size()) return false; - for (auto &kv_a : map_a) { + const auto &map_a = a.Value>(); + const auto &map_b = b.Value>(); + if (map_a.size() != map_b.size()) + return TypedValue(false, a.GetMemoryResource()); + for (const auto &kv_a : map_a) { auto found_b_it = map_b.find(kv_a.first); - if (found_b_it == map_b.end()) return false; + if (found_b_it == map_b.end()) + return TypedValue(false, a.GetMemoryResource()); TypedValue comparison = kv_a.second == found_b_it->second; - if (comparison.IsNull() || !comparison.Value()) return false; + if (comparison.IsNull() || !comparison.Value()) + return TypedValue(false, a.GetMemoryResource()); } - return true; + return TypedValue(true, a.GetMemoryResource()); } case TypedValue::Type::Path: - return a.ValuePath() == b.ValuePath(); + return TypedValue(a.ValuePath() == b.ValuePath(), a.GetMemoryResource()); default: LOG(FATAL) << "Unhandled comparison for types"; } } TypedValue operator!(const TypedValue &a) { - if (a.IsNull()) return TypedValue::Null; - if (a.IsBool()) return TypedValue(!a.Value()); + if (a.IsNull()) return TypedValue(a.GetMemoryResource()); + if (a.IsBool()) return TypedValue(!a.Value(), a.GetMemoryResource()); throw TypedValueException("Invalid logical not operand type (!{})", a.type()); } @@ -628,16 +671,18 @@ std::string ValueToString(const TypedValue &value) { } TypedValue operator-(const TypedValue &a) { - if (a.IsNull()) return TypedValue::Null; - if (a.IsInt()) return -a.Value(); - if (a.IsDouble()) return -a.Value(); + if (a.IsNull()) return TypedValue(a.GetMemoryResource()); + if (a.IsInt()) return TypedValue(-a.Value(), a.GetMemoryResource()); + if (a.IsDouble()) + return TypedValue(-a.Value(), a.GetMemoryResource()); throw TypedValueException("Invalid unary minus operand type (-{})", a.type()); } TypedValue operator+(const TypedValue &a) { - if (a.IsNull()) return TypedValue::Null; - if (a.IsInt()) return +a.Value(); - if (a.IsDouble()) return +a.Value(); + if (a.IsNull()) return TypedValue(a.GetMemoryResource()); + if (a.IsInt()) return TypedValue(+a.Value(), a.GetMemoryResource()); + if (a.IsDouble()) + return TypedValue(+a.Value(), a.GetMemoryResource()); throw TypedValueException("Invalid unary plus operand type (+{})", a.type()); } @@ -669,7 +714,7 @@ inline void EnsureArithmeticallyOk(const TypedValue &a, const TypedValue &b, } TypedValue operator+(const TypedValue &a, const TypedValue &b) { - if (a.IsNull() || b.IsNull()) return TypedValue::Null; + if (a.IsNull() || b.IsNull()) return TypedValue(a.GetMemoryResource()); if (a.IsList() || b.IsList()) { std::vector list; @@ -683,70 +728,78 @@ TypedValue operator+(const TypedValue &a, const TypedValue &b) { }; append_list(a); append_list(b); - return TypedValue(list); + return TypedValue(list, a.GetMemoryResource()); } EnsureArithmeticallyOk(a, b, true, "addition"); // no more Bool nor Null, summing works on anything from here onward - if (a.IsString() || b.IsString()) return ValueToString(a) + ValueToString(b); + if (a.IsString() || b.IsString()) + return TypedValue(ValueToString(a) + ValueToString(b), + a.GetMemoryResource()); // at this point we only have int and double if (a.IsDouble() || b.IsDouble()) { - return ToDouble(a) + ToDouble(b); + return TypedValue(ToDouble(a) + ToDouble(b), a.GetMemoryResource()); } else { - return a.Value() + b.Value(); + return TypedValue(a.Value() + b.Value(), + a.GetMemoryResource()); } } TypedValue operator-(const TypedValue &a, const TypedValue &b) { - if (a.IsNull() || b.IsNull()) return TypedValue::Null; + if (a.IsNull() || b.IsNull()) return TypedValue(a.GetMemoryResource()); EnsureArithmeticallyOk(a, b, false, "subtraction"); // at this point we only have int and double if (a.IsDouble() || b.IsDouble()) { - return ToDouble(a) - ToDouble(b); + return TypedValue(ToDouble(a) - ToDouble(b), a.GetMemoryResource()); } else { - return a.Value() - b.Value(); + return TypedValue(a.Value() - b.Value(), + a.GetMemoryResource()); } } TypedValue operator/(const TypedValue &a, const TypedValue &b) { - if (a.IsNull() || b.IsNull()) return TypedValue::Null; + if (a.IsNull() || b.IsNull()) return TypedValue(a.GetMemoryResource()); EnsureArithmeticallyOk(a, b, false, "division"); // at this point we only have int and double if (a.IsDouble() || b.IsDouble()) { - return ToDouble(a) / ToDouble(b); + return TypedValue(ToDouble(a) / ToDouble(b), a.GetMemoryResource()); } else { if (b.Value() == 0LL) throw TypedValueException("Division by zero"); - return a.Value() / b.Value(); + return TypedValue(a.Value() / b.Value(), + a.GetMemoryResource()); } } TypedValue operator*(const TypedValue &a, const TypedValue &b) { - if (a.IsNull() || b.IsNull()) return TypedValue::Null; + if (a.IsNull() || b.IsNull()) return TypedValue(a.GetMemoryResource()); EnsureArithmeticallyOk(a, b, false, "multiplication"); // at this point we only have int and double if (a.IsDouble() || b.IsDouble()) { - return ToDouble(a) * ToDouble(b); + return TypedValue(ToDouble(a) * ToDouble(b), a.GetMemoryResource()); } else { - return a.Value() * b.Value(); + return TypedValue(a.Value() * b.Value(), + a.GetMemoryResource()); } } TypedValue operator%(const TypedValue &a, const TypedValue &b) { - if (a.IsNull() || b.IsNull()) return TypedValue::Null; + if (a.IsNull() || b.IsNull()) return TypedValue(a.GetMemoryResource()); EnsureArithmeticallyOk(a, b, false, "modulo"); // at this point we only have int and double if (a.IsDouble() || b.IsDouble()) { - return (double)fmod(ToDouble(a), ToDouble(b)); + return TypedValue(static_cast(fmod(ToDouble(a), ToDouble(b))), + a.GetMemoryResource()); } else { if (b.Value() == 0LL) throw TypedValueException("Mod with zero"); - return a.Value() % b.Value(); + return TypedValue(a.Value() % b.Value(), + a.GetMemoryResource()); } } @@ -761,31 +814,36 @@ TypedValue operator&&(const TypedValue &a, const TypedValue &b) { EnsureLogicallyOk(a, b, "logical AND"); // at this point we only have null and bool // if either operand is false, the result is false - if (a.IsBool() && !a.Value()) return false; - if (b.IsBool() && !b.Value()) return false; - if (a.IsNull() || b.IsNull()) return TypedValue::Null; + if (a.IsBool() && !a.Value()) + return TypedValue(false, a.GetMemoryResource()); + if (b.IsBool() && !b.Value()) + return TypedValue(false, a.GetMemoryResource()); + if (a.IsNull() || b.IsNull()) return TypedValue(a.GetMemoryResource()); // neither is false, neither is null, thus both are true - return true; + return TypedValue(true, a.GetMemoryResource()); } TypedValue operator||(const TypedValue &a, const TypedValue &b) { EnsureLogicallyOk(a, b, "logical OR"); // at this point we only have null and bool // if either operand is true, the result is true - if (a.IsBool() && a.Value()) return true; - if (b.IsBool() && b.Value()) return true; - if (a.IsNull() || b.IsNull()) return TypedValue::Null; + if (a.IsBool() && a.Value()) + return TypedValue(true, a.GetMemoryResource()); + if (b.IsBool() && b.Value()) + return TypedValue(true, a.GetMemoryResource()); + if (a.IsNull() || b.IsNull()) return TypedValue(a.GetMemoryResource()); // neither is true, neither is null, thus both are false - return false; + return TypedValue(false, a.GetMemoryResource()); } TypedValue operator^(const TypedValue &a, const TypedValue &b) { EnsureLogicallyOk(a, b, "logical XOR"); // at this point we only have null and bool if (a.IsNull() || b.IsNull()) - return TypedValue::Null; + return TypedValue(a.GetMemoryResource()); else - return static_cast(a.Value() ^ b.Value()); + return TypedValue(static_cast(a.Value() ^ b.Value()), + a.GetMemoryResource()); } bool TypedValue::BoolEqual::operator()(const TypedValue &lhs, diff --git a/src/query/typed_value.hpp b/src/query/typed_value.hpp index 7cbacce4e..cbb38463e 100644 --- a/src/query/typed_value.hpp +++ b/src/query/typed_value.hpp @@ -14,6 +14,7 @@ #include "storage/edge_accessor.hpp" #include "storage/vertex_accessor.hpp" #include "utils/exceptions.hpp" +#include "utils/memory.hpp" namespace query { @@ -48,18 +49,6 @@ class TypedValue { size_t operator()(const TypedValue &value) const; }; - /** - * Unordered set of TypedValue items. Can contain at most one Null element, - * and treats an integral and floating point value as same if they are equal - * in the floating point domain (TypedValue operator== behaves the same). - * */ - using unordered_set = std::unordered_set; - using value_map_t = std::map; - - /** Private default constructor, makes Null */ - TypedValue() : type_(Type::Null) {} - - public: /** A value type. Each type corresponds to exactly one C++ type */ enum class Type : unsigned { Null, @@ -74,65 +63,204 @@ class TypedValue { Path }; + /** + * Unordered set of TypedValue items. Can contain at most one Null element, + * and treats an integral and floating point value as same if they are equal + * in the floating point domain (TypedValue operator== behaves the same). + */ + using unordered_set = std::unordered_set; + using value_map_t = std::map; + + /** Allocator type so that STL containers are aware that we need one */ + using allocator_type = utils::Allocator; + + /** Construct a Null value with default utils::NewDeleteResource(). */ + TypedValue() : type_(Type::Null) {} + + /** Construct a Null value with given utils::MemoryResource. */ + explicit TypedValue(utils::MemoryResource *memory) + : memory_(memory), type_(Type::Null) {} + // single static reference to Null, used whenever Null should be returned + // TODO: Remove this as it may be needed to construct TypedValue with a + // different MemoryResource. static const TypedValue Null; + /** + * Construct a copy of other. + * utils::MemoryResource is obtained by calling + * std::allocator_traits<>::select_on_container_copy_construction(other.memory_). + * Since we use utils::Allocator, which does not propagate, this means that + * memory_ will be the default utils::NewDeleteResource(). + */ TypedValue(const TypedValue &other); + + /** Construct a copy using the given utils::MemoryResource */ + TypedValue(const TypedValue &other, utils::MemoryResource *memory); + + /** + * Construct with the value of other. + * utils::MemoryResource is obtained from other. After the move, other will be + * set to Null. + */ TypedValue(TypedValue &&other) noexcept; + /** + * Construct with the value of other, but use the given utils::MemoryResource. + * After the move, other will be set to Null. + * If `*memory != *other.GetMemoryResource()`, then a copy is made instead of + * a move. + */ + TypedValue(TypedValue &&other, utils::MemoryResource *memory); + // constructors for primitive types - TypedValue(bool value) : type_(Type::Bool) { bool_v = value; } - TypedValue(int value) : type_(Type::Int) { int_v = value; } - TypedValue(int64_t value) : type_(Type::Int) { int_v = value; } - TypedValue(double value) : type_(Type::Double) { double_v = value; } + TypedValue(bool value, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::Bool) { + bool_v = value; + } + + TypedValue(int value, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::Int) { + int_v = value; + } + + TypedValue(int64_t value, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::Int) { + int_v = value; + } + + TypedValue(double value, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::Double) { + double_v = value; + } // conversion function to PropertyValue explicit operator PropertyValue() const; // copy constructors for non-primitive types - TypedValue(const std::string &value) : type_(Type::String) { + TypedValue(const std::string &value, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::String) { new (&string_v) std::string(value); } - TypedValue(const char *value) : type_(Type::String) { + + TypedValue(const char *value, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::String) { new (&string_v) std::string(value); } - TypedValue(const std::vector &value) : type_(Type::List) { + + TypedValue(const std::vector &value, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::List) { new (&list_v) std::vector(value); } - TypedValue(const std::map &value) - : type_(Type::Map) { + + TypedValue(const std::map &value, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::Map) { new (&map_v) std::map(value); } - TypedValue(const VertexAccessor &vertex) : type_(Type::Vertex) { + + TypedValue(const VertexAccessor &vertex, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::Vertex) { new (&vertex_v) VertexAccessor(vertex); } - TypedValue(const EdgeAccessor &edge) : type_(Type::Edge) { + + TypedValue(const EdgeAccessor &edge, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::Edge) { new (&edge_v) EdgeAccessor(edge); } - TypedValue(const Path &path) : type_(Type::Path) { new (&path_v) Path(path); } + + TypedValue(const Path &path, + utils::MemoryResource *memory = utils::NewDeleteResource()) + : memory_(memory), type_(Type::Path) { + new (&path_v) Path(path); + } + + /** Construct a copy using default utils::NewDeleteResource() */ TypedValue(const PropertyValue &value); + /** Construct a copy using the given utils::MemoryResource */ + TypedValue(const PropertyValue &value, utils::MemoryResource *memory); + // move constructors for non-primitive types TypedValue(std::string &&value) noexcept : type_(Type::String) { new (&string_v) std::string(std::move(value)); } + + TypedValue(std::string &&value, utils::MemoryResource *memory) noexcept + : memory_(memory), type_(Type::String) { + new (&string_v) std::string(std::move(value)); + } + TypedValue(std::vector &&value) noexcept : type_(Type::List) { new (&list_v) std::vector(std::move(value)); } + + TypedValue(std::vector &&value, + utils::MemoryResource *memory) noexcept + : memory_(memory), type_(Type::List) { + new (&list_v) std::vector(std::move(value)); + } + TypedValue(std::map &&value) noexcept : type_(Type::Map) { new (&map_v) std::map(std::move(value)); } + + TypedValue(std::map &&value, + utils::MemoryResource *memory) noexcept + : memory_(memory), type_(Type::Map) { + new (&map_v) std::map(std::move(value)); + } + TypedValue(VertexAccessor &&vertex) noexcept : type_(Type::Vertex) { new (&vertex_v) VertexAccessor(std::move(vertex)); } + + TypedValue(VertexAccessor &&vertex, utils::MemoryResource *memory) noexcept + : memory_(memory), type_(Type::Vertex) { + new (&vertex_v) VertexAccessor(std::move(vertex)); + } + TypedValue(EdgeAccessor &&edge) noexcept : type_(Type::Edge) { new (&edge_v) EdgeAccessor(std::move(edge)); } + + TypedValue(EdgeAccessor &&edge, utils::MemoryResource *memory) noexcept + : memory_(memory), type_(Type::Edge) { + new (&edge_v) EdgeAccessor(std::move(edge)); + } + TypedValue(Path &&path) noexcept : type_(Type::Path) { new (&path_v) Path(std::move(path)); } - TypedValue(PropertyValue &&value) noexcept; + + TypedValue(Path &&path, utils::MemoryResource *memory) noexcept + : memory_(memory), type_(Type::Path) { + new (&path_v) Path(std::move(path)); + } + + /** + * Construct with the value of other. + * Default utils::NewDeleteResource() is used for allocations. After the move, + * other will be set to Null. + */ + // NOLINTNEXTLINE(hicpp-explicit-conversions) + TypedValue(PropertyValue &&other); + + /** + * Construct with the value of other, but use the given utils::MemoryResource. + * After the move, other will be set to Null. + */ + TypedValue(PropertyValue &&other, utils::MemoryResource *memory); // copy assignment operators TypedValue &operator=(const char *); @@ -146,7 +274,9 @@ class TypedValue { TypedValue &operator=(const VertexAccessor &); TypedValue &operator=(const EdgeAccessor &); TypedValue &operator=(const Path &); - TypedValue &operator=(const TypedValue &); + + /** Copy assign other, utils::MemoryResource of `this` is used */ + TypedValue &operator=(const TypedValue &other); // move assignment operators TypedValue &operator=(std::string &&) noexcept; @@ -155,7 +285,9 @@ class TypedValue { TypedValue &operator=(VertexAccessor &&) noexcept; TypedValue &operator=(EdgeAccessor &&) noexcept; TypedValue &operator=(Path &&) noexcept; - TypedValue &operator=(TypedValue &&) noexcept; + + /** Move assign other, utils::MemoryResource of `this` is used. */ + TypedValue &operator=(TypedValue &&other) noexcept; ~TypedValue(); @@ -174,7 +306,7 @@ class TypedValue { template const T &Value() const; -// TODO consider adding getters for primitives by value (and not by ref) + // TODO consider adding getters for primitives by value (and not by ref) #define DECLARE_VALUE_AND_TYPE_GETTERS(type_param, field) \ /** Gets the value of type field. Throws if value is not field*/ \ @@ -207,11 +339,14 @@ class TypedValue { * PropertyValue */ bool IsPropertyValue() const; - friend std::ostream &operator<<(std::ostream &stream, const TypedValue &prop); + utils::MemoryResource *GetMemoryResource() const { return memory_; } private: void DestroyValue(); + // Memory resource for allocations of non primitive values + utils::MemoryResource *memory_{utils::NewDeleteResource()}; + // storage for the value of the property union { bool bool_v; @@ -248,45 +383,209 @@ class TypedValueException : public utils::BasicException { using utils::BasicException::BasicException; }; -// comparison operators -// they return TypedValue because Null can be returned -TypedValue operator==(const TypedValue &a, const TypedValue &b); -TypedValue operator<(const TypedValue &a, const TypedValue &b); +// binary bool operators + +/** + * Perform logical 'and' on TypedValues. + * + * If any of the values is false, return false. Otherwise checks if any value is + * Null and return Null. All other cases return true. The resulting value uses + * the same MemoryResource as the left hand side arguments. + * + * @throw TypedValueException if arguments are not boolean or Null. + */ +TypedValue operator&&(const TypedValue &a, const TypedValue &b); + +/** + * Perform logical 'or' on TypedValues. + * + * If any of the values is true, return true. Otherwise checks if any value is + * Null and return Null. All other cases return false. The resulting value uses + * the same MemoryResource as the left hand side arguments. + * + * @throw TypedValueException if arguments are not boolean or Null. + */ +TypedValue operator||(const TypedValue &a, const TypedValue &b); + +/** + * Logically negate a TypedValue. + * + * Negating Null value returns Null. Values other than null raise an exception. + * The resulting value uses the same MemoryResource as the argument. + * + * @throw TypedValueException if TypedValue is not a boolean or Null. + */ TypedValue operator!(const TypedValue &a); -// arithmetic operators -TypedValue operator-(const TypedValue &a); -TypedValue operator+(const TypedValue &a); -TypedValue operator+(const TypedValue &a, const TypedValue &b); -TypedValue operator-(const TypedValue &a, const TypedValue &b); -TypedValue operator/(const TypedValue &a, const TypedValue &b); -TypedValue operator*(const TypedValue &a, const TypedValue &b); -TypedValue operator%(const TypedValue &a, const TypedValue &b); - -// binary bool operators -TypedValue operator&&(const TypedValue &a, const TypedValue &b); -TypedValue operator||(const TypedValue &a, const TypedValue &b); // binary bool xor, not power operator // Be careful: since ^ is binary operator and || and && are logical operators // they have different priority in c++. TypedValue operator^(const TypedValue &a, const TypedValue &b); -// stream output -std::ostream &operator<<(std::ostream &os, const TypedValue::Type type); +// comparison operators + +/** + * Compare TypedValues and return true, false or Null. + * + * Null is returned if either of the two values is Null. + * Since each TypedValue may have a different MemoryResource for allocations, + * the results is allocated using MemoryResource obtained from the left hand + * side. + */ +TypedValue operator==(const TypedValue &a, const TypedValue &b); + +/** + * Compare TypedValues and return true, false or Null. + * + * Null is returned if either of the two values is Null. + * Since each TypedValue may have a different MemoryResource for allocations, + * the results is allocated using MemoryResource obtained from the left hand + * side. + */ inline TypedValue operator!=(const TypedValue &a, const TypedValue &b) { return !(a == b); } +/** + * Compare TypedValues and return true, false or Null. + * + * Null is returned if either of the two values is Null. + * The resulting value uses the same MemoryResource as the left hand side + * argument. + * + * @throw TypedValueException if the values cannot be compared, i.e. they are + * not either Null, numeric or a character string type. + */ +TypedValue operator<(const TypedValue &a, const TypedValue &b); + +/** + * Compare TypedValues and return true, false or Null. + * + * Null is returned if either of the two values is Null. + * The resulting value uses the same MemoryResource as the left hand side + * argument. + * + * @throw TypedValueException if the values cannot be compared, i.e. they are + * not either Null, numeric or a character string type. + */ inline TypedValue operator<=(const TypedValue &a, const TypedValue &b) { return a < b || a == b; } +/** + * Compare TypedValues and return true, false or Null. + * + * Null is returned if either of the two values is Null. + * The resulting value uses the same MemoryResource as the left hand side + * argument. + * + * @throw TypedValueException if the values cannot be compared, i.e. they are + * not either Null, numeric or a character string type. + */ inline TypedValue operator>(const TypedValue &a, const TypedValue &b) { return !(a <= b); } +/** + * Compare TypedValues and return true, false or Null. + * + * Null is returned if either of the two values is Null. + * The resulting value uses the same MemoryResource as the left hand side + * argument. + * + * @throw TypedValueException if the values cannot be compared, i.e. they are + * not either Null, numeric or a character string type. + */ inline TypedValue operator>=(const TypedValue &a, const TypedValue &b) { return !(a < b); } +// arithmetic operators + +/** + * Arithmetically negate a value. + * + * If the value is Null, then Null is returned. + * The resulting value uses the same MemoryResource as the argument. + * + * @throw TypedValueException if the value is not numeric or Null. + */ +TypedValue operator-(const TypedValue &a); + +/** + * Apply the unary plus operator to a value. + * + * If the value is Null, then Null is returned. + * The resulting value uses the same MemoryResource as the argument. + * + * @throw TypedValueException if the value is not numeric or Null. + */ +TypedValue operator+(const TypedValue &a); + +/** + * Perform addition or concatenation on two values. + * + * Numeric values are summed, while lists and character strings are + * concatenated. If either value is Null, then Null is returned. The resulting + * value uses the same MemoryResource as the left hand side argument. + * + * @throw TypedValueException if values cannot be summed or concatenated. + */ +TypedValue operator+(const TypedValue &a, const TypedValue &b); + +/** + * Subtract two values. + * + * If any of the values is Null, then Null is returned. + * The resulting value uses the same MemoryResource as the left hand side + * argument. + * + * @throw TypedValueException if the values are not numeric or Null. + */ +TypedValue operator-(const TypedValue &a, const TypedValue &b); + +/** + * Divide two values. + * + * If any of the values is Null, then Null is returned. + * The resulting value uses the same MemoryResource as the left hand side + * argument. + * + * @throw TypedValueException if the values are not numeric or Null, or if + * dividing two integer values by zero. + */ +TypedValue operator/(const TypedValue &a, const TypedValue &b); + +/** + * Multiply two values. + * + * If any of the values is Null, then Null is returned. + * The resulting value uses the same MemoryResource as the left hand side + * argument. + * + * @throw TypedValueException if the values are not numeric or Null. + */ +TypedValue operator*(const TypedValue &a, const TypedValue &b); + +/** + * Perform modulo operation on two values. + * + * If any of the values is Null, then Null is returned. + * The resulting value uses the same MemoryResource as the left hand side + * argument. + * + * @throw TypedValueException if the values are not numeric or Null. + */ +TypedValue operator%(const TypedValue &a, const TypedValue &b); + +/** Output the TypedValue::Type value as a string */ +std::ostream &operator<<(std::ostream &os, const TypedValue::Type &type); + +/** + * Output the TypedValue value as a readable string. + * Note that the primary use of this is for debugging and may not yield the + * pretty results you want to display to the user. + */ +std::ostream &operator<<(std::ostream &os, const TypedValue &value); + } // namespace query diff --git a/tests/unit/typed_value.cpp b/tests/unit/typed_value.cpp index 2f9be8437..ddebba119 100644 --- a/tests/unit/typed_value.cpp +++ b/tests/unit/typed_value.cpp @@ -429,3 +429,34 @@ TEST_F(TypedValueLogicTest, LogicalXor) { EXPECT_PROP_EQ(TypedValue(true) ^ TypedValue(false), TypedValue(true)); EXPECT_PROP_EQ(TypedValue(false) ^ TypedValue(false), TypedValue(false)); } + +// NOLINTNEXTLINE(hicpp-special-member-functions) +TEST_F(AllTypesFixture, ConstructionWithMemoryResource) { + std::vector values_with_custom_memory; + utils::MonotonicBufferResource monotonic_memory(1024); + for (const auto &value : values_) { + EXPECT_EQ(value.GetMemoryResource(), utils::NewDeleteResource()); + TypedValue copy_constructed_value(value, &monotonic_memory); + EXPECT_EQ(copy_constructed_value.GetMemoryResource(), &monotonic_memory); + values_with_custom_memory.emplace_back(std::move(copy_constructed_value)); + const auto &move_constructed_value = values_with_custom_memory.back(); + EXPECT_EQ(move_constructed_value.GetMemoryResource(), &monotonic_memory); + } +} + +// NOLINTNEXTLINE(hicpp-special-member-functions) +TEST_F(AllTypesFixture, AssignmentWithMemoryResource) { + std::vector values_with_default_memory; + utils::MonotonicBufferResource monotonic_memory(1024); + for (const auto &value : values_) { + EXPECT_EQ(value.GetMemoryResource(), utils::NewDeleteResource()); + TypedValue copy_assigned_value(&monotonic_memory); + copy_assigned_value = value; + EXPECT_EQ(copy_assigned_value.GetMemoryResource(), &monotonic_memory); + values_with_default_memory.emplace_back(utils::NewDeleteResource()); + auto &move_assigned_value = values_with_default_memory.back(); + move_assigned_value = std::move(copy_assigned_value); + EXPECT_EQ(move_assigned_value.GetMemoryResource(), + utils::NewDeleteResource()); + } +}