Merge pull request #21872 from vespa-engine/balder/gc-unused-clone

GC unused clone interface.

3 files changed, 71 insertions, 70 deletions

diff --git a/document/src/vespa/document/select/value.cpp b/document/src/vespa/document/select/value.cpp
index 8b5d32cf842..e0a2bf84f09 100644
--- a/document/src/vespa/document/select/value.cpp
+++ b/document/src/vespa/document/select/value.cpp
@@ -196,11 +196,12 @@ FloatValue::print(std::ostream& out, bool verbose,
     out << _value << 'f';
 }
 
-ArrayValue::ArrayValue(const std::vector<VariableValue>& values)
+ArrayValue::ArrayValue(std::vector<VariableValue> values)
     : Value(Array),
-      _values(values)
+      _values(std::move(values))
 {
 }
+ArrayValue::~ArrayValue() = default;
 
 struct ArrayValue::EqualsComparator {
     bool operator()(std::size_t lhs, std::size_t rhs) const { return lhs == rhs; }
@@ -318,12 +319,14 @@ ArrayValue::print(std::ostream& out, bool verbose,
     out << "<no array representation in language yet>";
 }
 
-StructValue::StructValue(const ValueMap & values)
+StructValue::StructValue(ValueMap values)
     : Value(Struct),
-      _values(values)
+      _values(std::move(values))
 {
 }
 
+StructValue::~StructValue() = default;
+
 ResultList
 StructValue::operator<(const Value& value) const
 {
diff --git a/document/src/vespa/document/select/value.h b/document/src/vespa/document/select/value.h
index 9f9ced18296..aeb814262f2 100644
--- a/document/src/vespa/document/select/value.h
+++ b/document/src/vespa/document/select/value.h
@@ -33,29 +33,30 @@ public:
     enum Type { Invalid, Null, String, Integer, Float, Array, Struct, Bucket };
 
     Value(Type t) : _type(t) {}
-    virtual ~Value() {}
+    virtual ~Value() = default;
 
     Type getType() const { return _type; }
 
     virtual ResultList operator<(const Value& value) const = 0;
     virtual ResultList operator==(const Value& value) const = 0;
 
-    virtual UP clone() const = 0;
-
-    virtual ResultList operator!=(const Value& value) const
-        { return !(this->operator==(value)); }
-    virtual ResultList operator>(const Value& value) const
-        { return (!(this->operator<(value)) && !(this->operator==(value))); }
-    virtual ResultList operator>=(const Value& value) const
-        { return !(this->operator<(value)); }
-    virtual ResultList operator<=(const Value& value) const
-        { return ((this->operator<(value)) || (this->operator==(value))); }
+    virtual ResultList operator!=(const Value& value) const {
+        return !(this->operator==(value));
+    }
+    virtual ResultList operator>(const Value& value) const {
+        return (!(this->operator<(value)) && !(this->operator==(value)));
+    }
+    virtual ResultList operator>=(const Value& value) const {
+        return !(this->operator<(value));
+    }
+    virtual ResultList operator<=(const Value& value) const {
+        return ((this->operator<(value)) || (this->operator==(value)));
+    }
 
     virtual ResultList globCompare(const Value& value) const;
     virtual ResultList regexCompare(const Value& value) const;
     virtual ResultList globTrace(const Value& value, std::ostream& trace) const;
     virtual ResultList regexTrace(const Value& value, std::ostream& trace) const;
-
 private:
     Type _type;
 };
@@ -68,7 +69,6 @@ public:
     ResultList operator<(const Value&) const override;
     ResultList operator==(const Value&) const override;
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;
-    Value::UP clone() const override { return std::make_unique<InvalidValue>(); }
 };
 
 class NullValue : public Value
@@ -82,7 +82,6 @@ public:
     ResultList operator>=(const Value &) const override;
     ResultList operator<=(const Value &) const override;
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;
-    Value::UP clone() const override { return std::make_unique<NullValue>(); }
 };
 
 class StringValue : public Value
@@ -96,7 +95,6 @@ public:
     ResultList operator<(const Value& value) const override;
     ResultList operator==(const Value& value) const override;
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;
-    Value::UP clone() const override { return std::make_unique<StringValue>(_value); }
 };
 
 class IntegerValue;
@@ -137,10 +135,6 @@ public:
     ResultList operator==(const IntegerValue& value) const override;
     ResultList operator==(const FloatValue& value) const override;
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;
-
-    Value::UP clone() const override {
-        return std::make_unique<IntegerValue>(_value, getType() == Value::Bucket);
-    }
 private:
     ValueType _value;
 };
