Use cloneableptr for expressiontree. It aint't copied anyway.

8 files changed, 69 insertions, 40 deletions

diff --git a/searchlib/src/tests/grouping/grouping_serialization_test.cpp b/searchlib/src/tests/grouping/grouping_serialization_test.cpp
index f3f8d477c44..8592995915c 100644
--- a/searchlib/src/tests/grouping/grouping_serialization_test.cpp
+++ b/searchlib/src/tests/grouping/grouping_serialization_test.cpp
@@ -292,33 +292,41 @@ TEST_F("testGroup", Fixture("testGroup")) {
                           .addChild(Group().setId(Int64ResultNode(131)))));
 }
 
+GroupingLevel
+createDummyLevel(size_t maxGroups, size_t numAggr) {
+    GroupingLevel l;
+    l.setMaxGroups(maxGroups);
+    l.setExpression(createDummyExpression());
+    for (size_t i(0); i < numAggr; i++) {
+        l.addAggregationResult(createAggr<SumAggregationResult>(createDummyExpression()));
+    }
+    return l;
+}
+
+GroupingLevel
+createLargeLevel() {
+    GroupingLevel l;
+    l.setExpression(MU<AttributeNode>("folder"));
+    l.addAggregationResult(createAggr<XorAggregationResult>(MU<MD5BitFunctionNode>(MU<AttributeNode>("docid"), 64)));
+    l.addAggregationResult(createAggr<SumAggregationResult>(
+            ExpressionNode::UP(MinFunctionNode()
+                                       .addArg(MU<AttributeNode>("attribute1"))
+                                       .addArg(MU<AttributeNode>("attribute2")).clone())));
+    l.addAggregationResult(createAggr<XorAggregationResult>(
+            ExpressionNode::UP(XorBitFunctionNode(
+                    ExpressionNode::UP(CatFunctionNode()
+                                               .addArg(MU<GetDocIdNamespaceSpecificFunctionNode>())
+                                               .addArg(MU<DocumentFieldNode>("folder"))
+                                               .addArg(MU<DocumentFieldNode>("flags")).clone())
+                    , 64).clone())));
+    return l;
+}
 TEST_F("testGrouping", Fixture("testGrouping")) {
+
     f.checkObject(Grouping());
-    f.checkObject(Grouping()
-                .addLevel(GroupingLevel()
-                          .setMaxGroups(100)
-                          .setExpression(createDummyExpression())
-                          .addAggregationResult(createAggr<SumAggregationResult>(createDummyExpression())))
-                .addLevel(GroupingLevel()
-                          .setMaxGroups(10)
-                          .setExpression(createDummyExpression())
-                          .addAggregationResult(createAggr<SumAggregationResult>(createDummyExpression()))
-                                  .addAggregationResult(createAggr<SumAggregationResult>(createDummyExpression()))));
-    f.checkObject(Grouping()
-                .addLevel(GroupingLevel()
-                          .setExpression(MU<AttributeNode>("folder"))
-                          .addAggregationResult(createAggr<XorAggregationResult>(MU<MD5BitFunctionNode>(MU<AttributeNode>("docid"), 64)))
-                          .addAggregationResult(createAggr<SumAggregationResult>(
-                                  ExpressionNode::UP(MinFunctionNode()
-                                                             .addArg(MU<AttributeNode>("attribute1"))
-                                                             .addArg(MU<AttributeNode>("attribute2")).clone())))
-                          .addAggregationResult(createAggr<XorAggregationResult>(
-                                  ExpressionNode::UP(XorBitFunctionNode(
-                                          ExpressionNode::UP(CatFunctionNode()
-                                                                     .addArg(MU<GetDocIdNamespaceSpecificFunctionNode>())
-                                                                     .addArg(MU<DocumentFieldNode>("folder"))
-                                                                     .addArg(MU<DocumentFieldNode>("flags")).clone())
-                                          , 64).clone())))));
+    f.checkObject(Grouping().addLevel(createDummyLevel(100, 1))
+                            .addLevel(createDummyLevel(10, 2)));
+    f.checkObject(Grouping().addLevel(createLargeLevel()));
 }
 
 }  // namespace
