Reduce use of CloneablePtr

11 files changed, 83 insertions, 75 deletions

diff --git a/document/src/vespa/document/base/fieldpath.cpp b/document/src/vespa/document/base/fieldpath.cpp
index fcac59847cb..01855af55eb 100644
--- a/document/src/vespa/document/base/fieldpath.cpp
+++ b/document/src/vespa/document/base/fieldpath.cpp
@@ -11,7 +11,6 @@ using vespalib::make_string;
 
 namespace document {
 
-FieldPathEntry::FieldPathEntry(const FieldPathEntry &) = default;
 FieldPathEntry::~FieldPathEntry() = default;
 
 FieldPathEntry::FieldPathEntry() :
@@ -46,7 +45,7 @@ FieldPathEntry::FieldPathEntry(const Field &fieldRef) :
     _lookupIndex(0),
     _lookupKey(),
     _variableName(),
-    _fillInVal(fieldRef.createValue().release())
+    _fillInVal(fieldRef.createValue())
 { }
 
 FieldPathEntry::FieldPathEntry(const DataType & dataType, const DataType& fillType,
@@ -56,13 +55,24 @@ FieldPathEntry::FieldPathEntry(const DataType & dataType, const DataType& fillTy
     _field(),
     _dataType(&dataType),
     _lookupIndex(0),
-    _lookupKey(lookupKey.release()),
+    _lookupKey(std::move(lookupKey)),
     _variableName(),
     _fillInVal()
 {
     setFillValue(fillType);
 }
 
+FieldPathEntry::FieldPathEntry(const FieldPathEntry &rhs)
+    : _type(rhs._type),
+      _name(rhs._name),
+      _field(rhs._field),
+      _dataType(rhs._dataType),
+      _lookupIndex(rhs._lookupIndex),
+      _lookupKey(rhs._lookupKey ? rhs._lookupKey->clone() : nullptr),
+      _variableName(rhs._variableName),
+      _fillInVal(rhs._fillInVal ? rhs._fillInVal->clone() : nullptr)
+{}
+
 void
 FieldPathEntry::setFillValue(const DataType & dataType)
 {
@@ -82,7 +92,7 @@ FieldPathEntry::setFillValue(const DataType & dataType)
         }
     }
     if (dt->isPrimitive()) {
-        _fillInVal.reset(dt->createFieldValue().release());
+        _fillInVal = dt->createFieldValue();
     }
 }
 
@@ -123,7 +133,7 @@ FieldPathEntry::getDataType() const
 FieldValue::UP
 FieldPathEntry::stealFieldValueToSet() const
 {
-    return FieldValue::UP(_fillInVal.release());
+    return std::move(_fillInVal);
 }
 
 vespalib::string
@@ -171,17 +181,20 @@ FieldPathEntry::parseKey(vespalib::stringref & key)
     return v;
 }
 
-FieldPath::FieldPath()
-    : _path()
-{ }
-
-FieldPath::FieldPath(const FieldPath &) = default;
-FieldPath & FieldPath::operator=(const FieldPath &) = default;
+FieldPath::FieldPath() = default;
 FieldPath::~FieldPath() = default;
 
+FieldPath::FieldPath(const FieldPath & rhs)
+    : _path()
+{
+    _path.reserve(rhs.size());
+    for (const auto & e : rhs._path) {
+        _path.emplace_back(std::make_unique<FieldPathEntry>(*e));
+    }
+}
 FieldPath::iterator
 FieldPath::insert(iterator pos, std::unique_ptr<FieldPathEntry> entry) {
-    return _path.insert(pos, vespalib::CloneablePtr<FieldPathEntry>(entry.release()));
+    return _path.insert(pos, std::move(entry));
 }
 void FieldPath::push_back(std::unique_ptr<FieldPathEntry> entry) { _path.emplace_back(entry.release()); }
 void FieldPath::pop_back() { _path.pop_back(); }
