diff options
author | Alexander Shalamov <alexander.shalamov@mapbox.com> | 2019-04-05 17:22:55 +0300 |
---|---|---|
committer | Fabian Guerra Soto <fabian.guerra@mapbox.com> | 2019-04-05 14:36:37 -0700 |
commit | 7dbd1e91a2fb207195c0b37fc47bda0831034a06 (patch) | |
tree | 9eafc5a20b523d80b95d96b8da39f20dfeaa4362 | |
parent | fab134ba361900b11a7f51adc00621ece912b2df (diff) | |
download | qtlocation-mapboxgl-7dbd1e91a2fb207195c0b37fc47bda0831034a06.tar.gz |
[core] Move should not call destructors
-rw-r--r-- | include/mbgl/style/conversion_impl.hpp | 25 | ||||
-rw-r--r-- | test/style/conversion/conversion_impl.test.cpp | 100 | ||||
-rw-r--r-- | test/test-files.json | 1 |
3 files changed, 108 insertions, 18 deletions
diff --git a/include/mbgl/style/conversion_impl.hpp b/include/mbgl/style/conversion_impl.hpp index 27b2ee1917..0551187f6a 100644 --- a/include/mbgl/style/conversion_impl.hpp +++ b/include/mbgl/style/conversion_impl.hpp @@ -89,29 +89,20 @@ public: new (static_cast<void*>(&storage)) std::decay_t<T>(std::forward<T>(value)); } - Convertible(Convertible&& v) - : vtable(v.vtable) - { - if (vtable) { - vtable->move(std::move(v.storage), this->storage); - } + Convertible(Convertible&& v) : vtable(v.vtable) { + vtable->move(std::move(v.storage), storage); } ~Convertible() { - if (vtable) { - vtable->destroy(storage); - } + vtable->destroy(storage); } Convertible& operator=(Convertible&& v) { - if (vtable) { + if (this != &v) { vtable->destroy(storage); + vtable = v.vtable; + vtable->move(std::move(v.storage), storage); } - vtable = v.vtable; - if (vtable) { - vtable->move(std::move(v.storage), this->storage); - } - v.vtable = nullptr; return *this; } @@ -235,9 +226,7 @@ private: using Traits = ConversionTraits<T>; static VTable vtable = { [] (Storage&& src, Storage& dest) { - auto srcValue = reinterpret_cast<T&&>(src); - new (static_cast<void*>(&dest)) T(std::move(srcValue)); - srcValue.~T(); + new (static_cast<void*>(&dest)) T(reinterpret_cast<T&&>(src)); }, [] (Storage& s) { reinterpret_cast<T&>(s).~T(); diff --git a/test/style/conversion/conversion_impl.test.cpp b/test/style/conversion/conversion_impl.test.cpp new file mode 100644 index 0000000000..633d77886f --- /dev/null +++ b/test/style/conversion/conversion_impl.test.cpp @@ -0,0 +1,100 @@ +#include <mbgl/test/util.hpp> +#include <mbgl/style/conversion_impl.hpp> + +namespace mbgl { +namespace style { +namespace conversion { + +class MockConvertible { +public: + MockConvertible() = default; + MockConvertible(int* counter_) : counter(counter_) {} + + MockConvertible(MockConvertible&& other) noexcept : counter(other.counter) {} + + ~MockConvertible() { + ++*counter; + } + + int* counter = nullptr; +}; + +template <> +class ConversionTraits<MockConvertible> { +public: + static bool isUndefined(const MockConvertible&) { + return false; + } + + static bool isArray(const MockConvertible&) { + return false; + } + + static bool isObject(const MockConvertible&) { + return false; + } + + static std::size_t arrayLength(const MockConvertible&) { + return 0u; + } + + static MockConvertible arrayMember(const MockConvertible&, std::size_t) { + return {}; + } + + static optional<MockConvertible> objectMember(const MockConvertible&, const char *) { + return nullopt; + } + + template <class Fn> + static optional<Error> eachMember(const MockConvertible&, Fn&&) { + return nullopt; + } + + static optional<bool> toBool(const MockConvertible&) { + return nullopt; + } + + static optional<float> toNumber(const MockConvertible&) { + return nullopt; + } + + static optional<double> toDouble(const MockConvertible&) { + return nullopt; + } + + static optional<std::string> toString(const MockConvertible&) { + return nullopt; + } + + static optional<mbgl::Value> toValue(const MockConvertible&) { + return nullopt; + } + + static optional<GeoJSON> toGeoJSON(const MockConvertible&, Error&) { + return nullopt; + } +}; + +} // namespace conversion +} // namespace style +} // namespace mbgl + +using namespace mbgl; +using namespace mbgl::style; +using namespace mbgl::style::conversion; + +TEST(Conversion, Move) { + int dtorCounter = 0; + + { + MockConvertible mock(&dtorCounter); + Convertible a(std::move(mock)); + Convertible b(std::move(a)); + a = std::move(b); + Convertible* c = &a; + a = std::move(*c); + } + + ASSERT_EQ(dtorCounter, 4); +} diff --git a/test/test-files.json b/test/test-files.json index 498237e072..f958fba0d3 100644 --- a/test/test-files.json +++ b/test/test-files.json @@ -49,6 +49,7 @@ "test/storage/online_file_source.test.cpp", "test/storage/resource.test.cpp", "test/storage/sqlite.test.cpp", + "test/style/conversion/conversion_impl.test.cpp", "test/style/conversion/function.test.cpp", "test/style/conversion/geojson_options.test.cpp", "test/style/conversion/layer.test.cpp", |