From 2b184427781fe6c26b32318ea0c47c4ad2e14ca1 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Tue, 4 Jun 2024 11:35:49 -0700 Subject: [PATCH] Move all `WeakPtrFactory` members to the end of the containing class (#2472) --- include/mbgl/style/layer.hpp | 11 ++++++----- include/mbgl/style/sources/custom_geometry_source.hpp | 1 + include/mbgl/style/sources/geojson_source.hpp | 1 + include/mbgl/style/sources/image_source.hpp | 1 + include/mbgl/style/sources/raster_source.hpp | 1 + include/mbgl/style/sources/vector_source.hpp | 3 ++- src/mbgl/annotation/annotation_manager.hpp | 1 + src/mbgl/annotation/annotation_source.hpp | 1 + src/mbgl/sprite/sprite_loader.hpp | 1 + src/mbgl/tile/geojson_tile.hpp | 1 + src/mbgl/util/thread_pool.hpp | 4 +++- test/actor/actor.test.cpp | 4 +++- 12 files changed, 22 insertions(+), 8 deletions(-) diff --git a/include/mbgl/style/layer.hpp b/include/mbgl/style/layer.hpp index a1d1f4b3e..af75b755a 100644 --- a/include/mbgl/style/layer.hpp +++ b/include/mbgl/style/layer.hpp @@ -165,6 +165,11 @@ public: /// Collect dependencies expression::Dependency getDependencies() const noexcept; +private: + std::optional setVisibility(const conversion::Convertible& value); + std::optional setMinZoom(const conversion::Convertible& value); + std::optional setMaxZoom(const conversion::Convertible& value); + protected: virtual Mutable mutableBaseImpl() const = 0; void serializeProperty(Value&, const StyleProperty&, const char* propertyName, bool isPaint) const; @@ -172,11 +177,7 @@ protected: const conversion::Convertible& value) = 0; LayerObserver* observer; mapbox::base::WeakPtrFactory weakFactory{this}; - -private: - std::optional setVisibility(const conversion::Convertible& value); - std::optional setMinZoom(const conversion::Convertible& value); - std::optional setMaxZoom(const conversion::Convertible& value); + // Do not add members here, see `WeakPtrFactory` }; } // namespace style diff --git a/include/mbgl/style/sources/custom_geometry_source.hpp b/include/mbgl/style/sources/custom_geometry_source.hpp index 172bf300a..6dabe5e6e 100644 --- a/include/mbgl/style/sources/custom_geometry_source.hpp +++ b/include/mbgl/style/sources/custom_geometry_source.hpp @@ -57,6 +57,7 @@ private: std::shared_ptr threadPool; std::unique_ptr> loader; mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; template <> diff --git a/include/mbgl/style/sources/geojson_source.hpp b/include/mbgl/style/sources/geojson_source.hpp index 25e0e068b..273ec7d38 100644 --- a/include/mbgl/style/sources/geojson_source.hpp +++ b/include/mbgl/style/sources/geojson_source.hpp @@ -83,6 +83,7 @@ private: std::shared_ptr threadPool; std::shared_ptr sequencedScheduler; mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; template <> diff --git a/include/mbgl/style/sources/image_source.hpp b/include/mbgl/style/sources/image_source.hpp index 61429c6ee..db139ab37 100644 --- a/include/mbgl/style/sources/image_source.hpp +++ b/include/mbgl/style/sources/image_source.hpp @@ -40,6 +40,7 @@ private: std::optional url; std::unique_ptr req; mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; template <> diff --git a/include/mbgl/style/sources/raster_source.hpp b/include/mbgl/style/sources/raster_source.hpp index 11de4eadd..58002c284 100644 --- a/include/mbgl/style/sources/raster_source.hpp +++ b/include/mbgl/style/sources/raster_source.hpp @@ -39,6 +39,7 @@ private: const variant urlOrTileset; std::unique_ptr req; mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; template <> diff --git a/include/mbgl/style/sources/vector_source.hpp b/include/mbgl/style/sources/vector_source.hpp index 1b5785126..88ef1602a 100644 --- a/include/mbgl/style/sources/vector_source.hpp +++ b/include/mbgl/style/sources/vector_source.hpp @@ -44,9 +44,10 @@ protected: private: const variant urlOrTileset; std::unique_ptr req; - mapbox::base::WeakPtrFactory weakFactory{this}; std::optional maxZoom; std::optional minZoom; + mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; template <> diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp index 0bfe89546..fa3b9dbf8 100644 --- a/src/mbgl/annotation/annotation_manager.hpp +++ b/src/mbgl/annotation/annotation_manager.hpp @@ -91,6 +91,7 @@ private: std::unordered_set tiles; mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; } // namespace mbgl diff --git a/src/mbgl/annotation/annotation_source.hpp b/src/mbgl/annotation/annotation_source.hpp index 65b2a83e2..721f08ebd 100644 --- a/src/mbgl/annotation/annotation_source.hpp +++ b/src/mbgl/annotation/annotation_source.hpp @@ -20,6 +20,7 @@ private: bool supportsLayerType(const mbgl::style::LayerTypeInfo*) const override; mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; class AnnotationSource::Impl : public style::Source::Impl { diff --git a/src/mbgl/sprite/sprite_loader.hpp b/src/mbgl/sprite/sprite_loader.hpp index 2333d697f..e34a71a7c 100644 --- a/src/mbgl/sprite/sprite_loader.hpp +++ b/src/mbgl/sprite/sprite_loader.hpp @@ -41,6 +41,7 @@ private: SpriteLoaderObserver* observer = nullptr; std::shared_ptr threadPool; mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; } // namespace mbgl diff --git a/src/mbgl/tile/geojson_tile.hpp b/src/mbgl/tile/geojson_tile.hpp index 1d5dda1df..8a3898bd2 100644 --- a/src/mbgl/tile/geojson_tile.hpp +++ b/src/mbgl/tile/geojson_tile.hpp @@ -24,6 +24,7 @@ public: private: std::shared_ptr data; mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; } // namespace mbgl diff --git a/src/mbgl/util/thread_pool.hpp b/src/mbgl/util/thread_pool.hpp index ee0735082..a287e9821 100644 --- a/src/mbgl/util/thread_pool.hpp +++ b/src/mbgl/util/thread_pool.hpp @@ -94,10 +94,12 @@ public: private: std::vector threads; - mapbox::base::WeakPtrFactory weakFactory{this}; std::queue> renderThreadQueue; std::mutex renderMutex; + + mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; class SequencedScheduler : public ThreadedScheduler { diff --git a/test/actor/actor.test.cpp b/test/actor/actor.test.cpp index 681930079..abc5655f6 100644 --- a/test/actor/actor.test.cpp +++ b/test/actor/actor.test.cpp @@ -82,7 +82,6 @@ TEST(Actor, DestructionBlocksOnSend) { std::promise promise; std::future future; std::atomic waited; - mapbox::base::WeakPtrFactory weakFactory{this}; TestScheduler(std::promise promise_, std::future future_) : promise(std::move(promise_)), @@ -103,6 +102,9 @@ TEST(Actor, DestructionBlocksOnSend) { waited = true; } mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } + + mapbox::base::WeakPtrFactory weakFactory{this}; + // Do not add members here, see `WeakPtrFactory` }; struct TestActor {