diff --git a/document/src/vespa/document/base/fieldpath.h b/document/src/vespa/document/base/fieldpath.h
index a79e4595a61..c2db3276fb4 100644
--- a/document/src/vespa/document/base/fieldpath.h
+++ b/document/src/vespa/document/base/fieldpath.h
@@ -2,7 +2,6 @@
 #pragma once
 
 #include "field.h"
-#include <vespa/vespalib/util/memory.h>
 
 namespace document {
 
@@ -23,13 +22,12 @@ public:
         VARIABLE,
         NONE
     };
-    using FieldValueCP = vespalib::CloneablePtr<FieldValue>;
-
     FieldPathEntry();
 
-    FieldPathEntry(FieldPathEntry &&) = default;
-    FieldPathEntry & operator=(FieldPathEntry &&) = default;
+    FieldPathEntry(FieldPathEntry &&) noexcept = default;
+    FieldPathEntry & operator=(FieldPathEntry &&) noexcept = default;
     FieldPathEntry(const FieldPathEntry &);
+    FieldPathEntry & operator=(const FieldPathEntry &) = delete;
 
     /**
        Creates a field path entry for a struct field lookup.
@@ -58,8 +56,6 @@ public:
     */
     FieldPathEntry(const DataType & dataType, vespalib::stringref variableName);
 
-    FieldPathEntry * clone() const { return new FieldPathEntry(*this); }
-
     Type getType() const { return _type; }
     const vespalib::string & getName() const { return _name; }
 
@@ -70,7 +66,7 @@ public:
 
     uint32_t getIndex() const { return _lookupIndex; }
 
-    const FieldValueCP & getLookupKey() const { return _lookupKey; }
+    FieldValue & getLookupKey() const { return *_lookupKey; }
 
     const vespalib::string& getVariableName() const { return _variableName; }
 
@@ -85,20 +81,20 @@ public:
     static vespalib::string parseKey(vespalib::stringref & key);
 private:
     void setFillValue(const DataType & dataType);
