Deinline large destructors and clean up some code based on clion hints.

8 files changed, 65 insertions, 43 deletions

diff --git a/storage/src/tests/distributor/CMakeLists.txt b/storage/src/tests/distributor/CMakeLists.txt
index c8dde75c73d..c59b56eb68f 100644
--- a/storage/src/tests/distributor/CMakeLists.txt
+++ b/storage/src/tests/distributor/CMakeLists.txt
@@ -22,6 +22,7 @@ vespa_add_executable(storage_distributor_gtest_runner_app TEST
     gtest_runner.cpp
     idealstatemanagertest.cpp
     joinbuckettest.cpp
+    maintenancemocks.cpp
     maintenanceschedulertest.cpp
     mergelimitertest.cpp
     mergeoperationtest.cpp
diff --git a/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp b/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp
index a02e98a93b8..6dfab5abc21 100644
--- a/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp
+++ b/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp
@@ -21,14 +21,9 @@ using BucketSpacesStats = BucketSpacesStatsProvider::BucketSpacesStats;
 using namespace ::testing;
 
 struct DistributorHostInfoReporterTest : Test {
-    void verifyBucketSpaceStats(const vespalib::Slime& root,
-                                uint16_t nodeIndex,
-                                const vespalib::string& bucketSpaceName,
-                                size_t bucketsTotal,
-                                size_t bucketsPending);
-    void verifyBucketSpaceStats(const vespalib::Slime& root,
-                                uint16_t nodeIndex,
-                                const vespalib::string& bucketSpaceName);
+    static void verifyBucketSpaceStats(const vespalib::Slime& root, uint16_t nodeIndex, const vespalib::string& bucketSpaceName,
+                                       size_t bucketsTotal, size_t bucketsPending);
+    static void verifyBucketSpaceStats(const vespalib::Slime& root, uint16_t nodeIndex, const vespalib::string& bucketSpaceName);
 };
 
 using ms = std::chrono::milliseconds;
@@ -40,19 +35,26 @@ struct MockedMinReplicaProvider : MinReplicaProvider
 {
     MinReplicaStats minReplica;
 
+    ~MockedMinReplicaProvider() override;
     std::unordered_map<uint16_t, uint32_t> getMinReplica() const override {
         return minReplica;
     }
 };
 
+MockedMinReplicaProvider::~MockedMinReplicaProvider() = default;
+
+
 struct MockedBucketSpacesStatsProvider : public BucketSpacesStatsProvider {
     PerNodeBucketSpacesStats stats;
 
+    ~MockedBucketSpacesStatsProvider() override;
     PerNodeBucketSpacesStats getBucketSpacesStats() const override {
         return stats;
     }
 };
 
+MockedBucketSpacesStatsProvider::~MockedBucketSpacesStatsProvider() = default;
+
 const vespalib::slime::Inspector&
 getNode(const vespalib::Slime& root, uint16_t nodeIndex)
 {
diff --git a/storage/src/tests/distributor/maintenancemocks.cpp b/storage/src/tests/distributor/maintenancemocks.cpp
new file mode 100644
index 00000000000..e942a3889c1
--- /dev/null
+++ b/storage/src/tests/distributor/maintenancemocks.cpp
@@ -0,0 +1,13 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "maintenancemocks.h"
+
+namespace storage::distributor {
+
+MockOperationStarter::MockOperationStarter() noexcept
+    : _shouldStart(true)
+{}
+
+MockOperationStarter::~MockOperationStarter() = default;
+
+}
diff --git a/storage/src/tests/distributor/maintenancemocks.h b/storage/src/tests/distributor/maintenancemocks.h
index 5bcd8f962a6..0698c5e096b 100644
--- a/storage/src/tests/distributor/maintenancemocks.h
+++ b/storage/src/tests/distributor/maintenancemocks.h
@@ -23,9 +23,7 @@ class MockMaintenancePriorityGenerator
     {
         stats.incMovingOut(1, makeBucketSpace());
         stats.incCopyingIn(2, makeBucketSpace());
-        return MaintenancePriorityAndType(
-                MaintenancePriority(MaintenancePriority::VERY_HIGH),
-                MaintenanceOperation::MERGE_BUCKET);
+        return { MaintenancePriority(MaintenancePriority::VERY_HIGH), MaintenanceOperation::MERGE_BUCKET };
     }
 };
 
@@ -38,41 +36,41 @@ class MockOperation : public MaintenanceOperation
     bool _was_blocked;
     bool _was_throttled;
 public:
-    MockOperation(const document::Bucket &bucket)
+    explicit MockOperation(const document::Bucket &bucket)
         : _bucket(bucket),
           _shouldBlock(false),
           _was_blocked(false),
           _was_throttled(false)
     {}
 
-    std::string toString() const override {
+    [[nodiscard]] std::string toString() const override {
         return _bucket.toString();
     }
 
     void onClose(DistributorStripeMessageSender&) override {}
-    const char* getName() const noexcept override { return "MockOperation"; }
-    const std::string& getDetailedReason() const override {
+    [[nodiscard]] const char* getName() const noexcept override { return "MockOperation"; }
+    [[nodiscard]] const std::string& getDetailedReason() const override {
         return _reason;
     }
     void onStart(DistributorStripeMessageSender&) override {}
     void onReceive(DistributorStripeMessageSender&, const std::shared_ptr<api::StorageReply>&) override {}
     void on_blocked() override { _was_blocked = true; }
     void on_throttled() override { _was_throttled = true; }
-    bool isBlocked(const DistributorStripeOperationContext&, const OperationSequencer&) const override {
+    [[nodiscard]] bool isBlocked(const DistributorStripeOperationContext&, const OperationSequencer&) const override {
         return _shouldBlock;
     }
     void setShouldBlock(bool shouldBlock) {
         _shouldBlock = shouldBlock;
     }
-    bool get_was_blocked() const noexcept { return _was_blocked; }
-    bool get_was_throttled() const noexcept { return _was_throttled; }
+    [[nodiscard]] bool get_was_blocked() const noexcept { return _was_blocked; }
+    [[nodiscard]] bool get_was_throttled() const noexcept { return _was_throttled; }
 };
 
 class MockMaintenanceOperationGenerator
     : public MaintenanceOperationGenerator
 {
 public:
-    MaintenanceOperation::SP generate(const document::Bucket&bucket) const override {
+    [[nodiscard]] MaintenanceOperation::SP generate(const document::Bucket&bucket) const override {
         return std::make_shared<MockOperation>(bucket);
     }
 
@@ -95,9 +93,8 @@ class MockOperationStarter
     std::vector<Operation::SP> _operations;
     bool _shouldStart;
 public:
-    MockOperationStarter()
-        : _shouldStart(true)
-    {}
+    MockOperationStarter() noexcept;
+    ~MockOperationStarter() override;
 
     bool start(const std::shared_ptr<Operation>& operation, Priority priority) override
     {
@@ -130,7 +127,7 @@ public:
         _allow = allow;
     }
 
-    bool may_allow_operation_with_priority(OperationStarter::Priority) const noexcept override {
+    [[nodiscard]] bool may_allow_operation_with_priority(OperationStarter::Priority) const noexcept override {
         return _allow;
     }
 };
diff --git a/storage/src/tests/storageserver/service_layer_error_listener_test.cpp b/storage/src/tests/storageserver/service_layer_error_listener_test.cpp
index 571bebd9c86..11ecc11f810 100644
--- a/storage/src/tests/storageserver/service_layer_error_listener_test.cpp
+++ b/storage/src/tests/storageserver/service_layer_error_listener_test.cpp
@@ -41,9 +41,11 @@ struct Fixture {
     TestShutdownListener shutdown_listener;
     ServiceLayerErrorListener error_listener{component, merge_throttler};
 
+    Fixture();
     ~Fixture();
 };
 
+Fixture::Fixture() = default;
 Fixture::~Fixture() = default;
 
 }
diff --git a/storage/src/tests/storageserver/statemanagertest.cpp b/storage/src/tests/storageserver/statemanagertest.cpp
index d7fc04ebe8c..5764460f5bb 100644
--- a/storage/src/tests/storageserver/statemanagertest.cpp
+++ b/storage/src/tests/storageserver/statemanagertest.cpp
@@ -5,7 +5,6 @@
 #include <vespa/storageapi/message/state.h>
 #include <vespa/vdslib/state/cluster_state_bundle.h>
 #include <vespa/vdslib/state/clusterstate.h>
-#include <vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h>
 #include <vespa/storage/storageserver/statemanager.h>
 #include <tests/common/teststorageapp.h>
 #include <tests/common/testhelper.h>
@@ -159,9 +158,8 @@ struct MyStateListener : public StateListener {
     lib::NodeState current;
     std::ostringstream ost;
 
-    MyStateListener(const NodeStateUpdater& upd)
-        : updater(upd), current(*updater.getReportedNodeState()) {}
-    ~MyStateListener() override = default;
+    MyStateListener(const NodeStateUpdater& upd);
+    ~MyStateListener() override;
 
     void handleNewState() noexcept override {
         ost << current << " -> ";
@@ -169,6 +167,11 @@ struct MyStateListener : public StateListener {
         ost << current << "\n";
     }
 };
+
+MyStateListener::MyStateListener(const NodeStateUpdater& upd)
+    : updater(upd), current(*updater.getReportedNodeState())
+{}
+MyStateListener::~MyStateListener() = default;
 }
 
 TEST_F(StateManagerTest, reported_node_state) {
diff --git a/storage/src/tests/storageserver/statereportertest.cpp b/storage/src/tests/storageserver/statereportertest.cpp
index d7c5bdbf6ea..47d70cf436e 100644
--- a/storage/src/tests/storageserver/statereportertest.cpp
+++ b/storage/src/tests/storageserver/statereportertest.cpp
@@ -42,10 +42,10 @@ struct StateReporterTest : Test {
     std::shared_ptr<FileStorMetrics> _filestorMetrics;
 
     StateReporterTest();
+    ~StateReporterTest() override;
 
     void SetUp() override;
     void TearDown() override;
-    void runLoad(uint32_t count = 1);
 };
 
 namespace {
@@ -68,6 +68,8 @@ StateReporterTest::StateReporterTest()
 {
 }
 
+StateReporterTest::~StateReporterTest() = default;
+
 void StateReporterTest::SetUp() {
     _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, "statereportertest"));
     assert(system(("rm -rf " + getRootFolder(*_config)).c_str()) == 0);
diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp
index 7dc34bb4b6f..f3a538b7832 100644
--- a/storage/src/tests/visiting/visitortest.cpp
+++ b/storage/src/tests/visiting/visitortest.cpp
@@ -65,7 +65,8 @@ struct VisitorTest : Test {
     DummyStorageLink* _bottom;
     VisitorManager* _manager;
 
-    VisitorTest() : _node() {}
+    VisitorTest();
+    ~VisitorTest() override;
 
     // Not using setUp since can't throw exception out of it.
     void initializeTest(const TestParams& params = TestParams());
@@ -138,6 +139,9 @@ protected:
 
 uint32_t VisitorTest::docCount = 10;
 
+VisitorTest::VisitorTest() = default;
+VisitorTest::~VisitorTest() = default;
+
 void
 VisitorTest::initializeTest(const TestParams& params)
 {
@@ -157,18 +161,16 @@ VisitorTest::initializeTest(const TestParams& params)
     std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d0", rootFolder.c_str())));
     std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d1", rootFolder.c_str())));
 
-    _messageSessionFactory.reset(
-            new TestVisitorMessageSessionFactory(config.getConfigId()));
+    _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>(config.getConfigId());
     if (params._autoReplyError.getCode() != mbus::ErrorCode::NONE) {
         _messageSessionFactory->_autoReplyError = params._autoReplyError;
         _messageSessionFactory->_createAutoReplyVisitorSessions = true;
     }
-    _node.reset(new TestServiceLayerApp(config.getConfigId()));
-    _top.reset(new DummyStorageLink());
+    _node = std::make_unique<TestServiceLayerApp>(config.getConfigId());
+    _top = std::make_unique<DummyStorageLink>();
     _top->push_back(std::unique_ptr<StorageLink>(_manager
-            = new VisitorManager(
-                    config::ConfigUri(config.getConfigId()),
-                _node->getComponentRegister(), *_messageSessionFactory)));
+            = new VisitorManager(config::ConfigUri(config.getConfigId()),
+                                 _node->getComponentRegister(), *_messageSessionFactory)));
     _bottom = new DummyStorageLink();
     _top->push_back(std::unique_ptr<StorageLink>(_bottom));
     _manager->setTimeBetweenTicks(10);
@@ -296,13 +298,13 @@ VisitorTest::getMessagesAndReply(
 
             switch (msg->getType()) {
             case documentapi::DocumentProtocol::MESSAGE_PUTDOCUMENT:
-                docs.push_back(static_cast<documentapi::PutDocumentMessage&>(*msg).getDocumentSP());
+                docs.push_back(dynamic_cast<documentapi::PutDocumentMessage&>(*msg).getDocumentSP());
                 break;
             case documentapi::DocumentProtocol::MESSAGE_REMOVEDOCUMENT:
-                docIds.push_back(static_cast<documentapi::RemoveDocumentMessage&>(*msg).getDocumentId());
+                docIds.push_back(dynamic_cast<documentapi::RemoveDocumentMessage&>(*msg).getDocumentId());
                 break;
             case documentapi::DocumentProtocol::MESSAGE_VISITORINFO:
-                infoMessages.push_back(static_cast<documentapi::VisitorInfoMessage&>(*msg).getErrorMessage());
+                infoMessages.push_back(dynamic_cast<documentapi::VisitorInfoMessage&>(*msg).getErrorMessage());
                 break;
             default:
                 break;
@@ -357,10 +359,10 @@ VisitorTest::verifyCreateVisitorReply(
 uint32_t
 VisitorTest::getMatchingDocuments(std::vector<Document::SP >& docs) {
     uint32_t equalCount = 0;
-    for (uint32_t i=0; i<docs.size(); ++i) {
-        for (uint32_t j=0; j<_documents.size(); ++j) {
-            if (*docs[i] == *_documents[j] &&
-                docs[i]->getId() == _documents[j]->getId())
+    for (auto & doc : docs) {
+        for (auto & _document : _documents) {
+            if (*doc == *_document &&
+                doc->getId() == _document->getId())
             {
                 equalCount++;
             }