@@ -163,36 +157,45 @@ public:
     ResultList operator==(const IntegerValue& value) const override;
     ResultList operator==(const FloatValue& value) const override;
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;
-
-    Value::UP clone() const override { return std::make_unique<FloatValue>(_value); }
 private:
     ValueType _value;
 };
 
-inline ResultList IntegerValue::operator>(const IntegerValue& value) const
-    { return ResultList(Result::get(_value > value.getValue())); }
-inline ResultList IntegerValue::operator>(const FloatValue& value) const
-    { return ResultList(Result::get(_value > value.getValue())); }
-inline ResultList IntegerValue::operator==(const IntegerValue& value) const
-    { return ResultList(Result::get(_value == value.getValue())); }
-inline ResultList IntegerValue::operator==(const FloatValue& value) const
-    { return ResultList(Result::get(_value == value.getValue())); }
-
-inline ResultList FloatValue::operator>(const IntegerValue& value) const
-    { return ResultList(Result::get(_value > value.getValue())); }
-inline ResultList FloatValue::operator>(const FloatValue& value) const
-    { return ResultList(Result::get(_value > value.getValue())); }
-inline ResultList FloatValue::operator==(const IntegerValue& value) const
-    { return ResultList(Result::get(_value == value.getValue())); }
-inline ResultList FloatValue::operator==(const FloatValue& value) const
-    { return ResultList(Result::get(_value == value.getValue())); }
+inline ResultList IntegerValue::operator>(const IntegerValue& value) const {
+    return ResultList(Result::get(_value > value.getValue()));
+}
+inline ResultList IntegerValue::operator>(const FloatValue& value) const {
+    return ResultList(Result::get(_value > value.getValue()));
+}
+inline ResultList IntegerValue::operator==(const IntegerValue& value) const {
+    return ResultList(Result::get(_value == value.getValue()));
+}
+inline ResultList IntegerValue::operator==(const FloatValue& value) const {
+    return ResultList(Result::get(_value == value.getValue()));
+}
+
+inline ResultList FloatValue::operator>(const IntegerValue& value) const {
+    return ResultList(Result::get(_value > value.getValue()));
+}
+inline ResultList FloatValue::operator>(const FloatValue& value) const {
+    return ResultList(Result::get(_value > value.getValue()));
+}
+inline ResultList FloatValue::operator==(const IntegerValue& value) const {
+    return ResultList(Result::get(_value == value.getValue()));
+}
+inline ResultList FloatValue::operator==(const FloatValue& value) const {
+    return ResultList(Result::get(_value == value.getValue()));
+}
 
 class ArrayValue : public Value
 {
 public:
     using VariableValue = std::pair<fieldvalue::VariableMap, Value::SP>;
 
-    ArrayValue(const std::vector<VariableValue>& values);
+    ArrayValue(std::vector<VariableValue> values);
+    ArrayValue(const ArrayValue &) = delete;
+    ArrayValue & operator =(const ArrayValue &) = delete;
+    ~ArrayValue() override;
 
     ResultList operator<(const Value& value) const override;
     ResultList operator>(const Value& value) const override;
@@ -209,9 +212,6 @@ public:
 
     template <typename Predicate>
     ResultList doCompare(const Value& value, const Predicate& cmp) const;
-
-    Value::UP clone() const override { return std::make_unique<ArrayValue>(_values); }
-
 private:
     struct EqualsComparator;
     struct NotEqualsComparator;
@@ -228,14 +228,15 @@ private:
 class StructValue : public Value
 {
 public:
-    typedef std::map<vespalib::string, Value::SP> ValueMap;
-    StructValue(const ValueMap & values);
+    using ValueMap = std::map<vespalib::string, Value::SP>;
+    StructValue(ValueMap values);
+    StructValue(const StructValue &) = delete;
+    StructValue & operator = (const StructValue &) = delete;
+    ~StructValue() override;
 
     ResultList operator<(const Value& value) const override;
     ResultList operator==(const Value& value) const override;
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;
-
-    Value::UP clone() const override { return std::make_unique<StructValue>(_values); }
 private:
     ValueMap _values;
 };
diff --git a/document/src/vespa/document/select/valuenodes.cpp b/document/src/vespa/document/select/valuenodes.cpp
index f34b2e83b08..cd66bac9635 100644
--- a/document/src/vespa/document/select/valuenodes.cpp
+++ b/document/src/vespa/document/select/valuenodes.cpp
@@ -225,9 +225,8 @@ public:
     IteratorHandler();
     ~IteratorHandler();
     bool hasSingleValue() const;
-    std::unique_ptr<Value> getSingleValue();
-    const std::vector<ArrayValue::VariableValue> &getValues();
-
+    std::unique_ptr<Value> stealSingleValue() &&;
+    std::vector<ArrayValue::VariableValue> stealValues() &&;
 private:
     std::unique_ptr<Value> _firstValue;
     std::vector<ArrayValue::VariableValue> _values;
@@ -241,21 +240,21 @@ IteratorHandler::~IteratorHandler() = default;
 
 bool
 IteratorHandler::hasSingleValue() const {
-    return _firstValue.get() && (_values.size() == 0);
+    return _firstValue && _values.empty();
 }
 
 std::unique_ptr<Value>
-IteratorHandler::getSingleValue() {
+IteratorHandler::stealSingleValue() && {
     return std::move(_firstValue);
 }
 
-const std::vector<ArrayValue::VariableValue>&
-IteratorHandler::getValues() {
-    if (_firstValue.get()) {
+std::vector<ArrayValue::VariableValue>
+IteratorHandler::stealValues() && {
+    if (_firstValue) {
         _values.insert(_values.begin(), ArrayValue::VariableValue(fieldvalue::VariableMap(), Value::SP(_firstValue.release())));
     }
 
-    return _values;
+    return std::move(_values);
 }
 
 void
@@ -322,9 +321,8 @@ IteratorHandler::getInternalValue(const FieldValue& fval) const
             if (val.size() == 0) {
                 return std::make_unique<NullValue>();
             } else {
-                std::vector<ArrayValue::VariableValue> values;
                 // TODO: Array comparison.
-                return std::make_unique<ArrayValue>(values);
+                return std::make_unique<ArrayValue>(std::vector<ArrayValue::VariableValue>());
             }
         }
         case FieldValue::Type::STRUCT:
@@ -338,7 +336,7 @@ IteratorHandler::getInternalValue(const FieldValue& fval) const
                     FieldValue::UP fv(val.getValue(it.field()));
                     values[it.field().getName()] = Value::SP(getInternalValue(*fv).release());
                 }
-                return std::make_unique<StructValue>(values);
+                return std::make_unique<StructValue>(std::move(values));
             }
         }
         case FieldValue::Type::MAP:
@@ -347,9 +345,8 @@ IteratorHandler::getInternalValue(const FieldValue& fval) const
             if (val.isEmpty()) {
                 return std::make_unique<NullValue>();
             } else {
-                std::vector<ArrayValue::VariableValue> values;
                 // TODO: Map comparison
-                return std::make_unique<ArrayValue>(values);
+                return std::make_unique<ArrayValue>(std::vector<ArrayValue::VariableValue>());
             }
         }
         default:
@@ -422,14 +419,14 @@ FieldValueNode::getValue(const Context& context) const
         doc.iterateNested(_fieldPath.getFullRange(), handler);
 
         if (handler.hasSingleValue()) {
-            return handler.getSingleValue();
+            return std::move(handler).stealSingleValue();
         } else {
-            const std::vector<ArrayValue::VariableValue>& values = handler.getValues();
+            std::vector<ArrayValue::VariableValue> values = std::move(handler).stealValues();
 
             if (values.empty()) {
                 return std::make_unique<NullValue>();
             } else {
-                return std::make_unique<ArrayValue>(handler.getValues());
+                return std::make_unique<ArrayValue>(std::move(values));
             }
         }
     } catch (vespalib::IllegalArgumentException& e) {
@@ -483,14 +480,14 @@ FieldValueNode::traceValue(const Context &context, std::ostream& out) const
         doc.iterateNested(_fieldPath.getFullRange(), handler);
 
         if (handler.hasSingleValue()) {
-            return handler.getSingleValue();
+            return std::move(handler).stealSingleValue();
         } else {
-            const std::vector<ArrayValue::VariableValue>& values = handler.getValues();
+            std::vector<ArrayValue::VariableValue> values = std::move(handler).stealValues();
 
             if (values.size() == 0) {
                 return std::make_unique<NullValue>();
             } else {
-                return std::make_unique<ArrayValue>(handler.getValues());
+                return std::make_unique<ArrayValue>(std::move(values));
             }
         }
     } catch (FieldNotFoundException& e) {