-    Type                 _type;
-    vespalib::string     _name;
-    Field                _field;
-    const DataType     * _dataType;
-    uint32_t             _lookupIndex;
-    FieldValueCP         _lookupKey;
-    vespalib::string     _variableName;
-    mutable FieldValueCP _fillInVal;
+    Type                                _type;
+    vespalib::string                    _name;
+    Field                               _field;
+    const DataType                    * _dataType;
+    uint32_t                            _lookupIndex;
+    std::unique_ptr<FieldValue>         _lookupKey;
+    vespalib::string                    _variableName;
+    mutable std::unique_ptr<FieldValue> _fillInVal;
 };
 
 //typedef std::deque<FieldPathEntry> FieldPath;
 // Facade over FieldPathEntry container that exposes cloneability
 class FieldPath {
-    typedef std::vector<vespalib::CloneablePtr<FieldPathEntry>> Container;
+    typedef std::vector<std::unique_ptr<FieldPathEntry>> Container;
 public:
     typedef Container::reference reference;
     typedef Container::const_reference const_reference;
@@ -110,16 +106,11 @@ public:
 
     FieldPath();
     FieldPath(const FieldPath &);
-    FieldPath & operator=(const FieldPath &);
+    FieldPath & operator=(const FieldPath &) = delete;
     FieldPath(FieldPath &&) noexcept = default;
     FieldPath & operator=(FieldPath &&) noexcept = default;
     ~FieldPath();
 
-    template <typename InputIterator>
-    FieldPath(InputIterator first, InputIterator last)
-        : _path(first, last)
-    { }
-
     iterator insert(iterator pos, std::unique_ptr<FieldPathEntry> entry);
     void push_back(std::unique_ptr<FieldPathEntry> entry);
 
diff --git a/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp b/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp
index 33ce4357f03..7385b261339 100644
--- a/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp
+++ b/document/src/vespa/document/fieldvalue/mapfieldvalue.cpp
@@ -421,9 +421,9 @@ MapFieldValue::iterateNestedImpl(PathRange nested,
         case FieldPathEntry::MAP_KEY:
         {
             LOG(spam, "MAP_KEY");
-            const_iterator iter = find(*fpe.getLookupKey());
+            const_iterator iter = find(fpe.getLookupKey());
             if (iter != end()) {
-                wasModified = checkAndRemove(*fpe.getLookupKey(),
+                wasModified = checkAndRemove(fpe.getLookupKey(),
                         iter->second->iterateNested(nested.next(), handler),
                         wasModified, keysToRemove);
             } else if (handler.createMissingPath()) {
@@ -431,7 +431,7 @@ MapFieldValue::iterateNestedImpl(PathRange nested,
                 FieldValue::UP val = getMapType().getValueType().createFieldValue();
                 ModificationStatus status = val->iterateNested(nested.next(), handler);
                 if (status == ModificationStatus::MODIFIED) {
-                    const_cast<MapFieldValue&>(*this).put(FieldValue::UP(fpe.getLookupKey()->clone()), std::move(val));
+                    const_cast<MapFieldValue&>(*this).put(FieldValue::UP(fpe.getLookupKey().clone()), std::move(val));
                     return status;
                 }
             }
diff --git a/searchlib/src/vespa/searchlib/expression/documentfieldnode.cpp b/searchlib/src/vespa/searchlib/expression/documentfieldnode.cpp
index 91e5a36a7cf..f48be061d15 100644
--- a/searchlib/src/vespa/searchlib/expression/documentfieldnode.cpp
+++ b/searchlib/src/vespa/searchlib/expression/documentfieldnode.cpp
@@ -26,7 +26,7 @@ DocumentFieldNode::~DocumentFieldNode() = default;
 
 DocumentFieldNode::DocumentFieldNode(const DocumentFieldNode & rhs) :
     DocumentAccessorNode(rhs),
-    _fieldPath(rhs._fieldPath),
+    _fieldPath(),
     _value(rhs._value),
     _fieldName(rhs._fieldName),
     _doc(nullptr)
@@ -38,7 +38,7 @@ DocumentFieldNode::operator = (const DocumentFieldNode & rhs)
 {
     if (this != &rhs) {
         DocumentAccessorNode::operator=(rhs);
-        _fieldPath = rhs._fieldPath;
+        _fieldPath.clear();
         _value = rhs._value;
         _fieldName = rhs._fieldName;
         _doc = nullptr;
@@ -146,7 +146,7 @@ void DocumentFieldNode::onDocType(const DocumentType & docType)
     _fieldPath.clear();
     docType.buildFieldPath(_fieldPath, _fieldName);
     if (_fieldPath.empty()) {
-        throw std::runtime_error(make_string("Field %s could not be loacated in documenttype %s", _fieldName.c_str(), docType.getName().c_str()));
+        throw std::runtime_error(make_string("Field %s could not be located in documenttype %s", _fieldName.c_str(), docType.getName().c_str()));
     }
 }
 
diff --git a/searchlib/src/vespa/searchlib/expression/documentfieldnode.h b/searchlib/src/vespa/searchlib/expression/documentfieldnode.h
index fd3923bd4a0..e1038f73fa3 100644
--- a/searchlib/src/vespa/searchlib/expression/documentfieldnode.h
+++ b/searchlib/src/vespa/searchlib/expression/documentfieldnode.h
@@ -32,11 +32,13 @@ public:
     DECLARE_NBO_SERIALIZE;
     void visitMembers(vespalib::ObjectVisitor &visitor) const override;
     DECLARE_EXPRESSIONNODE(DocumentFieldNode);
-    DocumentFieldNode() : _fieldPath(), _value(), _fieldName(), _doc(NULL) { }
-    ~DocumentFieldNode();
-    DocumentFieldNode(vespalib::stringref name) : _fieldPath(), _value(), _fieldName(name), _doc(NULL) { }
+    DocumentFieldNode() : _fieldPath(), _value(), _fieldName(), _doc(nullptr) { }
+    ~DocumentFieldNode() override;
+    DocumentFieldNode(vespalib::stringref name) : _fieldPath(), _value(), _fieldName(name), _doc(nullptr) { }
     DocumentFieldNode(const DocumentFieldNode & rhs);
     DocumentFieldNode & operator = (const DocumentFieldNode & rhs);
+    DocumentFieldNode(DocumentFieldNode && rhs) noexcept = default;
+    DocumentFieldNode & operator = (DocumentFieldNode && rhs) noexcept = default;
     const vespalib::string & getFieldName() const override { return _fieldName; }
 private:
     class Handler : public document::fieldvalue::IteratorHandler {
diff --git a/vsm/src/tests/docsum/docsum.cpp b/vsm/src/tests/docsum/docsum.cpp
index 35553c6f98c..475489d2f5a 100644
--- a/vsm/src/tests/docsum/docsum.cpp
+++ b/vsm/src/tests/docsum/docsum.cpp
@@ -204,12 +204,12 @@ DocsumTest::testSlimeFieldWriter()
             {
                 FieldPath path;
                 type.buildFieldPath(path, "a");
-                fields.push_back(DocsumFieldSpec::FieldIdentifier(0, path));
+                fields.push_back(DocsumFieldSpec::FieldIdentifier(0, std::move(path)));
             }
             {
                 FieldPath path;
                 type.buildFieldPath(path, "c.e");
-                fields.push_back(DocsumFieldSpec::FieldIdentifier(0, path));
+                fields.push_back(DocsumFieldSpec::FieldIdentifier(0, std::move(path)));
             }
             sfw.setInputFields(fields);
             TEST_DO(assertSlimeFieldWriter(sfw, value, "{\"a\":\"foo\",\"c\":{\"e\":\"qux\"}}"));
@@ -257,14 +257,14 @@ DocsumTest::requireThatSlimeFieldWriterHandlesMap()
             {
                 FieldPath path;
                 mapType.buildFieldPath(path, "value.b");
-                fields.push_back(DocsumFieldSpec::FieldIdentifier(0, path));
+                fields.push_back(DocsumFieldSpec::FieldIdentifier(0, std::move(path)));
             }
             sfw.setInputFields(fields);
             TEST_DO(assertSlimeFieldWriter(sfw, mapfv, "[{\"key\":\"k1\",\"value\":{\"b\":\"bar\"}}]"));
             {
                 FieldPath path;
                 mapType.buildFieldPath(path, "{k1}.a");
-                fields[0] = DocsumFieldSpec::FieldIdentifier(0, path);
+                fields[0] = DocsumFieldSpec::FieldIdentifier(0, std::move(path));
             }
             sfw.clear();
             sfw.setInputFields(fields);
diff --git a/vsm/src/vespa/vsm/common/documenttypemapping.cpp b/vsm/src/vespa/vsm/common/documenttypemapping.cpp
index 2379103653e..7886c44b2e0 100644
--- a/vsm/src/vespa/vsm/common/documenttypemapping.cpp
+++ b/vsm/src/vespa/vsm/common/documenttypemapping.cpp
@@ -45,13 +45,13 @@ bool DocumentTypeMapping::prepareBaseDoc(SharedFieldPathMap & map) const
 {
     FieldPathMapMapT::const_iterator found = _fieldMap.find(_defaultDocumentTypeName);
     if (found != _fieldMap.end()) {
-        map.reset(new FieldPathMapT(found->second));
+        map = std::make_shared<FieldPathMapT>(found->second);
         LOG(debug, "Found FieldPathMap for default document type '%s' with %zd elements",
             _defaultDocumentTypeName.c_str(), map->size());
     } else {
         LOG(warning, "No FieldPathMap found for default document type '%s'. Using empty one",
             _defaultDocumentTypeName.c_str());
-        map.reset(new FieldPathMapT());
+        map = std::make_shared<FieldPathMapT>();
     }
     return true;
 }
@@ -70,7 +70,7 @@ void DocumentTypeMapping::buildFieldMap(
     highestFNo++;
     FieldPathMapT & fieldMap = _fieldMap[typeId];
 
-    fieldMap.assign(highestFNo, FieldPath());
+    fieldMap.resize(highestFNo);
 
     size_t validCount(0);
     for (StringFieldIdTMapT::const_iterator it = fieldList.begin(), mt = fieldList.end(); it != mt; it++) {
diff --git a/vsm/src/vespa/vsm/common/storagedocument.h b/vsm/src/vespa/vsm/common/storagedocument.h
index 46a3e2f3251..a7f21cb052f 100644
--- a/vsm/src/vespa/vsm/common/storagedocument.h
+++ b/vsm/src/vespa/vsm/common/storagedocument.h
@@ -17,7 +17,7 @@ public:
 
     class SubDocument {
     public:
-        SubDocument() : _fieldValue(NULL) {}
+        SubDocument() : _fieldValue(nullptr) {}
         SubDocument(document::FieldValue *fv, document::FieldValue::PathRange nested) :
                 _fieldValue(fv),
                 _range(nested)
@@ -43,7 +43,7 @@ public:
     ~StorageDocument();
 
     const document::Document &docDoc() const { return *_doc; }
-    bool valid() const { return _doc.get() != NULL; }
+    bool valid() const { return _doc.get() != nullptr; }
     const SubDocument &getComplexField(FieldIdT fId) const;
     const document::FieldValue *getField(FieldIdT fId) const override;
     bool setField(FieldIdT fId, document::FieldValue::UP fv) override ;
diff --git a/vsm/src/vespa/vsm/vsm/docsumfieldspec.cpp b/vsm/src/vespa/vsm/vsm/docsumfieldspec.cpp
index ba8310a9879..936aaaa2091 100644
--- a/vsm/src/vespa/vsm/vsm/docsumfieldspec.cpp
+++ b/vsm/src/vespa/vsm/vsm/docsumfieldspec.cpp
@@ -10,9 +10,12 @@ DocsumFieldSpec::FieldIdentifier::FieldIdentifier() :
 
 DocsumFieldSpec::FieldIdentifier::FieldIdentifier(FieldIdT id, FieldPath path) :
     _id(id),
-    _path(path)
+    _path(std::move(path))
 { }
 
+DocsumFieldSpec::FieldIdentifier::FieldIdentifier(FieldIdentifier &&) noexcept = default;
+DocsumFieldSpec::FieldIdentifier & DocsumFieldSpec::FieldIdentifier::operator=(FieldIdentifier &&) noexcept = default;
+DocsumFieldSpec::FieldIdentifier::~FieldIdentifier() = default;
 
 DocsumFieldSpec::DocsumFieldSpec() :
     _resultType(search::docsummary::RES_INT),
diff --git a/vsm/src/vespa/vsm/vsm/docsumfieldspec.h b/vsm/src/vespa/vsm/vsm/docsumfieldspec.h
index 5acef140468..db6ee9fa223 100644
--- a/vsm/src/vespa/vsm/vsm/docsumfieldspec.h
+++ b/vsm/src/vespa/vsm/vsm/docsumfieldspec.h
@@ -24,6 +24,11 @@ public:
     public:
         FieldIdentifier();
         FieldIdentifier(FieldIdT id, FieldPath path);
+        FieldIdentifier(FieldIdentifier &&) noexcept;
+        FieldIdentifier & operator=(FieldIdentifier &&) noexcept;
+        FieldIdentifier(const FieldIdentifier &) = delete;
+        FieldIdentifier & operator=(const FieldIdentifier &) = delete;
+        ~FieldIdentifier();
         FieldIdT getId() const { return _id; }
         const FieldPath & getPath() const { return _path; }
     };
@@ -58,7 +63,7 @@ public:
     }
 
     const FieldIdentifier & getOutputField() const { return _outputField; }
-    void setOutputField(const FieldIdentifier & outputField) { _outputField = outputField; }
+    void setOutputField(FieldIdentifier outputField) { _outputField = std::move(outputField); }
     const FieldIdentifierVector & getInputFields() const { return _inputFields; }
     FieldIdentifierVector & getInputFields() { return _inputFields; }
 };
diff --git a/vsm/src/vespa/vsm/vsm/docsumfilter.cpp b/vsm/src/vespa/vsm/vsm/docsumfilter.cpp
index bd16c687fc7..42ab32afbf7 100644
--- a/vsm/src/vespa/vsm/vsm/docsumfilter.cpp
+++ b/vsm/src/vespa/vsm/vsm/docsumfilter.cpp
@@ -118,6 +118,18 @@ public:
 
 namespace vsm {
 
+FieldPath
+copyPathButFirst(const FieldPath & rhs) {
+    // skip the element that correspond to the start field value
+    FieldPath path;
+    if ( ! rhs.empty()) {
+        for (auto it = rhs.begin() + 1; it != rhs.end(); it++) {
+            path.push_back(std::make_unique<document::FieldPathEntry>(**it));
+        }
+    }
+    return path;
+}
+
 void
 DocsumFilter::prepareFieldSpec(DocsumFieldSpec & spec, const DocsumTools::FieldSpec & toolsSpec,
                                const FieldMap & fieldMap, const FieldPathMapT & fieldPathMap)
@@ -128,14 +140,7 @@ DocsumFilter::prepareFieldSpec(DocsumFieldSpec & spec, const DocsumTools::FieldS
         FieldIdT field = fieldMap.fieldNo(name);
         if (field != FieldMap::npos) {
             if (field < fieldPathMap.size()) {
-               if (!fieldPathMap[field].empty()) {
-                   // skip the element that correspond to the start field value
-                   spec.setOutputField(DocsumFieldSpec::FieldIdentifier
-                                       (field, FieldPath(fieldPathMap[field].begin() + 1,
-                                                         fieldPathMap[field].end())));
-               } else {
-                   spec.setOutputField(DocsumFieldSpec::FieldIdentifier(field, FieldPath()));
-               }
+                spec.setOutputField(DocsumFieldSpec::FieldIdentifier(field, copyPathButFirst(fieldPathMap[field])));
             } else {
                 LOG(warning, "Could not find a field path for field '%s' with id '%d'", name.c_str(), field);
                 spec.setOutputField(DocsumFieldSpec::FieldIdentifier(field, FieldPath()));
@@ -152,18 +157,7 @@ DocsumFilter::prepareFieldSpec(DocsumFieldSpec & spec, const DocsumTools::FieldS
         if (field != FieldMap::npos) {
             if (field < fieldPathMap.size()) {
                 LOG(debug, "field %u < map size %zu", field, fieldPathMap.size());
-                if (!fieldPathMap[field].empty()) {
-                    FieldPath relPath(fieldPathMap[field].begin() + 1,
-                                      fieldPathMap[field].end());
-                    LOG(debug, "map[%u] -> %zu elements", field, fieldPathMap[field].end() - fieldPathMap[field].begin());
-                    // skip the element that correspond to the start field value
-                    spec.getInputFields().push_back(DocsumFieldSpec::FieldIdentifier
-                                                    (field, FieldPath(fieldPathMap[field].begin() + 1,
-                                                                      fieldPathMap[field].end())));
-                } else {
-                    LOG(debug, "map[%u] empty", field);
-                    spec.getInputFields().push_back(DocsumFieldSpec::FieldIdentifier(field, FieldPath()));
-                }
+                spec.getInputFields().push_back(DocsumFieldSpec::FieldIdentifier(field, copyPathButFirst(fieldPathMap[field])));
             } else {
                 LOG(warning, "Could not find a field path for field '%s' with id '%d'", name.c_str(), field);
                 spec.getInputFields().push_back(DocsumFieldSpec::FieldIdentifier(field, FieldPath()));