diff --git a/searchlib/src/vespa/searchlib/aggregation/group.h b/searchlib/src/vespa/searchlib/aggregation/group.h
index 63d52bed147..c52a830f89e 100644
--- a/searchlib/src/vespa/searchlib/aggregation/group.h
+++ b/searchlib/src/vespa/searchlib/aggregation/group.h
@@ -124,7 +124,7 @@ public:
     Group & operator =(const Group & rhs);
     Group(Group &&) noexcept;
     Group & operator = (Group &&) noexcept;
-    ~Group();
+    ~Group() noexcept;
     void swap(Group & rhs);
 
     int cmpId(const Group &rhs) const { return _id->cmpFast(*rhs._id); }
diff --git a/searchlib/src/vespa/searchlib/aggregation/grouping.cpp b/searchlib/src/vespa/searchlib/aggregation/grouping.cpp
index 11b354b4e9b..4cce5cb6f59 100644
--- a/searchlib/src/vespa/searchlib/aggregation/grouping.cpp
+++ b/searchlib/src/vespa/searchlib/aggregation/grouping.cpp
@@ -121,6 +121,12 @@ Grouping::Grouping()
 {
 }
 
+Grouping::Grouping(const Grouping &) = default;
+Grouping & Grouping::operator = (const Grouping &) = default;
+Grouping::Grouping(Grouping &&) noexcept = default;
+Grouping & Grouping::operator = (Grouping &&) noexcept = default;
+Grouping::~Grouping() { }
+
 void
 Grouping::selectMembers(const vespalib::ObjectPredicate &predicate,
                         vespalib::ObjectOperation &operation)
diff --git a/searchlib/src/vespa/searchlib/aggregation/grouping.h b/searchlib/src/vespa/searchlib/aggregation/grouping.h
index a4a93726488..eceb40db6e6 100644
--- a/searchlib/src/vespa/searchlib/aggregation/grouping.h
+++ b/searchlib/src/vespa/searchlib/aggregation/grouping.h
@@ -42,6 +42,11 @@ public:
     DECLARE_NBO_SERIALIZE;
     virtual void visitMembers(vespalib::ObjectVisitor &visitor) const;
     Grouping();
+    Grouping(const Grouping &);
+    Grouping & operator = (const Grouping &);
+    Grouping(Grouping &&) noexcept;
+    Grouping & operator = (Grouping &&) noexcept;
+    ~Grouping();
 
     Grouping unchain() const { return *this; }
 
@@ -51,7 +56,7 @@ public:
     Grouping &setTopN(int64_t v)                { _topN = v;                return *this; }
     Grouping &setFirstLevel(unsigned int level) { _firstLevel = level;      return *this; }
     Grouping &setLastLevel(unsigned int level)  { _lastLevel = level;       return *this; }
-    Grouping &addLevel(GroupingLevel level)     { _levels.emplace_back(std::move(level)); return *this; }
+    Grouping &addLevel(GroupingLevel && level)  { _levels.emplace_back(std::move(level)); return *this; }
     Grouping &setRoot(const Group &root_)       { _root = root_;            return *this; }
     Grouping &setClock(const vespalib::Clock * clock) { _clock = clock; return *this; }
     Grouping &setTimeOfDoom(fastos::TimeStamp timeOfDoom) { _timeOfDoom = timeOfDoom; return *this; }
diff --git a/searchlib/src/vespa/searchlib/aggregation/groupinglevel.cpp b/searchlib/src/vespa/searchlib/aggregation/groupinglevel.cpp
index ad2a8511f58..cc67d522c63 100644
--- a/searchlib/src/vespa/searchlib/aggregation/groupinglevel.cpp
+++ b/searchlib/src/vespa/searchlib/aggregation/groupinglevel.cpp
@@ -28,15 +28,15 @@ GroupingLevel::~GroupingLevel() { }
 
 GroupingLevel::GroupingLevel(const GroupingLevel &) = default;
 GroupingLevel & GroupingLevel::operator =(const GroupingLevel &) = default;
-GroupingLevel::GroupingLevel(GroupingLevel &&) = default;
-GroupingLevel & GroupingLevel::operator =(GroupingLevel &&) = default;
 
-Serializer & GroupingLevel::onSerialize(Serializer & os) const
+Serializer &
+GroupingLevel::onSerialize(Serializer & os) const
 {
     return os << _maxGroups << _precision << _classify << _collect;
 }
 
