summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlexander Shalamov <alexander.shalamov@mapbox.com>2019-04-05 17:22:55 +0300
committerFabian Guerra <fabian.guerra@mapbox.com>2019-04-05 13:08:09 -0700
commit32a06cad1df9f94aa14cf9c8ae7a86989a464b87 (patch)
tree9eafc5a20b523d80b95d96b8da39f20dfeaa4362
parentfab134ba361900b11a7f51adc00621ece912b2df (diff)
downloadqtlocation-mapboxgl-32a06cad1df9f94aa14cf9c8ae7a86989a464b87.tar.gz
[core] Move should not call destructorsupstream/fabian-cp-14352
-rw-r--r--include/mbgl/style/conversion_impl.hpp25
-rw-r--r--test/style/conversion/conversion_impl.test.cpp100
-rw-r--r--test/test-files.json1
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",