diff options
author | Jacob Evans <jacob.evans@10gen.com> | 2021-03-14 21:11:21 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-03-24 01:03:55 +0000 |
commit | c2a96ca05d4e196d3b5ffb81d05a88346e4e8fbe (patch) | |
tree | 6364d416be318b9b2ddd021b8bfc00d40de44830 /src/mongo/db/matcher | |
parent | a97002800ba3597cea321cae42298da4a3b12626 (diff) | |
download | mongo-c2a96ca05d4e196d3b5ffb81d05a88346e4e8fbe.tar.gz |
SERVER-55183 Fix owning raw pointers in matcher/
Diffstat (limited to 'src/mongo/db/matcher')
29 files changed, 510 insertions, 564 deletions
diff --git a/src/mongo/db/matcher/expression.cpp b/src/mongo/db/matcher/expression.cpp index 5c7908ff70f..0057f0c2903 100644 --- a/src/mongo/db/matcher/expression.cpp +++ b/src/mongo/db/matcher/expression.cpp @@ -106,7 +106,9 @@ void MatchExpression::sortTree(MatchExpression* tree) { sortTree(tree->getChild(i)); } if (auto&& children = tree->getChildVector()) { - std::stable_sort(children->begin(), children->end(), matchExpressionLessThan); + std::stable_sort(children->begin(), children->end(), [](auto&& lhs, auto&& rhs) { + return matchExpressionLessThan(lhs.get(), rhs.get()); + }); } } diff --git a/src/mongo/db/matcher/expression.h b/src/mongo/db/matcher/expression.h index 2a9216e85e6..6d8a0d6eab5 100644 --- a/src/mongo/db/matcher/expression.h +++ b/src/mongo/db/matcher/expression.h @@ -328,7 +328,7 @@ public: * Do not use to traverse the MatchExpression tree. Use numChildren() and getChild(), which * provide access to all nodes. */ - virtual boost::optional<std::vector<MatchExpression*>&> getChildVector() = 0; + virtual std::vector<std::unique_ptr<MatchExpression>>* getChildVector() = 0; /** * Get the path of the leaf. Returns StringData() if there is no path (node is logical). diff --git a/src/mongo/db/matcher/expression_algo.cpp b/src/mongo/db/matcher/expression_algo.cpp index 9bcde065e81..2f06021a775 100644 --- a/src/mongo/db/matcher/expression_algo.cpp +++ b/src/mongo/db/matcher/expression_algo.cpp @@ -252,9 +252,8 @@ unique_ptr<MatchExpression> createAndOfNodes(std::vector<unique_ptr<MatchExpress } unique_ptr<AndMatchExpression> splitAnd = std::make_unique<AndMatchExpression>(); - for (auto&& expr : *children) { - splitAnd->add(expr.release()); - } + for (auto&& expr : *children) + splitAnd->add(std::move(expr)); return splitAnd; } @@ -268,9 +267,8 @@ unique_ptr<MatchExpression> createNorOfNodes(std::vector<unique_ptr<MatchExpress } unique_ptr<NorMatchExpression> splitNor = std::make_unique<NorMatchExpression>(); - for (auto&& expr : *children) { - splitNor->add(expr.release()); - } + for (auto&& expr : *children) + splitNor->add(std::move(expr)); return splitNor; } diff --git a/src/mongo/db/matcher/expression_always_boolean.h b/src/mongo/db/matcher/expression_always_boolean.h index 74587158040..73dc9be4f0b 100644 --- a/src/mongo/db/matcher/expression_always_boolean.h +++ b/src/mongo/db/matcher/expression_always_boolean.h @@ -82,8 +82,8 @@ public: MONGO_UNREACHABLE; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } private: diff --git a/src/mongo/db/matcher/expression_arity.h b/src/mongo/db/matcher/expression_arity.h index 20a0818fb7e..61c03567368 100644 --- a/src/mongo/db/matcher/expression_arity.h +++ b/src/mongo/db/matcher/expression_arity.h @@ -81,8 +81,8 @@ public: [](const auto& expr1, const auto& expr2) { return expr1->equivalent(expr2.get()); }); } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } size_t numChildren() const final { diff --git a/src/mongo/db/matcher/expression_array.cpp b/src/mongo/db/matcher/expression_array.cpp index c3a9b2924a2..cbff0adf1ca 100644 --- a/src/mongo/db/matcher/expression_array.cpp +++ b/src/mongo/db/matcher/expression_array.cpp @@ -28,9 +28,9 @@ */ #include "mongo/db/matcher/expression_array.h" - #include "mongo/db/field_ref.h" #include "mongo/db/jsobj.h" +#include "mongo/db/query/util/make_data_structure.h" namespace mongo { @@ -67,8 +67,9 @@ bool ArrayMatchingMatchExpression::equivalent(const MatchExpression* other) cons // ------- ElemMatchObjectMatchExpression::ElemMatchObjectMatchExpression( - StringData path, MatchExpression* sub, clonable_ptr<ErrorAnnotation> annotation) - : ArrayMatchingMatchExpression(ELEM_MATCH_OBJECT, path, std::move(annotation)), _sub(sub) {} + StringData path, std::unique_ptr<MatchExpression> sub, clonable_ptr<ErrorAnnotation> annotation) + : ArrayMatchingMatchExpression(ELEM_MATCH_OBJECT, path, std::move(annotation)), + _sub(std::move(sub)) {} bool ElemMatchObjectMatchExpression::matchesArray(const BSONObj& anArray, MatchDetails* details) const { @@ -118,24 +119,16 @@ MatchExpression::ExpressionOptimizerFunc ElemMatchObjectMatchExpression::getOpti // ------- ElemMatchValueMatchExpression::ElemMatchValueMatchExpression( - StringData path, MatchExpression* sub, clonable_ptr<ErrorAnnotation> annotation) - : ArrayMatchingMatchExpression(ELEM_MATCH_VALUE, path, std::move(annotation)) { - add(sub); -} + StringData path, std::unique_ptr<MatchExpression> sub, clonable_ptr<ErrorAnnotation> annotation) + : ArrayMatchingMatchExpression(ELEM_MATCH_VALUE, path, std::move(annotation)), + _subs(makeVector(std::move(sub))) {} ElemMatchValueMatchExpression::ElemMatchValueMatchExpression( StringData path, clonable_ptr<ErrorAnnotation> annotation) : ArrayMatchingMatchExpression(ELEM_MATCH_VALUE, path, std::move(annotation)) {} -ElemMatchValueMatchExpression::~ElemMatchValueMatchExpression() { - for (unsigned i = 0; i < _subs.size(); i++) - delete _subs[i]; - _subs.clear(); -} - -void ElemMatchValueMatchExpression::add(MatchExpression* sub) { - verify(sub); - _subs.push_back(sub); +void ElemMatchValueMatchExpression::add(std::unique_ptr<MatchExpression> sub) { + _subs.push_back(std::move(sub)); } bool ElemMatchValueMatchExpression::matchesArray(const BSONObj& anArray, @@ -191,11 +184,8 @@ MatchExpression::ExpressionOptimizerFunc ElemMatchValueMatchExpression::getOptim return [](std::unique_ptr<MatchExpression> expression) { auto& subs = static_cast<ElemMatchValueMatchExpression&>(*expression)._subs; - for (MatchExpression*& subExpression : subs) { - auto optimizedSubExpression = - MatchExpression::optimize(std::unique_ptr<MatchExpression>(subExpression)); - subExpression = optimizedSubExpression.release(); - } + for (auto& subExpression : subs) + subExpression = MatchExpression::optimize(std::move(subExpression)); return expression; }; diff --git a/src/mongo/db/matcher/expression_array.h b/src/mongo/db/matcher/expression_array.h index 29b2e1d8962..edcadfe2b5e 100644 --- a/src/mongo/db/matcher/expression_array.h +++ b/src/mongo/db/matcher/expression_array.h @@ -54,8 +54,6 @@ public: ElementPath::NonLeafArrayBehavior::kTraverse, std::move(annotation)) {} - virtual ~ArrayMatchingMatchExpression() {} - /** * Returns whether or not the nested array, represented as the object 'anArray', matches. * @@ -75,7 +73,7 @@ public: class ElemMatchObjectMatchExpression final : public ArrayMatchingMatchExpression { public: ElemMatchObjectMatchExpression(StringData path, - MatchExpression* sub, + std::unique_ptr<MatchExpression> sub, clonable_ptr<ErrorAnnotation> annotation = nullptr); bool matchesArray(const BSONObj& anArray, MatchDetails* details) const; @@ -83,7 +81,7 @@ public: virtual std::unique_ptr<MatchExpression> shallowClone() const { std::unique_ptr<ElemMatchObjectMatchExpression> e = std::make_unique<ElemMatchObjectMatchExpression>( - path(), _sub->shallowClone().release(), _errorAnnotation); + path(), _sub->shallowClone(), _errorAnnotation); if (getTag()) { e->setTag(getTag()->clone()); } @@ -94,8 +92,8 @@ public: BSONObj getSerializedRightHandSide() const final; - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } virtual size_t numChildren() const { @@ -130,17 +128,13 @@ private: class ElemMatchValueMatchExpression final : public ArrayMatchingMatchExpression { public: - /** - * This constructor takes ownership of 'sub.' - */ ElemMatchValueMatchExpression(StringData path, - MatchExpression* sub, + std::unique_ptr<MatchExpression> sub, clonable_ptr<ErrorAnnotation> annotation = nullptr); explicit ElemMatchValueMatchExpression(StringData path, clonable_ptr<ErrorAnnotation> annotation = nullptr); - virtual ~ElemMatchValueMatchExpression(); - void add(MatchExpression* sub); + void add(std::unique_ptr<MatchExpression> sub); bool matchesArray(const BSONObj& anArray, MatchDetails* details) const; @@ -148,7 +142,7 @@ public: std::unique_ptr<ElemMatchValueMatchExpression> e = std::make_unique<ElemMatchValueMatchExpression>(path(), _errorAnnotation); for (size_t i = 0; i < _subs.size(); ++i) { - e->add(_subs[i]->shallowClone().release()); + e->add(_subs[i]->shallowClone()); } if (getTag()) { e->setTag(getTag()->clone()); @@ -160,8 +154,8 @@ public: BSONObj getSerializedRightHandSide() const final; - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return _subs; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return &_subs; } virtual size_t numChildren() const { @@ -169,7 +163,7 @@ public: } virtual MatchExpression* getChild(size_t i) const { - return _subs[i]; + return _subs[i].get(); } void acceptVisitor(MatchExpressionMutableVisitor* visitor) final { @@ -185,7 +179,7 @@ private: bool _arrayElementMatchesAll(const BSONElement& e) const; - std::vector<MatchExpression*> _subs; + std::vector<std::unique_ptr<MatchExpression>> _subs; }; class SizeMatchExpression : public ArrayMatchingMatchExpression { @@ -211,8 +205,8 @@ public: return nullptr; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } virtual bool matchesArray(const BSONObj& anArray, MatchDetails* details) const; diff --git a/src/mongo/db/matcher/expression_array_test.cpp b/src/mongo/db/matcher/expression_array_test.cpp index 40e94ef38ee..e1dea305466 100644 --- a/src/mongo/db/matcher/expression_array_test.cpp +++ b/src/mongo/db/matcher/expression_array_test.cpp @@ -40,46 +40,44 @@ namespace mongo { -using std::unique_ptr; - TEST(ElemMatchObjectMatchExpression, MatchesElementSingle) { - BSONObj baseOperand = BSON("b" << 5); - BSONObj match = BSON("a" << BSON_ARRAY(BSON("b" << 5.0))); - BSONObj notMatch = BSON("a" << BSON_ARRAY(BSON("b" << 6))); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("b", baseOperand["b"])); - ElemMatchObjectMatchExpression op("a", eq.release()); + auto baseOperand = BSON("b" << 5); + auto match = BSON("a" << BSON_ARRAY(BSON("b" << 5.0))); + auto notMatch = BSON("a" << BSON_ARRAY(BSON("b" << 6))); + auto eq = std::make_unique<EqualityMatchExpression>("b", baseOperand["b"]); + auto op = ElemMatchObjectMatchExpression{"a", std::move(eq)}; ASSERT(op.matchesSingleElement(match["a"])); ASSERT(!op.matchesSingleElement(notMatch["a"])); } TEST(ElemMatchObjectMatchExpression, MatchesElementArray) { - BSONObj baseOperand = BSON("1" << 5); - BSONObj match = BSON("a" << BSON_ARRAY(BSON_ARRAY('s' << 5.0))); - BSONObj notMatch = BSON("a" << BSON_ARRAY(BSON_ARRAY(5 << 6))); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("1", baseOperand["1"])); - ElemMatchObjectMatchExpression op("a", eq.release()); + auto baseOperand = BSON("1" << 5); + auto match = BSON("a" << BSON_ARRAY(BSON_ARRAY('s' << 5.0))); + auto notMatch = BSON("a" << BSON_ARRAY(BSON_ARRAY(5 << 6))); + auto eq = std::make_unique<EqualityMatchExpression>("1", baseOperand["1"]); + auto op = ElemMatchObjectMatchExpression{"a", std::move(eq)}; ASSERT(op.matchesSingleElement(match["a"])); ASSERT(!op.matchesSingleElement(notMatch["a"])); } TEST(ElemMatchObjectMatchExpression, MatchesElementMultiple) { - BSONObj baseOperand1 = BSON("b" << 5); - BSONObj baseOperand2 = BSON("b" << 6); - BSONObj baseOperand3 = BSON("c" << 7); - BSONObj notMatch1 = BSON("a" << BSON_ARRAY(BSON("b" << 5 << "c" << 7))); - BSONObj notMatch2 = BSON("a" << BSON_ARRAY(BSON("b" << 6 << "c" << 7))); - BSONObj notMatch3 = BSON("a" << BSON_ARRAY(BSON("b" << BSON_ARRAY(5 << 6)))); - BSONObj match = BSON("a" << BSON_ARRAY(BSON("b" << BSON_ARRAY(5 << 6) << "c" << 7))); - unique_ptr<ComparisonMatchExpression> eq1(new EqualityMatchExpression("b", baseOperand1["b"])); - unique_ptr<ComparisonMatchExpression> eq2(new EqualityMatchExpression("b", baseOperand2["b"])); - unique_ptr<ComparisonMatchExpression> eq3(new EqualityMatchExpression("c", baseOperand3["c"])); - - unique_ptr<AndMatchExpression> andOp(new AndMatchExpression()); - andOp->add(eq1.release()); - andOp->add(eq2.release()); - andOp->add(eq3.release()); - - ElemMatchObjectMatchExpression op("a", andOp.release()); + auto baseOperand1 = BSON("b" << 5); + auto baseOperand2 = BSON("b" << 6); + auto baseOperand3 = BSON("c" << 7); + auto notMatch1 = BSON("a" << BSON_ARRAY(BSON("b" << 5 << "c" << 7))); + auto notMatch2 = BSON("a" << BSON_ARRAY(BSON("b" << 6 << "c" << 7))); + auto notMatch3 = BSON("a" << BSON_ARRAY(BSON("b" << BSON_ARRAY(5 << 6)))); + auto match = BSON("a" << BSON_ARRAY(BSON("b" << BSON_ARRAY(5 << 6) << "c" << 7))); + auto eq1 = std::make_unique<EqualityMatchExpression>("b", baseOperand1["b"]); + auto eq2 = std::make_unique<EqualityMatchExpression>("b", baseOperand2["b"]); + auto eq3 = std::make_unique<EqualityMatchExpression>("c", baseOperand3["c"]); + + auto andOp = std::make_unique<AndMatchExpression>(); + andOp->add(std::move(eq1)); + andOp->add(std::move(eq2)); + andOp->add(std::move(eq3)); + + auto op = ElemMatchObjectMatchExpression{"a", std::move(andOp)}; ASSERT(!op.matchesSingleElement(notMatch1["a"])); ASSERT(!op.matchesSingleElement(notMatch2["a"])); ASSERT(!op.matchesSingleElement(notMatch3["a"])); @@ -87,9 +85,9 @@ TEST(ElemMatchObjectMatchExpression, MatchesElementMultiple) { } TEST(ElemMatchObjectMatchExpression, MatchesNonArray) { - BSONObj baseOperand = BSON("b" << 5); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("b", baseOperand["b"])); - ElemMatchObjectMatchExpression op("a", eq.release()); + auto baseOperand = BSON("b" << 5); + auto eq = std::make_unique<EqualityMatchExpression>("b", baseOperand["b"]); + auto op = ElemMatchObjectMatchExpression{"a", std::move(eq)}; // Directly nested objects are not matched with $elemMatch. An intervening array is // required. ASSERT(!op.matchesBSON(BSON("a" << BSON("b" << 5)), nullptr)); @@ -98,9 +96,9 @@ TEST(ElemMatchObjectMatchExpression, MatchesNonArray) { } TEST(ElemMatchObjectMatchExpression, MatchesArrayObject) { - BSONObj baseOperand = BSON("b" << 5); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("b", baseOperand["b"])); - ElemMatchObjectMatchExpression op("a", eq.release()); + auto baseOperand = BSON("b" << 5); + auto eq = std::make_unique<EqualityMatchExpression>("b", baseOperand["b"]); + auto op = ElemMatchObjectMatchExpression{"a", std::move(eq)}; ASSERT(op.matchesBSON(BSON("a" << BSON_ARRAY(BSON("b" << 5))), nullptr)); ASSERT(op.matchesBSON(BSON("a" << BSON_ARRAY(4 << BSON("b" << 5))), nullptr)); ASSERT(op.matchesBSON(BSON("a" << BSON_ARRAY(BSONObj() << BSON("b" << 5))), nullptr)); @@ -108,9 +106,9 @@ TEST(ElemMatchObjectMatchExpression, MatchesArrayObject) { } TEST(ElemMatchObjectMatchExpression, MatchesMultipleNamedValues) { - BSONObj baseOperand = BSON("c" << 5); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("c", baseOperand["c"])); - ElemMatchObjectMatchExpression op("a.b", eq.release()); + auto baseOperand = BSON("c" << 5); + auto eq = std::make_unique<EqualityMatchExpression>("c", baseOperand["c"]); + auto op = ElemMatchObjectMatchExpression{"a.b", std::move(eq)}; ASSERT( op.matchesBSON(BSON("a" << BSON_ARRAY(BSON("b" << BSON_ARRAY(BSON("c" << 5))))), nullptr)); ASSERT(op.matchesBSON(BSON("a" << BSON_ARRAY(BSON("b" << BSON_ARRAY(BSON("c" << 1))) @@ -119,10 +117,10 @@ TEST(ElemMatchObjectMatchExpression, MatchesMultipleNamedValues) { } TEST(ElemMatchObjectMatchExpression, ElemMatchKey) { - BSONObj baseOperand = BSON("c" << 6); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("c", baseOperand["c"])); - ElemMatchObjectMatchExpression op("a.b", eq.release()); - MatchDetails details; + auto baseOperand = BSON("c" << 6); + auto eq = std::make_unique<EqualityMatchExpression>("c", baseOperand["c"]); + auto op = ElemMatchObjectMatchExpression{"a.b", std::move(eq)}; + auto details = MatchDetails{}; details.requestElemMatchKey(); ASSERT(!op.matchesBSON(BSONObj(), &details)); ASSERT(!details.hasElemMatchKey()); @@ -141,56 +139,56 @@ TEST(ElemMatchObjectMatchExpression, ElemMatchKey) { } TEST(ElemMatchObjectMatchExpression, Collation) { - BSONObj baseOperand = BSON("b" - << "string"); - BSONObj match = BSON("a" << BSON_ARRAY(BSON("b" - << "string"))); - BSONObj notMatch = BSON("a" << BSON_ARRAY(BSON("b" - << "string2"))); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("b", baseOperand["b"])); - ElemMatchObjectMatchExpression op("a", eq.release()); - CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual); + auto baseOperand = BSON("b" + << "string"); + auto match = BSON("a" << BSON_ARRAY(BSON("b" + << "string"))); + auto notMatch = BSON("a" << BSON_ARRAY(BSON("b" + << "string2"))); + auto eq = std::make_unique<EqualityMatchExpression>("b", baseOperand["b"]); + auto op = ElemMatchObjectMatchExpression{"a", std::move(eq)}; + auto collator = CollatorInterfaceMock{CollatorInterfaceMock::MockType::kAlwaysEqual}; op.setCollator(&collator); ASSERT(op.matchesSingleElement(match["a"])); ASSERT(op.matchesSingleElement(notMatch["a"])); } /** -TEST( ElemMatchObjectMatchExpression, MatchesIndexKey ) { - BSONObj baseOperand = BSON( "b" << 5 ); - unique_ptr<ComparisonMatchExpression> eq( new ComparisonMatchExpression() ); - ASSERT( eq->init( "b", baseOperand[ "b" ] ).isOK() ); - ElemMatchObjectMatchExpression op; - ASSERT( op.init( "a", eq.release() ).isOK() ); - IndexSpec indexSpec( BSON( "a.b" << 1 ) ); - BSONObj indexKey = BSON( "" << "5" ); - ASSERT( MatchMatchExpression::PartialMatchResult_Unknown == - op.matchesIndexKey( indexKey, indexSpec ) ); +TEST(ElemMatchObjectMatchExpression, MatchesIndexKey) { + auto baseOperand = BSON("b" << 5); + auto eq = std::make_unique<EqualityMatchExpression>(); + ASSERT(eq->init("b", baseOperand["b"]).isOK()); + auto op = ElemMatchObjectMatchExpression{}; + ASSERT(op.init("a", std::move(eq)).isOK()); + auto indexSpec = IndexSpec{BSON("a.b" << 1)}; + auto indexKey = BSON("" << "5"); + ASSERT(MatchMatchExpression::PartialMatchResult_Unknown == + op.matchesIndexKey(indexKey, indexSpec)); } */ TEST(ElemMatchValueMatchExpression, MatchesElementSingle) { - BSONObj baseOperand = BSON("$gt" << 5); - BSONObj match = BSON("a" << BSON_ARRAY(6)); - BSONObj notMatch = BSON("a" << BSON_ARRAY(4)); - unique_ptr<ComparisonMatchExpression> gt(new GTMatchExpression("", baseOperand["$gt"])); - ElemMatchValueMatchExpression op("a", gt.release()); + auto baseOperand = BSON("$gt" << 5); + auto match = BSON("a" << BSON_ARRAY(6)); + auto notMatch = BSON("a" << BSON_ARRAY(4)); + auto gt = std::make_unique<GTMatchExpression>("", baseOperand["$gt"]); + auto op = ElemMatchValueMatchExpression{"a", std::unique_ptr<MatchExpression>{std::move(gt)}}; ASSERT(op.matchesSingleElement(match["a"])); ASSERT(!op.matchesSingleElement(notMatch["a"])); } TEST(ElemMatchValueMatchExpression, MatchesElementMultiple) { - BSONObj baseOperand1 = BSON("$gt" << 1); - BSONObj baseOperand2 = BSON("$lt" << 10); - BSONObj notMatch1 = BSON("a" << BSON_ARRAY(0 << 1)); - BSONObj notMatch2 = BSON("a" << BSON_ARRAY(10 << 11)); - BSONObj match = BSON("a" << BSON_ARRAY(0 << 5 << 11)); - unique_ptr<ComparisonMatchExpression> gt(new GTMatchExpression("", baseOperand1["$gt"])); - unique_ptr<ComparisonMatchExpression> lt(new LTMatchExpression("", baseOperand2["$lt"])); - - ElemMatchValueMatchExpression op("a"); - op.add(gt.release()); - op.add(lt.release()); + auto baseOperand1 = BSON("$gt" << 1); + auto baseOperand2 = BSON("$lt" << 10); + auto notMatch1 = BSON("a" << BSON_ARRAY(0 << 1)); + auto notMatch2 = BSON("a" << BSON_ARRAY(10 << 11)); + auto match = BSON("a" << BSON_ARRAY(0 << 5 << 11)); + auto gt = std::make_unique<GTMatchExpression>("", baseOperand1["$gt"]); + auto lt = std::make_unique<LTMatchExpression>("", baseOperand2["$lt"]); + + auto op = ElemMatchValueMatchExpression{"a"}; + op.add(std::move(gt)); + op.add(std::move(lt)); ASSERT(!op.matchesSingleElement(notMatch1["a"])); ASSERT(!op.matchesSingleElement(notMatch2["a"])); @@ -198,9 +196,9 @@ TEST(ElemMatchValueMatchExpression, MatchesElementMultiple) { } TEST(ElemMatchValueMatchExpression, MatchesNonArray) { - BSONObj baseOperand = BSON("$gt" << 5); - unique_ptr<ComparisonMatchExpression> gt(new GTMatchExpression("", baseOperand["$gt"])); - ElemMatchObjectMatchExpression op("a", gt.release()); + auto baseOperand = BSON("$gt" << 5); + auto gt = std::make_unique<GTMatchExpression>("", baseOperand["$gt"]); + auto op = ElemMatchObjectMatchExpression("a", std::move(gt)); // Directly nested objects are not matched with $elemMatch. An intervening array is // required. ASSERT(!op.matchesBSON(BSON("a" << 6), nullptr)); @@ -208,18 +206,18 @@ TEST(ElemMatchValueMatchExpression, MatchesNonArray) { } TEST(ElemMatchValueMatchExpression, MatchesArrayScalar) { - BSONObj baseOperand = BSON("$gt" << 5); - unique_ptr<ComparisonMatchExpression> gt(new GTMatchExpression("", baseOperand["$gt"])); - ElemMatchValueMatchExpression op("a", gt.release()); + auto baseOperand = BSON("$gt" << 5); + auto gt = std::make_unique<GTMatchExpression>("", baseOperand["$gt"]); + auto op = ElemMatchValueMatchExpression{"a", std::unique_ptr<MatchExpression>{std::move(gt)}}; ASSERT(op.matchesBSON(BSON("a" << BSON_ARRAY(6)), nullptr)); ASSERT(op.matchesBSON(BSON("a" << BSON_ARRAY(4 << 6)), nullptr)); ASSERT(op.matchesBSON(BSON("a" << BSON_ARRAY(BSONObj() << 7)), nullptr)); } TEST(ElemMatchValueMatchExpression, MatchesMultipleNamedValues) { - BSONObj baseOperand = BSON("$gt" << 5); - unique_ptr<ComparisonMatchExpression> gt(new GTMatchExpression("", baseOperand["$gt"])); - ElemMatchValueMatchExpression op("a.b", gt.release()); + auto baseOperand = BSON("$gt" << 5); + auto gt = std::make_unique<GTMatchExpression>("", baseOperand["$gt"]); + auto op = ElemMatchValueMatchExpression{"a.b", std::unique_ptr<MatchExpression>{std::move(gt)}}; ASSERT(op.matchesBSON(BSON("a" << BSON_ARRAY(BSON("b" << BSON_ARRAY(6)))), nullptr)); ASSERT(op.matchesBSON( BSON("a" << BSON_ARRAY(BSON("b" << BSON_ARRAY(4)) << BSON("b" << BSON_ARRAY(4 << 6)))), @@ -227,10 +225,10 @@ TEST(ElemMatchValueMatchExpression, MatchesMultipleNamedValues) { } TEST(ElemMatchValueMatchExpression, ElemMatchKey) { - BSONObj baseOperand = BSON("$gt" << 6); - unique_ptr<ComparisonMatchExpression> gt(new GTMatchExpression("", baseOperand["$gt"])); - ElemMatchValueMatchExpression op("a.b", gt.release()); - MatchDetails details; + auto baseOperand = BSON("$gt" << 6); + auto gt = std::make_unique<GTMatchExpression>("", baseOperand["$gt"]); + auto op = ElemMatchValueMatchExpression{"a.b", std::unique_ptr<MatchExpression>{std::move(gt)}}; + auto details = MatchDetails{}; details.requestElemMatchKey(); ASSERT(!op.matchesBSON(BSONObj(), &details)); ASSERT(!details.hasElemMatchKey()); @@ -248,155 +246,149 @@ TEST(ElemMatchValueMatchExpression, ElemMatchKey) { } /** -TEST( ElemMatchValueMatchExpression, MatchesIndexKey ) { - BSONObj baseOperand = BSON( "$lt" << 5 ); - unique_ptr<LtOp> lt( new ComparisonMatchExpression() ); - ASSERT( lt->init( "a", baseOperand[ "$lt" ] ).isOK() ); - ElemMatchValueMatchExpression op; - ASSERT( op.init( "a", lt.release() ).isOK() ); - IndexSpec indexSpec( BSON( "a" << 1 ) ); - BSONObj indexKey = BSON( "" << "3" ); - ASSERT( MatchMatchExpression::PartialMatchResult_Unknown == - op.matchesIndexKey( indexKey, indexSpec ) ); +TEST(ElemMatchValueMatchExpression, MatchesIndexKey) { + auto baseOperand = BSON("$lt" << 5); + auto lt = std::make_unique<LtOp>(); + ASSERT(lt->init("a", baseOperand["$lt"]).isOK()); + auto op = ElemMatchValueMatchExpression{}; + ASSERT(op.init("a", std::move(lt)).isOK()); + auto indexSpec = IndexSpec{BSON("a" << 1)}; + auto indexKey = BSON("" << "3"); + ASSERT(MatchMatchExpression::PartialMatchResult_Unknown == + op.matchesIndexKey(indexKey, indexSpec)); } */ TEST(AndOfElemMatch, MatchesElement) { - BSONObj baseOperanda1 = BSON("a" << 1); - unique_ptr<ComparisonMatchExpression> eqa1( - new EqualityMatchExpression("a", baseOperanda1["a"])); + auto baseOperanda1 = BSON("a" << 1); + auto eqa1 = std::make_unique<EqualityMatchExpression>("a", baseOperanda1["a"]); - BSONObj baseOperandb1 = BSON("b" << 1); - unique_ptr<ComparisonMatchExpression> eqb1( - new EqualityMatchExpression("b", baseOperandb1["b"])); + auto baseOperandb1 = BSON("b" << 1); + auto eqb1 = std::make_unique<EqualityMatchExpression>("b", baseOperandb1["b"]); - unique_ptr<AndMatchExpression> and1(new AndMatchExpression()); - and1->add(eqa1.release()); - and1->add(eqb1.release()); + auto and1 = std::make_unique<AndMatchExpression>(); + and1->add(std::move(eqa1)); + and1->add(std::move(eqb1)); // and1 = { a : 1, b : 1 } - unique_ptr<ElemMatchObjectMatchExpression> elemMatch1( - new ElemMatchObjectMatchExpression("x", and1.release())); + auto elemMatch1 = std::make_unique<ElemMatchObjectMatchExpression>("x", std::move(and1)); // elemMatch1 = { x : { $elemMatch : { a : 1, b : 1 } } } - BSONObj baseOperanda2 = BSON("a" << 2); - unique_ptr<ComparisonMatchExpression> eqa2( - new EqualityMatchExpression("a", baseOperanda2["a"])); + auto baseOperanda2 = BSON("a" << 2); + auto eqa2 = std::make_unique<EqualityMatchExpression>("a", baseOperanda2["a"]); - BSONObj baseOperandb2 = BSON("b" << 2); - unique_ptr<ComparisonMatchExpression> eqb2( - new EqualityMatchExpression("b", baseOperandb2["b"])); + auto baseOperandb2 = BSON("b" << 2); + auto eqb2 = std::make_unique<EqualityMatchExpression>("b", baseOperandb2["b"]); - unique_ptr<AndMatchExpression> and2(new AndMatchExpression()); - and2->add(eqa2.release()); - and2->add(eqb2.release()); + auto and2 = std::make_unique<AndMatchExpression>(); + and2->add(std::move(eqa2)); + and2->add(std::move(eqb2)); // and2 = { a : 2, b : 2 } - unique_ptr<ElemMatchObjectMatchExpression> elemMatch2( - new ElemMatchObjectMatchExpression("x", and2.release())); + auto elemMatch2 = std::make_unique<ElemMatchObjectMatchExpression>("x", std::move(and2)); // elemMatch2 = { x : { $elemMatch : { a : 2, b : 2 } } } - unique_ptr<AndMatchExpression> andOfEM(new AndMatchExpression()); - andOfEM->add(elemMatch1.release()); - andOfEM->add(elemMatch2.release()); + auto andOfEM = std::make_unique<AndMatchExpression>(); + andOfEM->add(std::move(elemMatch1)); + andOfEM->add(std::move(elemMatch2)); - BSONObj nonArray = BSON("x" << 4); + auto nonArray = BSON("x" << 4); ASSERT(!andOfEM->matchesSingleElement(nonArray["x"])); - BSONObj emptyArray = BSON("x" << BSONArray()); + auto emptyArray = BSON("x" << BSONArray()); ASSERT(!andOfEM->matchesSingleElement(emptyArray["x"])); - BSONObj nonObjArray = BSON("x" << BSON_ARRAY(4)); + auto nonObjArray = BSON("x" << BSON_ARRAY(4)); ASSERT(!andOfEM->matchesSingleElement(nonObjArray["x"])); - BSONObj singleObjMatch = BSON("x" << BSON_ARRAY(BSON("a" << 1 << "b" << 1))); + auto singleObjMatch = BSON("x" << BSON_ARRAY(BSON("a" << 1 << "b" << 1))); ASSERT(!andOfEM->matchesSingleElement(singleObjMatch["x"])); - BSONObj otherObjMatch = BSON("x" << BSON_ARRAY(BSON("a" << 2 << "b" << 2))); + auto otherObjMatch = BSON("x" << BSON_ARRAY(BSON("a" << 2 << "b" << 2))); ASSERT(!andOfEM->matchesSingleElement(otherObjMatch["x"])); - BSONObj bothObjMatch = + auto bothObjMatch = BSON("x" << BSON_ARRAY(BSON("a" << 1 << "b" << 1) << BSON("a" << 2 << "b" << 2))); ASSERT(andOfEM->matchesSingleElement(bothObjMatch["x"])); - BSONObj noObjMatch = + auto noObjMatch = BSON("x" << BSON_ARRAY(BSON("a" << 1 << "b" << 2) << BSON("a" << 2 << "b" << 1))); ASSERT(!andOfEM->matchesSingleElement(noObjMatch["x"])); } TEST(AndOfElemMatch, Matches) { - BSONObj baseOperandgt1 = BSON("$gt" << 1); - unique_ptr<ComparisonMatchExpression> gt1(new GTMatchExpression("", baseOperandgt1["$gt"])); + auto baseOperandgt1 = BSON("$gt" << 1); + auto gt1 = std::make_unique<GTMatchExpression>("", baseOperandgt1["$gt"]); - BSONObj baseOperandlt1 = BSON("$lt" << 10); - unique_ptr<ComparisonMatchExpression> lt1(new LTMatchExpression("", baseOperandlt1["$lt"])); + auto baseOperandlt1 = BSON("$lt" << 10); + auto lt1 = std::make_unique<LTMatchExpression>("", baseOperandlt1["$lt"]); - unique_ptr<ElemMatchValueMatchExpression> elemMatch1(new ElemMatchValueMatchExpression("x")); - elemMatch1->add(gt1.release()); - elemMatch1->add(lt1.release()); + auto elemMatch1 = std::make_unique<ElemMatchValueMatchExpression>("x"); + elemMatch1->add(std::move(gt1)); + elemMatch1->add(std::move(lt1)); // elemMatch1 = { x : { $elemMatch : { $gt : 1 , $lt : 10 } } } - BSONObj baseOperandgt2 = BSON("$gt" << 101); - unique_ptr<ComparisonMatchExpression> gt2(new GTMatchExpression("", baseOperandgt2["$gt"])); + auto baseOperandgt2 = BSON("$gt" << 101); + auto gt2 = std::make_unique<GTMatchExpression>("", baseOperandgt2["$gt"]); - BSONObj baseOperandlt2 = BSON("$lt" << 110); - unique_ptr<ComparisonMatchExpression> lt2(new LTMatchExpression("", baseOperandlt2["$lt"])); + auto baseOperandlt2 = BSON("$lt" << 110); + auto lt2 = std::make_unique<LTMatchExpression>("", baseOperandlt2["$lt"]); - unique_ptr<ElemMatchValueMatchExpression> elemMatch2(new ElemMatchValueMatchExpression("x")); - elemMatch2->add(gt2.release()); - elemMatch2->add(lt2.release()); + auto elemMatch2 = std::make_unique<ElemMatchValueMatchExpression>("x"); + elemMatch2->add(std::move(gt2)); + elemMatch2->add(std::move(lt2)); // elemMatch2 = { x : { $elemMatch : { $gt : 101 , $lt : 110 } } } - unique_ptr<AndMatchExpression> andOfEM(new AndMatchExpression()); - andOfEM->add(elemMatch1.release()); - andOfEM->add(elemMatch2.release()); + auto andOfEM = std::make_unique<AndMatchExpression>(); + andOfEM->add(std::move(elemMatch1)); + andOfEM->add(std::move(elemMatch2)); - BSONObj nonArray = BSON("x" << 4); + auto nonArray = BSON("x" << 4); ASSERT(!andOfEM->matchesBSON(nonArray, nullptr)); - BSONObj emptyArray = BSON("x" << BSONArray()); + auto emptyArray = BSON("x" << BSONArray()); ASSERT(!andOfEM->matchesBSON(emptyArray, nullptr)); - BSONObj nonNumberArray = BSON("x" << BSON_ARRAY("q")); + auto nonNumberArray = BSON("x" << BSON_ARRAY("q")); ASSERT(!andOfEM->matchesBSON(nonNumberArray, nullptr)); - BSONObj singleMatch = BSON("x" << BSON_ARRAY(5)); + auto singleMatch = BSON("x" << BSON_ARRAY(5)); ASSERT(!andOfEM->matchesBSON(singleMatch, nullptr)); - BSONObj otherMatch = BSON("x" << BSON_ARRAY(105)); + auto otherMatch = BSON("x" << BSON_ARRAY(105)); ASSERT(!andOfEM->matchesBSON(otherMatch, nullptr)); - BSONObj bothMatch = BSON("x" << BSON_ARRAY(5 << 105)); + auto bothMatch = BSON("x" << BSON_ARRAY(5 << 105)); ASSERT(andOfEM->matchesBSON(bothMatch, nullptr)); - BSONObj neitherMatch = BSON("x" << BSON_ARRAY(0 << 200)); + auto neitherMatch = BSON("x" << BSON_ARRAY(0 << 200)); ASSERT(!andOfEM->matchesBSON(neitherMatch, nullptr)); } TEST(SizeMatchExpression, MatchesElement) { - BSONObj match = BSON("a" << BSON_ARRAY(5 << 6)); - BSONObj notMatch = BSON("a" << BSON_ARRAY(5)); - SizeMatchExpression size("", 2); + auto match = BSON("a" << BSON_ARRAY(5 << 6)); + auto notMatch = BSON("a" << BSON_ARRAY(5)); + auto size = SizeMatchExpression{"", 2}; ASSERT(size.matchesSingleElement(match.firstElement())); ASSERT(!size.matchesSingleElement(notMatch.firstElement())); } TEST(SizeMatchExpression, MatchesNonArray) { // Non arrays do not match. - BSONObj stringValue = BSON("a" - << "z"); - BSONObj numberValue = BSON("a" << 0); - BSONObj arrayValue = BSON("a" << BSONArray()); - SizeMatchExpression size("", 0); + auto stringValue = BSON("a" + << "z"); + auto numberValue = BSON("a" << 0); + auto arrayValue = BSON("a" << BSONArray()); + auto size = SizeMatchExpression{"", 0}; ASSERT(!size.matchesSingleElement(stringValue.firstElement())); ASSERT(!size.matchesSingleElement(numberValue.firstElement())); ASSERT(size.matchesSingleElement(arrayValue.firstElement())); } TEST(SizeMatchExpression, MatchesArray) { - SizeMatchExpression size("a", 2); + auto size = SizeMatchExpression{"a", 2}; ASSERT(size.matchesBSON(BSON("a" << BSON_ARRAY(4 << 5.5)), nullptr)); // Arrays are not unwound to look for matching subarrays. ASSERT(!size.matchesBSON(BSON("a" << BSON_ARRAY(4 << 5.5 << BSON_ARRAY(1 << 2))), nullptr)); } TEST(SizeMatchExpression, MatchesNestedArray) { - SizeMatchExpression size("a.2", 2); + auto size = SizeMatchExpression{"a.2", 2}; // A numerically referenced nested array is matched. ASSERT(size.matchesBSON(BSON("a" << BSON_ARRAY(4 << 5.5 << BSON_ARRAY(1 << 2))), nullptr)); } TEST(SizeMatchExpression, ElemMatchKey) { - SizeMatchExpression size("a.b", 3); - MatchDetails details; + auto size = SizeMatchExpression{"a.b", 3}; + auto details = MatchDetails{}; details.requestElemMatchKey(); ASSERT(!size.matchesBSON(BSON("a" << 1), &details)); ASSERT(!details.hasElemMatchKey()); @@ -409,9 +401,9 @@ TEST(SizeMatchExpression, ElemMatchKey) { } TEST(SizeMatchExpression, Equivalent) { - SizeMatchExpression e1("a", 5); - SizeMatchExpression e2("a", 6); - SizeMatchExpression e3("v", 5); + auto e1 = SizeMatchExpression{"a", 5}; + auto e2 = SizeMatchExpression{"a", 6}; + auto e3 = SizeMatchExpression{"v", 5}; ASSERT(e1.equivalent(&e1)); ASSERT(!e1.equivalent(&e2)); @@ -419,14 +411,14 @@ TEST(SizeMatchExpression, Equivalent) { } /** - TEST( SizeMatchExpression, MatchesIndexKey ) { - BSONObj operand = BSON( "$size" << 4 ); - SizeMatchExpression size; - ASSERT( size.init( "a", operand[ "$size" ] ).isOK() ); - IndexSpec indexSpec( BSON( "a" << 1 ) ); - BSONObj indexKey = BSON( "" << 1 ); - ASSERT( MatchMatchExpression::PartialMatchResult_Unknown == - size.matchesIndexKey( indexKey, indexSpec ) ); + TEST(SizeMatchExpression, MatchesIndexKey) { + auto operand = BSON("$size" << 4); + auto size = SizeMatchExpression{}; + ASSERT(size.init("a", operand["$size"]).isOK()); + auto indexSpec = IndexSpec{BSON("a" << 1)}; + auto indexKey = BSON("" << 1); + ASSERT(MatchMatchExpression::PartialMatchResult_Unknown == + size.matchesIndexKey(indexKey, indexSpec)); } */ diff --git a/src/mongo/db/matcher/expression_expr.cpp b/src/mongo/db/matcher/expression_expr.cpp index 7e08b8fb688..567039995f6 100644 --- a/src/mongo/db/matcher/expression_expr.cpp +++ b/src/mongo/db/matcher/expression_expr.cpp @@ -144,8 +144,8 @@ MatchExpression::ExpressionOptimizerFunc ExprMatchExpression::getOptimizer() con if (exprMatchExpr._rewriteResult->matchExpression()) { auto andMatch = std::make_unique<AndMatchExpression>(); - andMatch->add(exprMatchExpr._rewriteResult->releaseMatchExpression().release()); - andMatch->add(expression.release()); + andMatch->add(exprMatchExpr._rewriteResult->releaseMatchExpression()); + andMatch->add(std::move(expression)); // Re-optimize the new AND in order to make sure that any AND children are absorbed. expression = MatchExpression::optimize(std::move(andMatch)); } diff --git a/src/mongo/db/matcher/expression_expr.h b/src/mongo/db/matcher/expression_expr.h index 73a5d99f46b..d9bd3af7608 100644 --- a/src/mongo/db/matcher/expression_expr.h +++ b/src/mongo/db/matcher/expression_expr.h @@ -90,8 +90,8 @@ public: MONGO_UNREACHABLE; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } boost::intrusive_ptr<ExpressionContext> getExpressionContext() const { diff --git a/src/mongo/db/matcher/expression_leaf.h b/src/mongo/db/matcher/expression_leaf.h index e042e6d99fb..fe6878eb05d 100644 --- a/src/mongo/db/matcher/expression_leaf.h +++ b/src/mongo/db/matcher/expression_leaf.h @@ -78,8 +78,8 @@ public: MONGO_UNREACHABLE; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } MatchCategory getCategory() const override { diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index cb2e31c67cb..51264e85aab 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -292,9 +292,8 @@ StatusWithMatchExpression parse(const BSONObj& obj, // be added to 'root', because it is handled outside of the MatchExpressionParser // library. The following operators currently follow this convention: // - $comment has no action associated with the operator. - if (parsedExpression.getValue().get()) { - root->add(parsedExpression.getValue().release()); - } + if (auto&& expr = parsedExpression.getValue()) + root->add(std::move(expr)); continue; } @@ -336,11 +335,8 @@ StatusWithMatchExpression parse(const BSONObj& obj, addExpressionToRoot(expCtx, root.get(), std::move(eq.getValue())); } - if (root->numChildren() == 1) { - std::unique_ptr<MatchExpression> real(root->getChild(0)); - root->clearAndRelease(); - return {std::move(real)}; - } + if (root->numChildren() == 1) + return {std::move((*root->getChildVector())[0])}; return {std::move(root)}; } @@ -1199,7 +1195,7 @@ StatusWithMatchExpression parseTreeTopLevel( if (!sub.isOK()) return sub.getStatus(); - temp->add(sub.getValue().release()); + temp->add(std::move(sub.getValue())); } if constexpr (std::is_same_v<T, InternalSchemaXorMatchExpression>) { @@ -1254,10 +1250,9 @@ StatusWithMatchExpression parseElemMatch(StringData name, expCtx, e.fieldNameStringData().toString(), BSON(name << e.wrap()))); doc_validation_error::annotateTreeToIgnoreForErrorDetails(expCtx, &theAnd); - for (size_t i = 0; i < theAnd.numChildren(); i++) { - emValueExpr->add(theAnd.getChild(i)); - } - theAnd.clearAndRelease(); + for (size_t i = 0; i < theAnd.numChildren(); i++) + emValueExpr->add(theAnd.releaseChild(i)); + theAnd.clear(); return {std::move(emValueExpr)}; } @@ -1284,7 +1279,7 @@ StatusWithMatchExpression parseElemMatch(StringData name, return {std::make_unique<ElemMatchObjectMatchExpression>( name, - sub.release(), + std::move(sub), doc_validation_error::createAnnotation( expCtx, e.fieldNameStringData().toString(), BSON(name << e.wrap())))}; } @@ -1329,7 +1324,7 @@ StatusWithMatchExpression parseAll(StringData name, return inner; doc_validation_error::annotateTreeToIgnoreForErrorDetails(expCtx, inner.getValue().get()); - myAnd->add(inner.getValue().release()); + myAnd->add(std::move(inner.getValue())); } return {std::move(myAnd)}; @@ -1341,7 +1336,7 @@ StatusWithMatchExpression parseAll(StringData name, if (e.type() == BSONType::RegEx) { auto expr = std::make_unique<RegexMatchExpression>( name, e, doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnore)); - myAnd->add(expr.release()); + myAnd->add(std::move(expr)); } else if (e.type() == BSONType::Object && MatchExpressionParser::parsePathAcceptingKeyword(e.Obj().firstElement())) { return {Status(ErrorCodes::BadValue, "no $ expressions in $all")}; @@ -1349,7 +1344,7 @@ StatusWithMatchExpression parseAll(StringData name, auto expr = std::make_unique<EqualityMatchExpression>( name, e, doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnore)); expr->setCollator(expCtx->getCollator()); - myAnd->add(expr.release()); + myAnd->add(std::move(expr)); } } diff --git a/src/mongo/db/matcher/expression_path.h b/src/mongo/db/matcher/expression_path.h index 4604e57e81b..16182705a62 100644 --- a/src/mongo/db/matcher/expression_path.h +++ b/src/mongo/db/matcher/expression_path.h @@ -51,8 +51,6 @@ public: : MatchExpression(matchType, std::move(annotation)), _elementPath(path, leafArrBehavior, nonLeafArrayBehavior) {} - virtual ~PathMatchExpression() {} - bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const final { MatchableDocument::IteratorHolder cursor(doc, &_elementPath); while (cursor->more()) { diff --git a/src/mongo/db/matcher/expression_tree.cpp b/src/mongo/db/matcher/expression_tree.cpp index 355156eee66..92fab31668b 100644 --- a/src/mongo/db/matcher/expression_tree.cpp +++ b/src/mongo/db/matcher/expression_tree.cpp @@ -27,6 +27,9 @@ * it in the license file. */ +#include <algorithm> +#include <iterator> + #include "mongo/db/matcher/expression_tree.h" #include "mongo/bson/bsonmisc.h" @@ -38,19 +41,6 @@ namespace mongo { -ListOfMatchExpression::~ListOfMatchExpression() { - for (unsigned i = 0; i < _expressions.size(); i++) { - delete _expressions[i]; - } - _expressions.clear(); -} - -void ListOfMatchExpression::add(MatchExpression* e) { - verify(e); - _expressions.push_back(e); -} - - void ListOfMatchExpression::_debugList(StringBuilder& debug, int indentationLevel) const { for (unsigned i = 0; i < _expressions.size(); i++) _expressions[i]->debugString(debug, indentationLevel + 1); @@ -69,37 +59,28 @@ MatchExpression::ExpressionOptimizerFunc ListOfMatchExpression::getOptimizer() c auto& children = static_cast<ListOfMatchExpression&>(*expression)._expressions; // Recursively apply optimizations to child expressions. - for (auto& childExpression : children) { - // Since 'childExpression' is a reference to a member of the ListOfMatchExpression's - // child array, this assignment replaces the original child with the optimized child. - // We must set this child's entry in '_expressions' to null after assigning ownership to - // 'childExpressionPtr'. Otherwise, if the call to optimize() throws we will attempt to - // free twice. - std::unique_ptr<MatchExpression> childExpressionPtr(childExpression); - childExpression = nullptr; - - auto optimizedExpression = MatchExpression::optimize(std::move(childExpressionPtr)); - childExpression = optimizedExpression.release(); - } + for (auto& childExpression : children) + childExpression = MatchExpression::optimize(std::move(childExpression)); // Associativity of AND and OR: an AND absorbs the children of any ANDs among its children // (and likewise for any OR with OR children). MatchType matchType = expression->matchType(); if (matchType == AND || matchType == OR) { - std::vector<MatchExpression*> absorbedExpressions; - for (MatchExpression*& childExpression : children) { + auto absorbedExpressions = std::vector<std::unique_ptr<MatchExpression>>{}; + for (auto& childExpression : children) { if (childExpression->matchType() == matchType) { // Move this child out of the children array. - std::unique_ptr<ListOfMatchExpression> childExpressionPtr( - static_cast<ListOfMatchExpression*>(childExpression)); + auto childExpressionPtr = std::move(childExpression); childExpression = nullptr; // Null out this child's entry in _expressions, so // that it will be deleted by the erase call below. // Move all of the grandchildren from the child expression to // absorbedExpressions. - auto& grandChildren = childExpressionPtr->_expressions; - absorbedExpressions.insert( - absorbedExpressions.end(), grandChildren.begin(), grandChildren.end()); + auto& grandChildren = + static_cast<ListOfMatchExpression&>(*childExpressionPtr)._expressions; + std::move(grandChildren.begin(), + grandChildren.end(), + std::back_inserter(absorbedExpressions)); grandChildren.clear(); // Note that 'childExpressionPtr' will now be destroyed. @@ -111,19 +92,18 @@ MatchExpression::ExpressionOptimizerFunc ListOfMatchExpression::getOptimizer() c children.erase(std::remove(children.begin(), children.end(), nullptr), children.end()); // Append the absorbed children to the end of the array. - children.insert(children.end(), absorbedExpressions.begin(), absorbedExpressions.end()); + std::move(absorbedExpressions.begin(), + absorbedExpressions.end(), + std::back_inserter(children)); } // Remove all children of AND that are $alwaysTrue and all children of OR that are // $alwaysFalse. if (matchType == AND || matchType == OR) { - for (MatchExpression*& childExpression : children) { + for (auto& childExpression : children) if ((childExpression->isTriviallyTrue() && matchType == MatchExpression::AND) || - (childExpression->isTriviallyFalse() && matchType == MatchExpression::OR)) { - std::unique_ptr<MatchExpression> childPtr(childExpression); + (childExpression->isTriviallyFalse() && matchType == MatchExpression::OR)) childExpression = nullptr; - } - } // We replaced each destroyed child expression with nullptr. Now we remove those // nullptrs from the vector. @@ -144,12 +124,13 @@ MatchExpression::ExpressionOptimizerFunc ListOfMatchExpression::getOptimizer() c if ((matchType == AND || matchType == OR || matchType == INTERNAL_SCHEMA_XOR)) { // Simplify AND/OR/XOR with exactly one operand to an expression consisting of just // that operand. - MatchExpression* simplifiedExpression = children.front(); + auto simplifiedExpression = std::move(children.front()); children.clear(); - return std::unique_ptr<MatchExpression>(simplifiedExpression); + return simplifiedExpression; } else if (matchType == NOR) { // Simplify NOR of exactly one operand to NOT of that operand. - auto simplifiedExpression = std::make_unique<NotMatchExpression>(children.front()); + auto simplifiedExpression = + std::make_unique<NotMatchExpression>(std::move(children.front())); children.clear(); return simplifiedExpression; } @@ -186,7 +167,7 @@ bool ListOfMatchExpression::equivalent(const MatchExpression* other) const { // TOOD: order doesn't matter for (unsigned i = 0; i < _expressions.size(); i++) - if (!_expressions[i]->equivalent(realOther->_expressions[i])) + if (!_expressions[i]->equivalent(realOther->_expressions[i].get())) return false; return true; diff --git a/src/mongo/db/matcher/expression_tree.h b/src/mongo/db/matcher/expression_tree.h index 25d71dde78f..5100131dcaa 100644 --- a/src/mongo/db/matcher/expression_tree.h +++ b/src/mongo/db/matcher/expression_tree.h @@ -30,8 +30,10 @@ #pragma once #include <boost/optional.hpp> +#include <memory> #include "mongo/db/matcher/expression.h" +#include "mongo/db/query/util/make_data_structure.h" /** * this contains all Expessions that define the structure of the tree @@ -41,25 +43,16 @@ namespace mongo { class ListOfMatchExpression : public MatchExpression { public: - explicit ListOfMatchExpression(MatchType type, - clonable_ptr<ErrorAnnotation> annotation = nullptr) - : MatchExpression(type, std::move(annotation)) {} - virtual ~ListOfMatchExpression(); - - /** - * @param e - I take ownership - */ - void add(MatchExpression* e); + ListOfMatchExpression(MatchType type, + clonable_ptr<ErrorAnnotation> annotation, + std::vector<std::unique_ptr<MatchExpression>> expressions) + : MatchExpression(type, std::move(annotation)), _expressions(std::move(expressions)) {} void add(std::unique_ptr<MatchExpression> e) { - add(e.release()); + _expressions.push_back(std::move(e)); } - /** - * clears all the thingsd we own, and does NOT delete - * someone else has taken ownership - */ - void clearAndRelease() { + void clear() { _expressions.clear(); } @@ -67,33 +60,31 @@ public: return _expressions.size(); } - virtual MatchExpression* getChild(size_t i) const { - return _expressions[i]; + MatchExpression* getChild(size_t i) const final { + return _expressions[i].get(); } /* * Replaces the ith child with nullptr, and releases ownership of the child. */ - virtual std::unique_ptr<MatchExpression> releaseChild(size_t i) { - auto child = std::unique_ptr<MatchExpression>(_expressions[i]); - _expressions[i] = nullptr; - return child; + std::unique_ptr<MatchExpression> releaseChild(size_t i) { + return std::move(_expressions[i]); } /* * Removes the ith child, and releases ownership of the child. */ virtual std::unique_ptr<MatchExpression> removeChild(size_t i) { - auto child = std::unique_ptr<MatchExpression>(_expressions[i]); + auto child = std::move(_expressions[i]); _expressions.erase(_expressions.begin() + i); return child; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return _expressions; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return &_expressions; } - bool equivalent(const MatchExpression* other) const; + bool equivalent(const MatchExpression* other) const final; MatchCategory getCategory() const final { return MatchCategory::kLogical; @@ -107,7 +98,7 @@ protected: private: ExpressionOptimizerFunc getOptimizer() const final; - std::vector<MatchExpression*> _expressions; + std::vector<std::unique_ptr<MatchExpression>> _expressions; }; class AndMatchExpression : public ListOfMatchExpression { @@ -115,10 +106,15 @@ public: static constexpr StringData kName = "$and"_sd; AndMatchExpression(clonable_ptr<ErrorAnnotation> annotation = nullptr) - : ListOfMatchExpression(AND, std::move(annotation)) {} - virtual ~AndMatchExpression() {} + : ListOfMatchExpression(AND, std::move(annotation), {}) {} + AndMatchExpression(std::vector<std::unique_ptr<MatchExpression>> expressions, + clonable_ptr<ErrorAnnotation> annotation = nullptr) + : ListOfMatchExpression(AND, std::move(annotation), std::move(expressions)) {} + AndMatchExpression(std::unique_ptr<MatchExpression> expression, + clonable_ptr<ErrorAnnotation> annotation = nullptr) + : ListOfMatchExpression(AND, std::move(annotation), makeVector(std::move(expression))) {} - virtual bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const; + bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const final; bool matchesSingleElement(const BSONElement&, MatchDetails* details = nullptr) const final; @@ -126,7 +122,7 @@ public: std::unique_ptr<AndMatchExpression> self = std::make_unique<AndMatchExpression>(_errorAnnotation); for (size_t i = 0; i < numChildren(); ++i) { - self->add(getChild(i)->shallowClone().release()); + self->add(getChild(i)->shallowClone()); } if (getTag()) { self->setTag(getTag()->clone()); @@ -154,10 +150,15 @@ public: static constexpr StringData kName = "$or"_sd; OrMatchExpression(clonable_ptr<ErrorAnnotation> annotation = nullptr) - : ListOfMatchExpression(OR, std::move(annotation)) {} - virtual ~OrMatchExpression() {} + : ListOfMatchExpression(OR, std::move(annotation), {}) {} + OrMatchExpression(std::vector<std::unique_ptr<MatchExpression>> expressions, + clonable_ptr<ErrorAnnotation> annotation = nullptr) + : ListOfMatchExpression(OR, std::move(annotation), std::move(expressions)) {} + OrMatchExpression(std::unique_ptr<MatchExpression> expression, + clonable_ptr<ErrorAnnotation> annotation = nullptr) + : ListOfMatchExpression(OR, std::move(annotation), makeVector(std::move(expression))) {} - virtual bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const; + bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const final; bool matchesSingleElement(const BSONElement&, MatchDetails* details = nullptr) const final; @@ -165,7 +166,7 @@ public: std::unique_ptr<OrMatchExpression> self = std::make_unique<OrMatchExpression>(_errorAnnotation); for (size_t i = 0; i < numChildren(); ++i) { - self->add(getChild(i)->shallowClone().release()); + self->add(getChild(i)->shallowClone()); } if (getTag()) { self->setTag(getTag()->clone()); @@ -193,10 +194,15 @@ public: static constexpr StringData kName = "$nor"_sd; NorMatchExpression(clonable_ptr<ErrorAnnotation> annotation = nullptr) - : ListOfMatchExpression(NOR, std::move(annotation)) {} - virtual ~NorMatchExpression() {} + : ListOfMatchExpression(NOR, std::move(annotation), {}) {} + NorMatchExpression(std::vector<std::unique_ptr<MatchExpression>> expressions, + clonable_ptr<ErrorAnnotation> annotation = nullptr) + : ListOfMatchExpression(NOR, std::move(annotation), std::move(expressions)) {} + NorMatchExpression(std::unique_ptr<MatchExpression> expression, + clonable_ptr<ErrorAnnotation> annotation = nullptr) + : ListOfMatchExpression(NOR, std::move(annotation), makeVector(std::move(expression))) {} - virtual bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const; + bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const final; bool matchesSingleElement(const BSONElement&, MatchDetails* details = nullptr) const final; @@ -204,7 +210,7 @@ public: std::unique_ptr<NorMatchExpression> self = std::make_unique<NorMatchExpression>(_errorAnnotation); for (size_t i = 0; i < numChildren(); ++i) { - self->add(getChild(i)->shallowClone().release()); + self->add(getChild(i)->shallowClone()); } if (getTag()) { self->setTag(getTag()->clone()); @@ -237,14 +243,14 @@ public: virtual std::unique_ptr<MatchExpression> shallowClone() const { std::unique_ptr<NotMatchExpression> self = - std::make_unique<NotMatchExpression>(_exp->shallowClone().release(), _errorAnnotation); + std::make_unique<NotMatchExpression>(_exp->shallowClone(), _errorAnnotation); if (getTag()) { self->setTag(getTag()->clone()); } return self; } - virtual bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const { + bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const final { return !_exp->matches(doc, nullptr); } @@ -256,7 +262,7 @@ public: virtual void serialize(BSONObjBuilder* out, bool includePath) const; - bool equivalent(const MatchExpression* other) const; + bool equivalent(const MatchExpression* other) const final; size_t numChildren() const final { return 1; @@ -266,11 +272,11 @@ public: return _exp.get(); } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } - MatchExpression* releaseChild(void) { + MatchExpression* releaseChild() { return _exp.release(); } diff --git a/src/mongo/db/matcher/expression_tree_test.cpp b/src/mongo/db/matcher/expression_tree_test.cpp index 74e5e94c0f5..16c03b31336 100644 --- a/src/mongo/db/matcher/expression_tree_test.cpp +++ b/src/mongo/db/matcher/expression_tree_test.cpp @@ -40,20 +40,18 @@ namespace mongo { -using std::unique_ptr; - TEST(NotMatchExpression, MatchesScalar) { - BSONObj baseOperand = BSON("$lt" << 5); - unique_ptr<ComparisonMatchExpression> lt(new LTMatchExpression("a", baseOperand["$lt"])); - NotMatchExpression notOp(lt.release()); + auto baseOperand = BSON("$lt" << 5); + auto lt = std::make_unique<LTMatchExpression>("a", baseOperand["$lt"]); + auto notOp = NotMatchExpression{lt.release()}; ASSERT(notOp.matchesBSON(BSON("a" << 6), nullptr)); ASSERT(!notOp.matchesBSON(BSON("a" << 4), nullptr)); } TEST(NotMatchExpression, MatchesArray) { - BSONObj baseOperand = BSON("$lt" << 5); - unique_ptr<ComparisonMatchExpression> lt(new LTMatchExpression("a", baseOperand["$lt"])); - NotMatchExpression notOp(lt.release()); + auto baseOperand = BSON("$lt" << 5); + auto lt = std::make_unique<LTMatchExpression>("a", baseOperand["$lt"]); + auto notOp = NotMatchExpression{lt.release()}; ASSERT(notOp.matchesBSON(BSON("a" << BSON_ARRAY(6)), nullptr)); ASSERT(!notOp.matchesBSON(BSON("a" << BSON_ARRAY(4)), nullptr)); // All array elements must match. @@ -61,10 +59,10 @@ TEST(NotMatchExpression, MatchesArray) { } TEST(NotMatchExpression, ElemMatchKey) { - BSONObj baseOperand = BSON("$lt" << 5); - unique_ptr<ComparisonMatchExpression> lt(new LTMatchExpression("a", baseOperand["$lt"])); - NotMatchExpression notOp(lt.release()); - MatchDetails details; + auto baseOperand = BSON("$lt" << 5); + auto lt = std::make_unique<LTMatchExpression>("a", baseOperand["$lt"]); + auto notOp = NotMatchExpression{lt.release()}; + auto details = MatchDetails{}; details.requestElemMatchKey(); ASSERT(!notOp.matchesBSON(BSON("a" << BSON_ARRAY(1)), &details)); ASSERT(!details.hasElemMatchKey()); @@ -76,11 +74,11 @@ TEST(NotMatchExpression, ElemMatchKey) { } TEST(NotMatchExpression, SetCollatorPropagatesToChild) { - BSONObj baseOperand = BSON("a" - << "string"); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("a", baseOperand["a"])); - NotMatchExpression notOp(eq.release()); - CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual); + auto baseOperand = BSON("a" + << "string"); + auto eq = std::make_unique<EqualityMatchExpression>("a", baseOperand["a"]); + auto notOp = NotMatchExpression{eq.release()}; + auto collator = CollatorInterfaceMock{CollatorInterfaceMock::MockType::kAlwaysEqual}; notOp.setCollator(&collator); ASSERT(!notOp.matchesBSON(BSON("a" << "string2"), @@ -88,32 +86,32 @@ TEST(NotMatchExpression, SetCollatorPropagatesToChild) { } TEST(AndOp, NoClauses) { - AndMatchExpression andMatchExpression; - ASSERT(andMatchExpression.matchesBSON(BSONObj(), nullptr)); + auto andMatchExpression = AndMatchExpression{}; + ASSERT(andMatchExpression.matchesBSON(BSONObj{}, nullptr)); } TEST(AndOp, MatchesElementThreeClauses) { - BSONObj baseOperand1 = BSON("$lt" - << "z1"); - BSONObj baseOperand2 = BSON("$gt" - << "a1"); - BSONObj match = BSON("a" - << "r1"); - BSONObj notMatch1 = BSON("a" + auto baseOperand1 = BSON("$lt" << "z1"); - BSONObj notMatch2 = BSON("a" + auto baseOperand2 = BSON("$gt" << "a1"); - BSONObj notMatch3 = BSON("a" - << "r"); - - unique_ptr<ComparisonMatchExpression> sub1(new LTMatchExpression("a", baseOperand1["$lt"])); - unique_ptr<ComparisonMatchExpression> sub2(new GTMatchExpression("a", baseOperand2["$gt"])); - unique_ptr<RegexMatchExpression> sub3(new RegexMatchExpression("a", "1", "")); - - AndMatchExpression andOp; - andOp.add(sub1.release()); - andOp.add(sub2.release()); - andOp.add(sub3.release()); + auto match = BSON("a" + << "r1"); + auto notMatch1 = BSON("a" + << "z1"); + auto notMatch2 = BSON("a" + << "a1"); + auto notMatch3 = BSON("a" + << "r"); + + auto sub1 = std::make_unique<LTMatchExpression>("a", baseOperand1["$lt"]); + auto sub2 = std::make_unique<GTMatchExpression>("a", baseOperand2["$gt"]); + auto sub3 = std::make_unique<RegexMatchExpression>("a", "1", ""); + + auto andOp = AndMatchExpression{}; + andOp.add(std::move(sub1)); + andOp.add(std::move(sub2)); + andOp.add(std::move(sub3)); ASSERT(andOp.matchesBSON(match)); ASSERT(!andOp.matchesBSON(notMatch1)); @@ -122,12 +120,12 @@ TEST(AndOp, MatchesElementThreeClauses) { } TEST(AndOp, MatchesSingleClause) { - BSONObj baseOperand = BSON("$ne" << 5); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("a", baseOperand["$ne"])); - unique_ptr<NotMatchExpression> ne(new NotMatchExpression(eq.release())); + auto baseOperand = BSON("$ne" << 5); + auto eq = std::make_unique<EqualityMatchExpression>("a", baseOperand["$ne"]); + auto ne = std::make_unique<NotMatchExpression>(eq.release()); - AndMatchExpression andOp; - andOp.add(ne.release()); + auto andOp = AndMatchExpression{}; + andOp.add(std::move(ne)); ASSERT(andOp.matchesBSON(BSON("a" << 4), nullptr)); ASSERT(andOp.matchesBSON(BSON("a" << BSON_ARRAY(4 << 6)), nullptr)); @@ -136,18 +134,18 @@ TEST(AndOp, MatchesSingleClause) { } TEST(AndOp, MatchesThreeClauses) { - BSONObj baseOperand1 = BSON("$gt" << 1); - BSONObj baseOperand2 = BSON("$lt" << 10); - BSONObj baseOperand3 = BSON("$lt" << 100); + auto baseOperand1 = BSON("$gt" << 1); + auto baseOperand2 = BSON("$lt" << 10); + auto baseOperand3 = BSON("$lt" << 100); - unique_ptr<ComparisonMatchExpression> sub1(new GTMatchExpression("a", baseOperand1["$gt"])); - unique_ptr<ComparisonMatchExpression> sub2(new LTMatchExpression("a", baseOperand2["$lt"])); - unique_ptr<ComparisonMatchExpression> sub3(new LTMatchExpression("b", baseOperand3["$lt"])); + auto sub1 = std::make_unique<GTMatchExpression>("a", baseOperand1["$gt"]); + auto sub2 = std::make_unique<LTMatchExpression>("a", baseOperand2["$lt"]); + auto sub3 = std::make_unique<LTMatchExpression>("b", baseOperand3["$lt"]); - AndMatchExpression andOp; - andOp.add(sub1.release()); - andOp.add(sub2.release()); - andOp.add(sub3.release()); + auto andOp = AndMatchExpression{}; + andOp.add(std::move(sub1)); + andOp.add(std::move(sub2)); + andOp.add(std::move(sub3)); ASSERT(andOp.matchesBSON(BSON("a" << 5 << "b" << 6), nullptr)); ASSERT(!andOp.matchesBSON(BSON("a" << 5), nullptr)); @@ -157,17 +155,17 @@ TEST(AndOp, MatchesThreeClauses) { } TEST(AndOp, ElemMatchKey) { - BSONObj baseOperand1 = BSON("a" << 1); - BSONObj baseOperand2 = BSON("b" << 2); + auto baseOperand1 = BSON("a" << 1); + auto baseOperand2 = BSON("b" << 2); - unique_ptr<ComparisonMatchExpression> sub1(new EqualityMatchExpression("a", baseOperand1["a"])); - unique_ptr<ComparisonMatchExpression> sub2(new EqualityMatchExpression("b", baseOperand2["b"])); + auto sub1 = std::make_unique<EqualityMatchExpression>("a", baseOperand1["a"]); + auto sub2 = std::make_unique<EqualityMatchExpression>("b", baseOperand2["b"]); - AndMatchExpression andOp; - andOp.add(sub1.release()); - andOp.add(sub2.release()); + auto andOp = AndMatchExpression{}; + andOp.add(std::move(sub1)); + andOp.add(std::move(sub2)); - MatchDetails details; + auto details = MatchDetails{}; details.requestElemMatchKey(); ASSERT(!andOp.matchesBSON(BSON("a" << BSON_ARRAY(1)), &details)); ASSERT(!details.hasElemMatchKey()); @@ -180,17 +178,17 @@ TEST(AndOp, ElemMatchKey) { } TEST(OrOp, NoClauses) { - OrMatchExpression orOp; - ASSERT(!orOp.matchesBSON(BSONObj(), nullptr)); + auto orOp = OrMatchExpression{}; + ASSERT(!orOp.matchesBSON(BSONObj{}, nullptr)); } TEST(OrOp, MatchesSingleClause) { - BSONObj baseOperand = BSON("$ne" << 5); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("a", baseOperand["$ne"])); - unique_ptr<NotMatchExpression> ne(new NotMatchExpression(eq.release())); + auto baseOperand = BSON("$ne" << 5); + auto eq = std::make_unique<EqualityMatchExpression>("a", baseOperand["$ne"]); + auto ne = std::make_unique<NotMatchExpression>(eq.release()); - OrMatchExpression orOp; - orOp.add(ne.release()); + auto orOp = OrMatchExpression{}; + orOp.add(std::move(ne)); ASSERT(orOp.matchesBSON(BSON("a" << 4), nullptr)); ASSERT(orOp.matchesBSON(BSON("a" << BSON_ARRAY(4 << 6)), nullptr)); @@ -201,14 +199,12 @@ TEST(OrOp, MatchesSingleClause) { TEST(OrOp, MatchesTwoClauses) { auto clauseObj1 = fromjson("{i: 5}"); auto clauseObj2 = fromjson("{'i.a': 6}"); - std::unique_ptr<ComparisonMatchExpression> clause1( - new EqualityMatchExpression("i", clauseObj1["i"])); - std::unique_ptr<ComparisonMatchExpression> clause2( - new EqualityMatchExpression("i.a", clauseObj2["i.a"])); + auto clause1 = std::make_unique<EqualityMatchExpression>("i", clauseObj1["i"]); + auto clause2 = std::make_unique<EqualityMatchExpression>("i.a", clauseObj2["i.a"]); - OrMatchExpression filter; - filter.add(clause1.release()); - filter.add(clause2.release()); + auto filter = OrMatchExpression{}; + filter.add(std::move(clause1)); + filter.add(std::move(clause2)); auto aClause1 = fromjson("{a: 5}"); auto iClause1 = fromjson("{i: 5}"); @@ -232,17 +228,17 @@ TEST(OrOp, MatchesTwoClauses) { } TEST(OrOp, MatchesThreeClauses) { - BSONObj baseOperand1 = BSON("$gt" << 10); - BSONObj baseOperand2 = BSON("$lt" << 0); - BSONObj baseOperand3 = BSON("b" << 100); - unique_ptr<ComparisonMatchExpression> sub1(new GTMatchExpression("a", baseOperand1["$gt"])); - unique_ptr<ComparisonMatchExpression> sub2(new LTMatchExpression("a", baseOperand2["$lt"])); - unique_ptr<ComparisonMatchExpression> sub3(new EqualityMatchExpression("b", baseOperand3["b"])); - - OrMatchExpression orOp; - orOp.add(sub1.release()); - orOp.add(sub2.release()); - orOp.add(sub3.release()); + auto baseOperand1 = BSON("$gt" << 10); + auto baseOperand2 = BSON("$lt" << 0); + auto baseOperand3 = BSON("b" << 100); + auto sub1 = std::make_unique<GTMatchExpression>("a", baseOperand1["$gt"]); + auto sub2 = std::make_unique<LTMatchExpression>("a", baseOperand2["$lt"]); + auto sub3 = std::make_unique<EqualityMatchExpression>("b", baseOperand3["b"]); + + auto orOp = OrMatchExpression{}; + orOp.add(std::move(sub1)); + orOp.add(std::move(sub2)); + orOp.add(std::move(sub3)); ASSERT(orOp.matchesBSON(BSON("a" << -1), nullptr)); ASSERT(orOp.matchesBSON(BSON("a" << 11), nullptr)); @@ -254,18 +250,18 @@ TEST(OrOp, MatchesThreeClauses) { } TEST(OrOp, ElemMatchKey) { - BSONObj baseOperand1 = BSON("a" << 1); - BSONObj baseOperand2 = BSON("b" << 2); - unique_ptr<ComparisonMatchExpression> sub1(new EqualityMatchExpression("a", baseOperand1["a"])); - unique_ptr<ComparisonMatchExpression> sub2(new EqualityMatchExpression("b", baseOperand2["b"])); + auto baseOperand1 = BSON("a" << 1); + auto baseOperand2 = BSON("b" << 2); + auto sub1 = std::make_unique<EqualityMatchExpression>("a", baseOperand1["a"]); + auto sub2 = std::make_unique<EqualityMatchExpression>("b", baseOperand2["b"]); - OrMatchExpression orOp; - orOp.add(sub1.release()); - orOp.add(sub2.release()); + auto orOp = OrMatchExpression{}; + orOp.add(std::move(sub1)); + orOp.add(std::move(sub2)); - MatchDetails details; + auto details = MatchDetails{}; details.requestElemMatchKey(); - ASSERT(!orOp.matchesBSON(BSONObj(), &details)); + ASSERT(!orOp.matchesBSON(BSONObj{}, &details)); ASSERT(!details.hasElemMatchKey()); ASSERT(!orOp.matchesBSON(BSON("a" << BSON_ARRAY(10) << "b" << BSON_ARRAY(10)), &details)); ASSERT(!details.hasElemMatchKey()); @@ -275,17 +271,17 @@ TEST(OrOp, ElemMatchKey) { } TEST(NorOp, NoClauses) { - NorMatchExpression norOp; - ASSERT(norOp.matchesBSON(BSONObj(), nullptr)); + auto norOp = NorMatchExpression{}; + ASSERT(norOp.matchesBSON(BSONObj{}, nullptr)); } TEST(NorOp, MatchesSingleClause) { - BSONObj baseOperand = BSON("$ne" << 5); - unique_ptr<ComparisonMatchExpression> eq(new EqualityMatchExpression("a", baseOperand["$ne"])); - unique_ptr<NotMatchExpression> ne(new NotMatchExpression(eq.release())); + auto baseOperand = BSON("$ne" << 5); + auto eq = std::make_unique<EqualityMatchExpression>("a", baseOperand["$ne"]); + auto ne = std::make_unique<NotMatchExpression>(eq.release()); - NorMatchExpression norOp; - norOp.add(ne.release()); + auto norOp = NorMatchExpression{}; + norOp.add(std::move(ne)); ASSERT(!norOp.matchesBSON(BSON("a" << 4), nullptr)); ASSERT(!norOp.matchesBSON(BSON("a" << BSON_ARRAY(4 << 6)), nullptr)); @@ -294,37 +290,37 @@ TEST(NorOp, MatchesSingleClause) { } TEST(NorOp, MatchesThreeClauses) { - BSONObj baseOperand1 = BSON("$gt" << 10); - BSONObj baseOperand2 = BSON("$lt" << 0); - BSONObj baseOperand3 = BSON("b" << 100); + auto baseOperand1 = BSON("$gt" << 10); + auto baseOperand2 = BSON("$lt" << 0); + auto baseOperand3 = BSON("b" << 100); - unique_ptr<ComparisonMatchExpression> sub1(new GTMatchExpression("a", baseOperand1["$gt"])); - unique_ptr<ComparisonMatchExpression> sub2(new LTMatchExpression("a", baseOperand2["$lt"])); - unique_ptr<ComparisonMatchExpression> sub3(new EqualityMatchExpression("b", baseOperand3["b"])); + auto sub1 = std::make_unique<GTMatchExpression>("a", baseOperand1["$gt"]); + auto sub2 = std::make_unique<LTMatchExpression>("a", baseOperand2["$lt"]); + auto sub3 = std::make_unique<EqualityMatchExpression>("b", baseOperand3["b"]); - NorMatchExpression norOp; - norOp.add(sub1.release()); - norOp.add(sub2.release()); - norOp.add(sub3.release()); + auto norOp = NorMatchExpression{}; + norOp.add(std::move(sub1)); + norOp.add(std::move(sub2)); + norOp.add(std::move(sub3)); ASSERT(!norOp.matchesBSON(BSON("a" << -1), nullptr)); ASSERT(!norOp.matchesBSON(BSON("a" << 11), nullptr)); ASSERT(norOp.matchesBSON(BSON("a" << 5), nullptr)); ASSERT(!norOp.matchesBSON(BSON("b" << 100), nullptr)); ASSERT(norOp.matchesBSON(BSON("b" << 101), nullptr)); - ASSERT(norOp.matchesBSON(BSONObj(), nullptr)); + ASSERT(norOp.matchesBSON(BSONObj{}, nullptr)); ASSERT(!norOp.matchesBSON(BSON("a" << 11 << "b" << 100), nullptr)); } TEST(NorOp, ElemMatchKey) { - BSONObj baseOperand1 = BSON("a" << 1); - BSONObj baseOperand2 = BSON("b" << 2); - unique_ptr<ComparisonMatchExpression> sub1(new EqualityMatchExpression("a", baseOperand1["a"])); - unique_ptr<ComparisonMatchExpression> sub2(new EqualityMatchExpression("b", baseOperand2["b"])); + auto baseOperand1 = BSON("a" << 1); + auto baseOperand2 = BSON("b" << 2); + auto sub1 = std::make_unique<EqualityMatchExpression>("a", baseOperand1["a"]); + auto sub2 = std::make_unique<EqualityMatchExpression>("b", baseOperand2["b"]); - NorMatchExpression norOp; - norOp.add(sub1.release()); - norOp.add(sub2.release()); + auto norOp = NorMatchExpression{}; + norOp.add(std::move(sub1)); + norOp.add(std::move(sub2)); MatchDetails details; details.requestElemMatchKey(); @@ -339,17 +335,17 @@ TEST(NorOp, ElemMatchKey) { TEST(NorOp, Equivalent) { - BSONObj baseOperand1 = BSON("a" << 1); - BSONObj baseOperand2 = BSON("b" << 2); - EqualityMatchExpression sub1("a", baseOperand1["a"]); - EqualityMatchExpression sub2("b", baseOperand2["b"]); + auto baseOperand1 = BSON("a" << 1); + auto baseOperand2 = BSON("b" << 2); + auto sub1 = EqualityMatchExpression{"a", baseOperand1["a"]}; + auto sub2 = EqualityMatchExpression{"b", baseOperand2["b"]}; - NorMatchExpression e1; - e1.add(sub1.shallowClone().release()); - e1.add(sub2.shallowClone().release()); + auto e1 = NorMatchExpression{}; + e1.add(sub1.shallowClone()); + e1.add(sub2.shallowClone()); - NorMatchExpression e2; - e2.add(sub1.shallowClone().release()); + auto e2 = NorMatchExpression{}; + e2.add(sub1.shallowClone()); ASSERT(e1.equivalent(&e1)); ASSERT(!e1.equivalent(&e2)); diff --git a/src/mongo/db/matcher/expression_where_base.h b/src/mongo/db/matcher/expression_where_base.h index f68ccb5efc6..8a548538943 100644 --- a/src/mongo/db/matcher/expression_where_base.h +++ b/src/mongo/db/matcher/expression_where_base.h @@ -54,8 +54,8 @@ public: MONGO_UNREACHABLE; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } bool matchesSingleElement(const BSONElement& e, MatchDetails* details = nullptr) const final { diff --git a/src/mongo/db/matcher/rewrite_expr.cpp b/src/mongo/db/matcher/rewrite_expr.cpp index 27e19d4c5fe..660073d5a8a 100644 --- a/src/mongo/db/matcher/rewrite_expr.cpp +++ b/src/mongo/db/matcher/rewrite_expr.cpp @@ -85,15 +85,12 @@ std::unique_ptr<MatchExpression> RewriteExpr::_rewriteAndExpression( auto andMatch = std::make_unique<AndMatchExpression>(); - for (auto&& child : currExprNode->getOperandList()) { - if (auto childMatch = _rewriteExpression(child)) { - andMatch->add(childMatch.release()); - } - } + for (auto&& child : currExprNode->getOperandList()) + if (auto childMatch = _rewriteExpression(child)) + andMatch->add(std::move(childMatch)); - if (andMatch->numChildren() > 0) { + if (andMatch->numChildren() > 0) return andMatch; - } return nullptr; } @@ -102,19 +99,16 @@ std::unique_ptr<MatchExpression> RewriteExpr::_rewriteOrExpression( const boost::intrusive_ptr<ExpressionOr>& currExprNode) { auto orMatch = std::make_unique<OrMatchExpression>(); - for (auto&& child : currExprNode->getOperandList()) { - if (auto childExpr = _rewriteExpression(child)) { - orMatch->add(childExpr.release()); - } else { + for (auto&& child : currExprNode->getOperandList()) + if (auto childExpr = _rewriteExpression(child)) + orMatch->add(std::move(childExpr)); + else // If any child cannot be rewritten to a MatchExpression then we must abandon adding // this $or clause. return nullptr; - } - } - if (orMatch->numChildren() > 0) { + if (orMatch->numChildren() > 0) return orMatch; - } return nullptr; } diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_all_elem_match_from_index.h b/src/mongo/db/matcher/schema/expression_internal_schema_all_elem_match_from_index.h index 58994d4f612..0f2fe41a35c 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_all_elem_match_from_index.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_all_elem_match_from_index.h @@ -88,8 +88,8 @@ public: return _index; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } size_t numChildren() const final { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.h b/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.h index 1e8fdbaf5d2..a286212ee61 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_allowed_properties.h @@ -140,8 +140,8 @@ public: std::unique_ptr<MatchExpression> shallowClone() const final; - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } size_t numChildren() const final { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_match_array_index.h b/src/mongo/db/matcher/schema/expression_internal_schema_match_array_index.h index 05519e17ff3..11126ae81d0 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_match_array_index.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_match_array_index.h @@ -77,8 +77,8 @@ public: std::unique_ptr<MatchExpression> shallowClone() const final; - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } size_t numChildren() const final { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_num_array_items.h b/src/mongo/db/matcher/schema/expression_internal_schema_num_array_items.h index ca6391bc821..8c6a4062d21 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_num_array_items.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_num_array_items.h @@ -63,8 +63,8 @@ public: MONGO_UNREACHABLE; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } protected: diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.h b/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.h index 758b374ee2f..563cfcda5a7 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_num_properties.h @@ -60,8 +60,8 @@ public: MONGO_UNREACHABLE; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } void debugString(StringBuilder& debug, int indentationLevel) const final; diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_object_match.h b/src/mongo/db/matcher/schema/expression_internal_schema_object_match.h index dee55fbb5c5..091d3af777f 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_object_match.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_object_match.h @@ -53,8 +53,8 @@ public: bool equivalent(const MatchExpression* other) const final; - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } size_t numChildren() const final { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_root_doc_eq.h b/src/mongo/db/matcher/schema/expression_internal_schema_root_doc_eq.h index 9e374b655c9..47003cf630c 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_root_doc_eq.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_root_doc_eq.h @@ -83,8 +83,8 @@ public: MONGO_UNREACHABLE; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } MatchCategory getCategory() const final { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_unique_items.h b/src/mongo/db/matcher/schema/expression_internal_schema_unique_items.h index 2bc5c2f1671..ecd70d9ff8c 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_unique_items.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_unique_items.h @@ -60,8 +60,8 @@ public: MONGO_UNREACHABLE; } - boost::optional<std::vector<MatchExpression*>&> getChildVector() final { - return boost::none; + std::vector<std::unique_ptr<MatchExpression>>* getChildVector() final { + return nullptr; } bool matchesArray(const BSONObj& array, MatchDetails*) const final { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_xor.h b/src/mongo/db/matcher/schema/expression_internal_schema_xor.h index 88a8d355c25..928a13413f1 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_xor.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_xor.h @@ -42,7 +42,15 @@ public: static constexpr StringData kName = "$_internalSchemaXor"_sd; InternalSchemaXorMatchExpression(clonable_ptr<ErrorAnnotation> annotation = nullptr) - : ListOfMatchExpression(INTERNAL_SCHEMA_XOR, std::move(annotation)) {} + : ListOfMatchExpression(INTERNAL_SCHEMA_XOR, std::move(annotation), {}) {} + InternalSchemaXorMatchExpression(std::vector<std::unique_ptr<MatchExpression>> expressions, + clonable_ptr<ErrorAnnotation> annotation = nullptr) + : ListOfMatchExpression( + INTERNAL_SCHEMA_XOR, std::move(annotation), std::move(expressions)) {} + InternalSchemaXorMatchExpression(std::unique_ptr<MatchExpression> expression, + clonable_ptr<ErrorAnnotation> annotation = nullptr) + : ListOfMatchExpression( + INTERNAL_SCHEMA_XOR, std::move(annotation), makeVector(std::move(expression))) {} bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const final; @@ -51,7 +59,7 @@ public: virtual std::unique_ptr<MatchExpression> shallowClone() const { auto xorCopy = std::make_unique<InternalSchemaXorMatchExpression>(_errorAnnotation); for (size_t i = 0; i < numChildren(); ++i) { - xorCopy->add(getChild(i)->shallowClone().release()); + xorCopy->add(getChild(i)->shallowClone()); } if (getTag()) { xorCopy->setTag(getTag()->clone()); diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_xor_test.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_xor_test.cpp index ebe6c3681b9..7626d83ca1b 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_xor_test.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_xor_test.cpp @@ -104,11 +104,11 @@ TEST(InternalSchemaXorOp, Equivalent) { EqualityMatchExpression sub2("b", baseOperand2["b"]); InternalSchemaXorMatchExpression e1; - e1.add(sub1.shallowClone().release()); - e1.add(sub2.shallowClone().release()); + e1.add(sub1.shallowClone()); + e1.add(sub2.shallowClone()); InternalSchemaXorMatchExpression e2; - e2.add(sub1.shallowClone().release()); + e2.add(sub1.shallowClone()); ASSERT(e1.equivalent(&e1)); ASSERT_FALSE(e1.equivalent(&e2)); diff --git a/src/mongo/db/matcher/schema/json_schema_parser.cpp b/src/mongo/db/matcher/schema/json_schema_parser.cpp index df73cced7a7..4aa7e67f343 100644 --- a/src/mongo/db/matcher/schema/json_schema_parser.cpp +++ b/src/mongo/db/matcher/schema/json_schema_parser.cpp @@ -155,13 +155,13 @@ std::unique_ptr<MatchExpression> makeRestriction( doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnore)); auto notExpr = std::make_unique<NotMatchExpression>( - typeExpr.release(), + std::move(typeExpr), doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnore)); auto orExpr = std::make_unique<OrMatchExpression>( doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnoreButDescend)); - orExpr->add(notExpr.release()); - orExpr->add(restrictionExpr.release()); + orExpr->add(std::move(notExpr)); + orExpr->add(std::move(restrictionExpr)); return orExpr; } @@ -388,7 +388,7 @@ StatusWithMatchExpression parseLogicalKeyword(const boost::intrusive_ptr<Express return nestedSchemaMatch.getStatus(); } - listOfExpr->add(nestedSchemaMatch.getValue().release()); + listOfExpr->add(std::move(nestedSchemaMatch.getValue())); } return {std::move(listOfExpr)}; @@ -431,14 +431,14 @@ StatusWithMatchExpression parseEnum(const boost::intrusive_ptr<ExpressionContext auto rootDocEq = std::make_unique<InternalSchemaRootDocEqMatchExpression>( arrayElem.embeddedObject(), doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnore)); - orExpr->add(rootDocEq.release()); + orExpr->add(std::move(rootDocEq)); } } else { auto eqExpr = std::make_unique<InternalSchemaEqMatchExpression>( path, arrayElem, doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnore)); - orExpr->add(eqExpr.release()); + orExpr->add(std::move(eqExpr)); } } @@ -509,7 +509,7 @@ StatusWithMatchExpression translateRequired(const boost::intrusive_ptr<Expressio for (auto&& propertyName : sortedProperties) { // This node is tagged as '_propertyExists' to indicate that it will produce a path instead // of a detailed BSONObj error during error generation. - andExpr->add(new ExistsMatchExpression( + andExpr->add(std::make_unique<ExistsMatchExpression>( propertyName, doc_validation_error::createAnnotation(expCtx, "_propertyExists", BSONObj()))); } @@ -568,7 +568,7 @@ StatusWithMatchExpression parseProperties(const boost::intrusive_ptr<ExpressionC if (requiredProperties.find(property.fieldNameStringData()) != requiredProperties.end()) { // The field name for which we created the nested schema is a required property. This // property must exist and therefore must match 'nestedSchemaMatch'. - andExpr->add(nestedSchemaMatch.getValue().release()); + andExpr->add(std::move(nestedSchemaMatch.getValue())); } else { // This property either must not exist or must match the nested schema. Therefore, we // generate the match expression (OR (NOT (EXISTS)) <nestedSchemaMatch>). @@ -577,15 +577,15 @@ StatusWithMatchExpression parseProperties(const boost::intrusive_ptr<ExpressionC doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnore)); auto notExpr = std::make_unique<NotMatchExpression>( - existsExpr.release(), + std::move(existsExpr), doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnore)); auto orExpr = std::make_unique<OrMatchExpression>( doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnoreButDescend)); - orExpr->add(notExpr.release()); - orExpr->add(nestedSchemaMatch.getValue().release()); + orExpr->add(std::move(notExpr)); + orExpr->add(std::move(nestedSchemaMatch.getValue())); - andExpr->add(orExpr.release()); + andExpr->add(std::move(orExpr)); } } @@ -892,7 +892,7 @@ StatusWithMatchExpression translatePropertyDependency( return propertyExistsExpr.getStatus(); } - propertyDependencyExpr->add(propertyExistsExpr.getValue().release()); + propertyDependencyExpr->add(std::move(propertyExistsExpr.getValue())); } auto ifClause = makeDependencyExistsClause(expCtx, path, dependency.fieldNameStringData()); @@ -945,7 +945,7 @@ StatusWithMatchExpression parseDependencies(const boost::intrusive_ptr<Expressio return dependencyExpr.getStatus(); } - andExpr->add(dependencyExpr.getValue().release()); + andExpr->add(std::move(dependencyExpr.getValue())); } return {std::move(andExpr)}; @@ -1025,22 +1025,18 @@ StatusWith<boost::optional<long long>> parseItems( expCtx, "" /* 'andExprForSubschemas' carries the operator name, not this expression */, BSONObj())); - andExprForSubschemas->add(matchArrayIndex.release()); + andExprForSubschemas->add(std::move(matchArrayIndex)); ++index; } startIndexForAdditionalItems = index; if (path.empty()) { andExpr->add( - std::make_unique<AlwaysTrueMatchExpression>( - doc_validation_error::createAnnotation( - expCtx, itemsElt.fieldNameStringData().toString(), itemsElt.wrap())) - .release()); + std::make_unique<AlwaysTrueMatchExpression>(doc_validation_error::createAnnotation( + expCtx, itemsElt.fieldNameStringData().toString(), itemsElt.wrap()))); } else { - andExpr->add( - makeRestriction( - expCtx, BSONType::Array, path, std::move(andExprForSubschemas), typeExpr) - .release()); + andExpr->add(makeRestriction( + expCtx, BSONType::Array, path, std::move(andExprForSubschemas), typeExpr)); } } else if (itemsElt.type() == BSONType::Object) { // When "items" is an object, generate a single AllElemMatchFromIndex that applies to every @@ -1060,8 +1056,7 @@ StatusWith<boost::optional<long long>> parseItems( auto errorAnnotation = doc_validation_error::createAnnotation( expCtx, itemsElt.fieldNameStringData().toString(), itemsElt.wrap()); if (path.empty()) { - andExpr->add( - std::make_unique<AlwaysTrueMatchExpression>(std::move(errorAnnotation)).release()); + andExpr->add(std::make_unique<AlwaysTrueMatchExpression>(std::move(errorAnnotation))); } else { constexpr auto startIndexForItems = 0LL; auto allElemMatch = @@ -1071,8 +1066,7 @@ StatusWith<boost::optional<long long>> parseItems( std::move(exprWithPlaceholder), std::move(errorAnnotation)); andExpr->add( - makeRestriction(expCtx, BSONType::Array, path, std::move(allElemMatch), typeExpr) - .release()); + makeRestriction(expCtx, BSONType::Array, path, std::move(allElemMatch), typeExpr)); } } else { return {ErrorCodes::TypeMismatch, @@ -1131,8 +1125,7 @@ Status parseAdditionalItems(const boost::intrusive_ptr<ExpressionContext>& expCt auto errorAnnotation = doc_validation_error::createAnnotation( expCtx, additionalItemsElt.fieldNameStringData().toString(), additionalItemsElt.wrap()); if (path.empty()) { - andExpr->add( - std::make_unique<AlwaysTrueMatchExpression>(std::move(errorAnnotation)).release()); + andExpr->add(std::make_unique<AlwaysTrueMatchExpression>(std::move(errorAnnotation))); } else { auto allElemMatch = std::make_unique<InternalSchemaAllElemMatchFromIndexMatchExpression>( @@ -1141,8 +1134,7 @@ Status parseAdditionalItems(const boost::intrusive_ptr<ExpressionContext>& expCt std::move(otherwiseExpr), std::move(errorAnnotation)); andExpr->add( - makeRestriction(expCtx, BSONType::Array, path, std::move(allElemMatch), typeExpr) - .release()); + makeRestriction(expCtx, BSONType::Array, path, std::move(allElemMatch), typeExpr)); } } return Status::OK(); @@ -1201,7 +1193,7 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, if (!allOfExpr.isOK()) { return allOfExpr.getStatus(); } - andExpr->add(allOfExpr.getValue().release()); + andExpr->add(std::move(allOfExpr.getValue())); } if (auto anyOfElt = keywordMap[JSONSchemaParser::kSchemaAnyOfKeyword]) { @@ -1210,7 +1202,7 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, if (!anyOfExpr.isOK()) { return anyOfExpr.getStatus(); } - andExpr->add(anyOfExpr.getValue().release()); + andExpr->add(std::move(anyOfExpr.getValue())); } if (auto oneOfElt = keywordMap[JSONSchemaParser::kSchemaOneOfKeyword]) { @@ -1219,7 +1211,7 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, if (!oneOfExpr.isOK()) { return oneOfExpr.getStatus(); } - andExpr->add(oneOfExpr.getValue().release()); + andExpr->add(std::move(oneOfExpr.getValue())); } if (auto notElt = keywordMap[JSONSchemaParser::kSchemaNotKeyword]) { @@ -1237,9 +1229,9 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, } auto notMatchExpr = std::make_unique<NotMatchExpression>( - parsedExpr.getValue().release(), + std::move(parsedExpr.getValue()), doc_validation_error::createAnnotation(expCtx, "not", BSONObj())); - andExpr->add(notMatchExpr.release()); + andExpr->add(std::move(notMatchExpr)); } if (auto enumElt = keywordMap[JSONSchemaParser::kSchemaEnumKeyword]) { @@ -1247,7 +1239,7 @@ Status translateLogicalKeywords(StringMap<BSONElement>& keywordMap, if (!enumExpr.isOK()) { return enumExpr.getStatus(); } - andExpr->add(enumExpr.getValue().release()); + andExpr->add(std::move(enumExpr.getValue())); } return Status::OK(); @@ -1277,7 +1269,7 @@ Status translateArrayKeywords(StringMap<BSONElement>& keywordMap, if (!minItemsExpr.isOK()) { return minItemsExpr.getStatus(); } - andExpr->add(minItemsExpr.getValue().release()); + andExpr->add(std::move(minItemsExpr.getValue())); } if (auto maxItemsElt = keywordMap[JSONSchemaParser::kSchemaMaxItemsKeyword]) { @@ -1286,7 +1278,7 @@ Status translateArrayKeywords(StringMap<BSONElement>& keywordMap, if (!maxItemsExpr.isOK()) { return maxItemsExpr.getStatus(); } - andExpr->add(maxItemsExpr.getValue().release()); + andExpr->add(std::move(maxItemsExpr.getValue())); } if (auto uniqueItemsElt = keywordMap[JSONSchemaParser::kSchemaUniqueItemsKeyword]) { @@ -1294,7 +1286,7 @@ Status translateArrayKeywords(StringMap<BSONElement>& keywordMap, if (!uniqueItemsExpr.isOK()) { return uniqueItemsExpr.getStatus(); } - andExpr->add(uniqueItemsExpr.getValue().release()); + andExpr->add(std::move(uniqueItemsExpr.getValue())); } return parseItemsAndAdditionalItems( @@ -1341,7 +1333,7 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, if (!propertiesExpr.isOK()) { return propertiesExpr.getStatus(); } - andExpr->add(propertiesExpr.getValue().release()); + andExpr->add(std::move(propertiesExpr.getValue())); } { @@ -1362,7 +1354,7 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, if (!allowedPropertiesExpr.isOK()) { return allowedPropertiesExpr.getStatus(); } - andExpr->add(allowedPropertiesExpr.getValue().release()); + andExpr->add(std::move(allowedPropertiesExpr.getValue())); } } @@ -1375,7 +1367,7 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, if (!requiredExpr.isOK()) { return requiredExpr.getStatus(); } - andExpr->add(requiredExpr.getValue().release()); + andExpr->add(std::move(requiredExpr.getValue())); } if (auto minPropertiesElt = keywordMap[JSONSchemaParser::kSchemaMinPropertiesKeyword]) { @@ -1384,7 +1376,7 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, if (!minPropExpr.isOK()) { return minPropExpr.getStatus(); } - andExpr->add(minPropExpr.getValue().release()); + andExpr->add(std::move(minPropExpr.getValue())); } if (auto maxPropertiesElt = keywordMap[JSONSchemaParser::kSchemaMaxPropertiesKeyword]) { @@ -1393,7 +1385,7 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, if (!maxPropExpr.isOK()) { return maxPropExpr.getStatus(); } - andExpr->add(maxPropExpr.getValue().release()); + andExpr->add(std::move(maxPropExpr.getValue())); } if (auto dependenciesElt = keywordMap[JSONSchemaParser::kSchemaDependenciesKeyword]) { @@ -1402,7 +1394,7 @@ Status translateObjectKeywords(StringMap<BSONElement>& keywordMap, if (!dependenciesExpr.isOK()) { return dependenciesExpr.getStatus(); } - andExpr->add(dependenciesExpr.getValue().release()); + andExpr->add(std::move(dependenciesExpr.getValue())); } return Status::OK(); @@ -1433,7 +1425,7 @@ Status translateScalarKeywords(const boost::intrusive_ptr<ExpressionContext>& ex if (!patternExpr.isOK()) { return patternExpr.getStatus(); } - andExpr->add(patternExpr.getValue().release()); + andExpr->add(std::move(patternExpr.getValue())); } if (auto maxLengthElt = keywordMap[JSONSchemaParser::kSchemaMaxLengthKeyword]) { @@ -1442,7 +1434,7 @@ Status translateScalarKeywords(const boost::intrusive_ptr<ExpressionContext>& ex if (!maxLengthExpr.isOK()) { return maxLengthExpr.getStatus(); } - andExpr->add(maxLengthExpr.getValue().release()); + andExpr->add(std::move(maxLengthExpr.getValue())); } if (auto minLengthElt = keywordMap[JSONSchemaParser::kSchemaMinLengthKeyword]) { @@ -1451,7 +1443,7 @@ Status translateScalarKeywords(const boost::intrusive_ptr<ExpressionContext>& ex if (!minLengthExpr.isOK()) { return minLengthExpr.getStatus(); } - andExpr->add(minLengthExpr.getValue().release()); + andExpr->add(std::move(minLengthExpr.getValue())); } // Numeric keywords. @@ -1460,7 +1452,7 @@ Status translateScalarKeywords(const boost::intrusive_ptr<ExpressionContext>& ex if (!multipleOfExpr.isOK()) { return multipleOfExpr.getStatus(); } - andExpr->add(multipleOfExpr.getValue().release()); + andExpr->add(std::move(multipleOfExpr.getValue())); } if (auto maximumElt = keywordMap[JSONSchemaParser::kSchemaMaximumKeyword]) { @@ -1480,7 +1472,7 @@ Status translateScalarKeywords(const boost::intrusive_ptr<ExpressionContext>& ex if (!maxExpr.isOK()) { return maxExpr.getStatus(); } - andExpr->add(maxExpr.getValue().release()); + andExpr->add(std::move(maxExpr.getValue())); } else if (keywordMap[JSONSchemaParser::kSchemaExclusiveMaximumKeyword]) { // If "exclusiveMaximum" is present, "maximum" must also be present. return {ErrorCodes::FailedToParse, @@ -1506,7 +1498,7 @@ Status translateScalarKeywords(const boost::intrusive_ptr<ExpressionContext>& ex if (!minExpr.isOK()) { return minExpr.getStatus(); } - andExpr->add(minExpr.getValue().release()); + andExpr->add(std::move(minExpr.getValue())); } else if (keywordMap[JSONSchemaParser::kSchemaExclusiveMinimumKeyword]) { // If "exclusiveMinimum" is present, "minimum" must also be present. return {ErrorCodes::FailedToParse, @@ -1578,14 +1570,14 @@ Status translateEncryptionKeywords(StringMap<BSONElement>& keywordMap, auto encryptInfo = EncryptionInfo::parse(encryptCtxt, encryptElt.embeddedObject()); auto infoType = encryptInfo.getBsonType(); - andExpr->add(new InternalSchemaBinDataSubTypeExpression( + andExpr->add(std::make_unique<InternalSchemaBinDataSubTypeExpression>( path, BinDataType::Encrypt, doc_validation_error::createAnnotation( expCtx, encryptElt.fieldNameStringData().toString(), BSONObj()))); if (auto typeOptional = infoType) { - andExpr->add(new InternalSchemaBinDataEncryptedTypeExpression( + andExpr->add(std::make_unique<InternalSchemaBinDataEncryptedTypeExpression>( path, typeOptional->typeSet(), doc_validation_error::createAnnotation( @@ -1806,7 +1798,7 @@ StatusWithMatchExpression _parse(const boost::intrusive_ptr<ExpressionContext>& } if (!path.empty() && typeExpr) { - andExpr->add(typeExpr.release()); + andExpr->add(std::move(typeExpr)); } return {std::move(andExpr)}; } |