Merge pull request #23933 from vespa-engine/balder/some-c++-modernisation-1

Some c++ cleanup while reading code.

14 files changed, 68 insertions, 59 deletions

diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp
index 800cb8aa0ce..e5314d1bf5d 100644
--- a/searchcore/src/tests/proton/attribute/attribute_test.cpp
+++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp
@@ -149,7 +149,7 @@ public:
     {
         setup(1);
     }
-    ~AttributeWriterTest();
+    ~AttributeWriterTest() override;
     void setup(uint32_t threads) {
         _aw.reset();
         _attributeFieldWriterReal = std::make_unique<ForegroundTaskExecutor>(threads);
diff --git a/searchcore/src/tests/proton/matching/match_phase_limiter/match_phase_limiter_test.cpp b/searchcore/src/tests/proton/matching/match_phase_limiter/match_phase_limiter_test.cpp
index 73771d700b4..5b3b9f962f7 100644
--- a/searchcore/src/tests/proton/matching/match_phase_limiter/match_phase_limiter_test.cpp
+++ b/searchcore/src/tests/proton/matching/match_phase_limiter/match_phase_limiter_test.cpp
@@ -41,7 +41,7 @@ struct MockSearch : SearchIterator {
     bool postings_fetched;
     uint32_t last_seek = beginId();
     uint32_t last_unpack = beginId();
-    MockSearch(const vespalib::string &term_in)
+    explicit MockSearch(const vespalib::string &term_in)
         : spec("", 0, 0), term(term_in), _strict(vespalib::Trinary::True), tfmda(), postings_fetched(false) {}
     MockSearch(const FieldSpec &spec_in, const vespalib::string &term_in, bool strict_in,
                const TermFieldMatchDataArray &tfmda_in, bool postings_fetched_in)
@@ -65,16 +65,14 @@ struct MockBlueprint : SimpleLeafBlueprint {
     {
         setEstimate(HitEstimate(756, false));
     }    
-    virtual SearchIterator::UP createLeafSearch(const TermFieldMatchDataArray &tfmda,
-                                                bool strict) const override
+    SearchIterator::UP createLeafSearch(const TermFieldMatchDataArray &tfmda, bool strict) const override
     {
         if (postings_fetched) {
             EXPECT_EQUAL(postings_strict.isStrict(), strict);
         }
-        return SearchIterator::UP(new MockSearch(spec, term, strict, tfmda,
-                                                 postings_fetched));
+        return std::make_unique<MockSearch>(spec, term, strict, tfmda, postings_fetched);
     }
-    virtual void fetchPostings(const search::queryeval::ExecuteInfo &execInfo) override {
+    void fetchPostings(const search::queryeval::ExecuteInfo &execInfo) override {
         postings_strict = execInfo;
         postings_fetched = true;
     }
@@ -88,7 +86,7 @@ struct MockSearchable : Searchable {
     {
         (void) requestContext;
         ++create_cnt;
-        return Blueprint::UP(new MockBlueprint(field, termAsString(term)));
+        return std::make_unique<MockBlueprint>(field, termAsString(term));
     }
 };
 
@@ -175,7 +173,7 @@ TEST("require that the attribute limiter works correctly") {
         EXPECT_EQUAL(1u, searchable.create_cnt);
         SearchIterator::UP s2 = limiter.create_search(42, diverse ? 3 : 42, strict);
         EXPECT_EQUAL(1u, searchable.create_cnt);
-        MockSearch *ms = dynamic_cast<MockSearch*>(s1.get());
+        auto *ms = dynamic_cast<MockSearch*>(s1.get());
         ASSERT_TRUE(ms != nullptr);
         EXPECT_EQUAL("limiter_attribute", ms->spec.getName());
         EXPECT_EQUAL(0u, ms->spec.getFieldId());
@@ -208,7 +206,7 @@ TEST("require that no limiter has no behavior") {
     SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 1.0, 100000000, nullptr);
     limiter.updateDocIdSpaceEstimate(1000, 9000);
     EXPECT_EQUAL(std::numeric_limits<size_t>::max(), limiter.getDocIdSpaceEstimate());
-    MockSearch *ms = dynamic_cast<MockSearch*>(search.get());
+    auto *ms = dynamic_cast<MockSearch*>(search.get());
     ASSERT_TRUE(ms != nullptr);
     EXPECT_EQUAL("search", ms->term);
     EXPECT_FALSE(limiter.was_limited());
@@ -226,7 +224,7 @@ TEST("require that the match phase limiter may chose not to limit the query") {
     SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.005, 100000, nullptr);
     limiter.updateDocIdSpaceEstimate(1000, 9000);
     EXPECT_EQUAL(10000u, limiter.getDocIdSpaceEstimate());
-    MockSearch *ms = dynamic_cast<MockSearch*>(search.get());
+    auto *ms = dynamic_cast<MockSearch*>(search.get());
     ASSERT_TRUE(ms != nullptr);
     EXPECT_EQUAL("search", ms->term);
     EXPECT_FALSE(limiter.was_limited());
@@ -254,7 +252,7 @@ TEST_F("require that the match phase limiter may chose not to limit the query wh
     SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.10, 1900000, nullptr);
     limiter.updateDocIdSpaceEstimate(1000, 1899000);
     EXPECT_EQUAL(1900000u, limiter.getDocIdSpaceEstimate());
-    MockSearch *ms = dynamic_cast<MockSearch *>(search.get());
+    auto *ms = dynamic_cast<MockSearch *>(search.get());
     ASSERT_TRUE(ms != nullptr);
     EXPECT_EQUAL("search", ms->term);
     EXPECT_FALSE(limiter.was_limited());
@@ -266,11 +264,11 @@ TEST_F("require that the match phase limiter may chose to limit the query even w
     SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.10, 2100000, nullptr);
     limiter.updateDocIdSpaceEstimate(1000, 2099000);
     EXPECT_EQUAL(159684u, limiter.getDocIdSpaceEstimate());
-    LimitedSearch *strict_and = dynamic_cast<LimitedSearch*>(search.get());
+    auto *strict_and = dynamic_cast<LimitedSearch*>(search.get());
     ASSERT_TRUE(strict_and != nullptr);
-    const MockSearch *ms1 = dynamic_cast<const MockSearch*>(&strict_and->getFirst());
+    const auto *ms1 = dynamic_cast<const MockSearch*>(&strict_and->getFirst());
     ASSERT_TRUE(ms1 != nullptr);
-    const MockSearch *ms2 = dynamic_cast<const MockSearch*>(&strict_and->getSecond());
+    const auto *ms2 = dynamic_cast<const MockSearch*>(&strict_and->getSecond());
     ASSERT_TRUE(ms2 != nullptr);
     EXPECT_EQUAL("[;;-100000]", ms1->term);
     EXPECT_EQUAL("search", ms2->term);
@@ -300,11 +298,11 @@ TEST("require that the match phase limiter is able to pre-limit the query") {
     SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.1, 100000, trace.maybeCreateCursor(7, "limit"));
     limiter.updateDocIdSpaceEstimate(1000, 9000);
     EXPECT_EQUAL(1680u, limiter.getDocIdSpaceEstimate());
-    LimitedSearch *strict_and = dynamic_cast<LimitedSearch*>(search.get());
+    auto *strict_and = dynamic_cast<LimitedSearch*>(search.get());
     ASSERT_TRUE(strict_and != nullptr);
-    const MockSearch *ms1 = dynamic_cast<const MockSearch*>(&strict_and->getFirst());
+    const auto *ms1 = dynamic_cast<const MockSearch*>(&strict_and->getFirst());
     ASSERT_TRUE(ms1 != nullptr);
-    const MockSearch *ms2 = dynamic_cast<const MockSearch*>(&strict_and->getSecond());
+    const auto *ms2 = dynamic_cast<const MockSearch*>(&strict_and->getSecond());
     ASSERT_TRUE(ms2 != nullptr);
     EXPECT_EQUAL("[;;-5000]", ms1->term);
     EXPECT_EQUAL("search", ms2->term);
@@ -352,11 +350,11 @@ TEST("require that the match phase limiter is able to post-limit the query") {
     SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.1, 100000, nullptr);
     limiter.updateDocIdSpaceEstimate(1000, 9000);
     EXPECT_EQUAL(1680u, limiter.getDocIdSpaceEstimate());
-    LimitedSearch *strict_and = dynamic_cast<LimitedSearch*>(search.get());
+    auto *strict_and = dynamic_cast<LimitedSearch*>(search.get());
     ASSERT_TRUE(strict_and != nullptr);
-    const MockSearch *ms1 = dynamic_cast<const MockSearch*>(&strict_and->getFirst());
+    const auto *ms1 = dynamic_cast<const MockSearch*>(&strict_and->getFirst());
     ASSERT_TRUE(ms1 != nullptr);
-    const MockSearch *ms2 = dynamic_cast<const MockSearch*>(&strict_and->getSecond());
+    const auto *ms2 = dynamic_cast<const MockSearch*>(&strict_and->getSecond());
     ASSERT_TRUE(ms2 != nullptr);
     EXPECT_EQUAL("search", ms1->term);
     EXPECT_EQUAL("[;;-15000]", ms2->term);
@@ -382,9 +380,9 @@ void verifyDiversity(AttributeLimiter::DiversityCutoffStrategy strategy)
     SearchIterator::UP search = limiter.maybe_limit(prepare(new MockSearch("search")), 0.1, 100000, nullptr);
     limiter.updateDocIdSpaceEstimate(1000, 9000);
     EXPECT_EQUAL(1680u, limiter.getDocIdSpaceEstimate());
-    LimitedSearch *strict_and = dynamic_cast<LimitedSearch*>(search.get());
+    auto *strict_and = dynamic_cast<LimitedSearch*>(search.get());
     ASSERT_TRUE(strict_and != nullptr);
-    const MockSearch *ms1 = dynamic_cast<const MockSearch*>(&strict_and->getFirst());
+    const auto *ms1 = dynamic_cast<const MockSearch*>(&strict_and->getFirst());
     ASSERT_TRUE(ms1 != nullptr);
     if (strategy == AttributeLimiter::LOOSE) {
         EXPECT_EQUAL("[;;-5000;category;500;131;loose]", ms1->term);
diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp
index aaacb971ee0..ec289c06122 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp
@@ -52,7 +52,7 @@ inject(Node::UP query, Node::UP to_inject) {
     if (auto * my_and = dynamic_cast<search::query::And *>(query.get())) {
         my_and->append(std::move(to_inject));
     } else  if (dynamic_cast<search::query::Rank *>(query.get()) || dynamic_cast<search::query::AndNot *>(query.get())) {
-        search::query::Intermediate & root = static_cast<search::query::Intermediate &>(*query);
+        auto & root = static_cast<search::query::Intermediate &>(*query);
         root.prepend(inject(root.stealFirst(), std::move(to_inject)));
     } else {
         auto new_root = std::make_unique<ProtonAnd>();
@@ -82,7 +82,7 @@ find_location_terms(Node *tree) {
     return locations;
 }
 
-GeoLocationSpec parse_location_string(string str) {
+GeoLocationSpec parse_location_string(const string & str) {
     GeoLocationSpec empty;
     if (str.empty()) {
         return empty;
diff --git a/searchcore/src/vespa/searchcore/proton/matching/querylimiter.h b/searchcore/src/vespa/searchcore/proton/matching/querylimiter.h
index 67faf74a65d..c3e6e9af475 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/querylimiter.h
+++ b/searchcore/src/vespa/searchcore/proton/matching/querylimiter.h
@@ -17,7 +17,7 @@ public:
     class Token {
     public:
         typedef std::unique_ptr<Token> UP;
-        virtual ~Token() { }
+        virtual ~Token() = default;
     };
 public:
     QueryLimiter();
@@ -31,7 +31,9 @@ private:
         QueryLimiter & _limiter;
     public:
         LimitedToken(const Doom & doom, QueryLimiter & limiter);
-        virtual ~LimitedToken();
+        LimitedToken(const NoLimitToken &) = delete;
+        LimitedToken & operator =(const NoLimitToken &) = delete;
+        ~LimitedToken() override;
     };
     void grabToken(const Doom & doom);
     void releaseToken();
diff --git a/searchcore/src/vespa/searchcore/proton/matching/querynodes.h b/searchcore/src/vespa/searchcore/proton/matching/querynodes.h
index bbe58338b12..20d602b7814 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/querynodes.h
+++ b/searchcore/src/vespa/searchcore/proton/matching/querynodes.h
@@ -61,7 +61,7 @@ public:
     ProtonTermData & operator = (const ProtonTermData &);
     ProtonTermData(ProtonTermData &&) = default;
     ProtonTermData & operator = (ProtonTermData &&) = default;
-    ~ProtonTermData();
+    ~ProtonTermData() override;
     void resolveFromChildren(const std::vector<search::query::Node *> &children);
     void allocateTerms(search::fef::MatchDataLayout &mdl);
     void setDocumentFrequency(uint32_t estHits, uint32_t numDocs);
@@ -93,9 +93,9 @@ struct ProtonTermBase : public Base,
     }
 
     // ITermData interface
-    uint32_t getPhraseLength() const override final { return numTerms<Base>(*this); }
-    search::query::Weight getWeight() const override final { return Base::getWeight(); }
-    uint32_t getUniqueId() const override final { return Base::getId(); }
+    uint32_t getPhraseLength() const final { return numTerms<Base>(*this); }
+    search::query::Weight getWeight() const final { return Base::getWeight(); }
+    uint32_t getUniqueId() const final { return Base::getId(); }
 };
 
 template <typename Base>
diff --git a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp
index 67bd0286884..29dbf33d29c 100644
--- a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp
+++ b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp
@@ -72,7 +72,7 @@ public:
     }
 
     AttributeGuard::UP getAttribute(const string &) const override {
-        return AttributeGuard::UP(new AttributeGuard(_attribute_vector));
+        return std::make_unique<AttributeGuard>(_attribute_vector);
     }
 
     std::unique_ptr<attribute::AttributeReadGuard>
diff --git a/searchlib/src/vespa/searchcommon/attribute/i_search_context.h b/searchlib/src/vespa/searchcommon/attribute/i_search_context.h
index ff62c535e7f..aa5e216ecd9 100644
--- a/searchlib/src/vespa/searchcommon/attribute/i_search_context.h
+++ b/searchlib/src/vespa/searchcommon/attribute/i_search_context.h
@@ -25,7 +25,7 @@ private:
     virtual int32_t onFind(DocId docId, int32_t elementId) const = 0;
 
 public:
-    virtual ~ISearchContext() {}
+    virtual ~ISearchContext() = default;
 
     virtual unsigned int approximateHits() const = 0;
 
diff --git a/searchlib/src/vespa/searchcommon/attribute/iattributecontext.h b/searchlib/src/vespa/searchcommon/attribute/iattributecontext.h
index bb349057ca9..9c89b6a0f8b 100644
--- a/searchlib/src/vespa/searchcommon/attribute/iattributecontext.h
+++ b/searchlib/src/vespa/searchcommon/attribute/iattributecontext.h
@@ -12,9 +12,9 @@ namespace search::attribute {
  **/
 class IAttributeContext : public IAttributeExecutor {
 public:
-    typedef vespalib::string string;
+    using string = vespalib::string;
     /** Convenience typedefs **/
-    typedef std::unique_ptr<IAttributeContext> UP;
+    using UP = std::unique_ptr<IAttributeContext>;
 
     /**
      * Returns the attribute vector with the given name.
@@ -48,7 +48,7 @@ public:
     /**
      * Virtual destructor to allow safe subclassing.
      **/
-    virtual ~IAttributeContext() {}
+    virtual ~IAttributeContext() = default;
 };
 
 }
diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp
index 5905995d586..57423f42a10 100644
--- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp
@@ -153,7 +153,7 @@ public:
                                       .diversityCutoffStrict(diversityCutoffStrict))
     {}
 
-    SearchIterator::UP createLeafSearch(const TermFieldMatchDataArray &tfmda, bool strict) const override {
+    SearchIteratorUP createLeafSearch(const TermFieldMatchDataArray &tfmda, bool strict) const override {
         assert(tfmda.size() == 1);
         return _search_context->createIterator(tfmda[0], strict);
     }
@@ -406,22 +406,7 @@ public:
         }
     }
 
-    SearchIterator::UP createLeafSearch(const TermFieldMatchDataArray &tfmda, bool) const override
-    {
-        assert(tfmda.size() == 1);
-        assert(getState().numFields() == 1);
-        if (_terms.empty()) {
-            return std::make_unique<queryeval::EmptySearch>();
-        }
-        std::vector<DocumentWeightIterator> iterators;
-        const size_t numChildren = _terms.size();
-        iterators.reserve(numChildren);
-        for (const IDocumentWeightAttribute::LookupResult &r : _terms) {
-            _attr.create(r.posting_idx, iterators);
-        }
-        bool field_is_filter = getState().fields()[0].isFilter();
-        return SearchType::create(*tfmda[0], field_is_filter, _weights, std::move(iterators));
-    }
+    SearchIterator::UP createLeafSearch(const TermFieldMatchDataArray &tfmda, bool) const override;
 
     std::unique_ptr<SearchIterator> createFilterSearch(bool strict, FilterConstraint constraint) const override;
     std::unique_ptr<queryeval::MatchingElementsSearch> create_matching_elements_search(const MatchingElementsFields &fields) const override {
@@ -434,6 +419,26 @@ public:
 };
 
 template <typename SearchType>
+SearchIterator::UP
+DirectWeightedSetBlueprint<SearchType>::createLeafSearch(const TermFieldMatchDataArray &tfmda, bool) const
+{
+    assert(tfmda.size() == 1);
+    assert(getState().numFields() == 1);
+    if (_terms.empty()) {
+        return std::make_unique<queryeval::EmptySearch>();
+    }
+    std::vector<DocumentWeightIterator> iterators;
+    const size_t numChildren = _terms.size();
+    iterators.reserve(numChildren);
+    for (const IDocumentWeightAttribute::LookupResult &r : _terms) {
+        _attr.create(r.posting_idx, iterators);
+    }
+    bool field_is_filter = getState().fields()[0].isFilter();
+    return SearchType::create(*tfmda[0], field_is_filter, _weights, std::move(iterators));
+}
+
+
+template <typename SearchType>
 DirectWeightedSetBlueprint<SearchType>::~DirectWeightedSetBlueprint() = default;
 
 template <typename SearchType>
diff --git a/searchlib/src/vespa/searchlib/common/geo_location_parser.cpp b/searchlib/src/vespa/searchlib/common/geo_location_parser.cpp
index d829e1b93e4..cb69d2346ce 100644
--- a/searchlib/src/vespa/searchlib/common/geo_location_parser.cpp
+++ b/searchlib/src/vespa/searchlib/common/geo_location_parser.cpp
@@ -2,7 +2,6 @@
 
 #include "geo_location_parser.h"
 #include <limits>
-#include <vespa/vespalib/stllike/asciistream.h>
 #include <vespa/vespalib/data/slime/slime.h>
 #include <vespa/vespalib/data/slime/json_format.h>
 
diff --git a/searchlib/src/vespa/searchlib/query/query_term_simple.cpp b/searchlib/src/vespa/searchlib/query/query_term_simple.cpp
index 17e50216a23..afd5e349da9 100644
--- a/searchlib/src/vespa/searchlib/query/query_term_simple.cpp
+++ b/searchlib/src/vespa/searchlib/query/query_term_simple.cpp
@@ -3,7 +3,6 @@
 #include "base.h"
 #include "query_term_simple.h"
 #include <vespa/vespalib/objects/visit.h>
-#include <vespa/vespalib/text/utf8.h>
 #include <vespa/vespalib/util/classname.h>
 #include <vespa/vespalib/locale/c.h>
 #include <cmath>
@@ -314,7 +313,7 @@ QueryTermSimple::getClassName() const
 void
 visit(vespalib::ObjectVisitor &self, const vespalib::string &name, const search::QueryTermSimple *obj)
 {
-    if (obj != 0) {
+    if (obj != nullptr) {
         self.openStruct(name, obj->getClassName());
         obj->visitMembers(self);
         self.closeStruct();
diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.h b/searchlib/src/vespa/searchlib/queryeval/blueprint.h
index b7fe627d854..83836f874f0 100644
--- a/searchlib/src/vespa/searchlib/queryeval/blueprint.h
+++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.h
@@ -169,7 +169,7 @@ protected:
 public:
     class IPredicate {
     public:
-        virtual ~IPredicate() {}
+        virtual ~IPredicate() = default;
         virtual bool check(const Blueprint & bp) const = 0;
     };
 
diff --git a/searchlib/src/vespa/searchlib/queryeval/fake_requestcontext.cpp b/searchlib/src/vespa/searchlib/queryeval/fake_requestcontext.cpp
index a123d3f6cd1..8b633302ff7 100644
--- a/searchlib/src/vespa/searchlib/queryeval/fake_requestcontext.cpp
+++ b/searchlib/src/vespa/searchlib/queryeval/fake_requestcontext.cpp
@@ -6,6 +6,11 @@
 
 namespace search::queryeval {
 
+FakeRequestContext::FakeRequestContext()
+    : FakeRequestContext(nullptr)
+{
+}
+
 FakeRequestContext::FakeRequestContext(attribute::IAttributeContext * context, vespalib::steady_time softDoom, vespalib::steady_time hardDoom)
     : _clock(std::make_unique<vespalib::TestClock>()),
       _doom(_clock->clock(), softDoom, hardDoom, false),
diff --git a/searchlib/src/vespa/searchlib/queryeval/fake_requestcontext.h b/searchlib/src/vespa/searchlib/queryeval/fake_requestcontext.h
index 4d7e092a812..9f829c9473c 100644
--- a/searchlib/src/vespa/searchlib/queryeval/fake_requestcontext.h
+++ b/searchlib/src/vespa/searchlib/queryeval/fake_requestcontext.h
@@ -19,10 +19,11 @@ namespace search::queryeval {
 class FakeRequestContext : public IRequestContext
 {
 public:
-    FakeRequestContext(attribute::IAttributeContext * context = nullptr,
+    FakeRequestContext();
+    FakeRequestContext(attribute::IAttributeContext * context,
                        vespalib::steady_time soft=vespalib::steady_time::max(),
                        vespalib::steady_time hard=vespalib::steady_time::max());
-    ~FakeRequestContext();
+    ~FakeRequestContext() override;
     const vespalib::Doom & getDoom() const override { return _doom; }
     const attribute::IAttributeVector *getAttribute(const vespalib::string &name) const override {
         return _attributeContext