-Deserializer & GroupingLevel::onDeserialize(Deserializer & is)
+Deserializer &
+GroupingLevel::onDeserialize(Deserializer & is)
 {
     return is >> _maxGroups >> _precision >> _classify >> _collect;
 }
diff --git a/searchlib/src/vespa/searchlib/aggregation/groupinglevel.h b/searchlib/src/vespa/searchlib/aggregation/groupinglevel.h
index 717f65d1484..ed7d2429682 100644
--- a/searchlib/src/vespa/searchlib/aggregation/groupinglevel.h
+++ b/searchlib/src/vespa/searchlib/aggregation/groupinglevel.h
@@ -78,8 +78,8 @@ private:
     vespalib::CloneablePtr<Grouper>    _grouper;
 public:
     GroupingLevel();
-    GroupingLevel(GroupingLevel &&);
-    GroupingLevel & operator =(GroupingLevel &&);
+    GroupingLevel(GroupingLevel &&) = default;
+    GroupingLevel & operator =(GroupingLevel &&) = default;
     GroupingLevel(const GroupingLevel &);
     GroupingLevel & operator =(const GroupingLevel &);
     ~GroupingLevel();
diff --git a/searchlib/src/vespa/searchlib/expression/expressiontree.cpp b/searchlib/src/vespa/searchlib/expression/expressiontree.cpp
index 5c4f170f87b..4742a13fa29 100644
--- a/searchlib/src/vespa/searchlib/expression/expressiontree.cpp
+++ b/searchlib/src/vespa/searchlib/expression/expressiontree.cpp
@@ -106,7 +106,8 @@ ExpressionTree::ExpressionTree(const ExpressionTree & rhs) :
     prepare(false);
 }
 
-ExpressionTree & ExpressionTree::operator = (const ExpressionTree & rhs)
+ExpressionTree &
+ExpressionTree::operator = (const ExpressionTree & rhs)
 {
     if (this != & rhs) {
         ExpressionTree eTree(rhs);
@@ -115,6 +116,14 @@ ExpressionTree & ExpressionTree::operator = (const ExpressionTree & rhs)
     return *this;
 }
 
+ExpressionTree &
+ExpressionTree::operator = (ExpressionNode::UP rhs)
+{
+    ExpressionTree eTree(std::move(rhs));
+    swap(eTree);
+    return *this;
+}
+
 void ExpressionTree::swap(ExpressionTree & e)
 {
     std::swap(_root, e._root);
diff --git a/searchlib/src/vespa/searchlib/expression/expressiontree.h b/searchlib/src/vespa/searchlib/expression/expressiontree.h
index 373b2fd2330..1d2bb6c4053 100644
--- a/searchlib/src/vespa/searchlib/expression/expressiontree.h
+++ b/searchlib/src/vespa/searchlib/expression/expressiontree.h
@@ -47,6 +47,7 @@ public:
     ExpressionTree(const ExpressionTree & rhs);
     ExpressionTree(ExpressionTree &&) = default;
     ~ExpressionTree();
+    ExpressionTree & operator = (ExpressionNode::UP rhs);
     ExpressionTree & operator = (const ExpressionTree & rhs);
     ExpressionTree & operator = (ExpressionTree &&) = default;
 
@@ -70,12 +71,12 @@ private:
     typedef std::vector<InterpolatedLookup *> InterpolatedLookupList;
     typedef std::vector<ArrayAtLookup *> ArrayAtLookupList;
 
-    vespalib::IdentifiableLinkedPtr<ExpressionNode>  _root;
-    AttributeNodeList        _attributeNodes;
-    DocumentAccessorNodeList _documentAccessorNodes;
-    RelevanceNodeList        _relevanceNodes;
-    InterpolatedLookupList   _interpolatedLookupNodes;
-    ArrayAtLookupList        _arrayAtLookupNodes;
+    ExpressionNode::CP        _root;
+    AttributeNodeList         _attributeNodes;
+    DocumentAccessorNodeList  _documentAccessorNodes;
+    RelevanceNodeList         _relevanceNodes;
+    InterpolatedLookupList    _interpolatedLookupNodes;
+    ArrayAtLookupList         _arrayAtLookupNodes;
 };