diff --git a/paddle/framework/operator.h b/paddle/framework/operator.h index 4a1dee6fb00d778799865a0199cffcb41ce0aaef..9e8aef6f8563bacaa0bc5bc46abc031dadaa39ae 100644 --- a/paddle/framework/operator.h +++ b/paddle/framework/operator.h @@ -112,8 +112,8 @@ class OperatorBase { const AttributeMap& Attrs() const { return attrs_; } // Return a new operator instance, which is as same as this. - // NOTE: It is caller's responsibility to delete that operator instance. - virtual OperatorBase* Clone() const = 0; + // Use unique_ptr to prevent caller forget to delete this pointer. + virtual std::unique_ptr Clone() const = 0; public: std::string type_; @@ -132,8 +132,10 @@ class OperatorBase { // Macro for define a clone method. // If you are writing an kernel operator, `Clone` will be defined when you // register it. -#define DEFINE_OP_CLONE_METHOD(CLS) \ - OperatorBase* Clone() const final { return new CLS(*this); } +#define DEFINE_OP_CLONE_METHOD(CLS) \ + std::unique_ptr Clone() const final { \ + return std::unique_ptr(new CLS(*this)); \ + } // Macro for define a default constructor for Operator. // You can also use diff --git a/paddle/framework/operator_test.cc b/paddle/framework/operator_test.cc index ceba7f5e6ea485f0e3642dcb4984822a783d6d4d..8836217126581510e10bf40e411e6fdf96b9d381 100644 --- a/paddle/framework/operator_test.cc +++ b/paddle/framework/operator_test.cc @@ -257,7 +257,6 @@ class OperatorClone : public paddle::framework::OperatorBase { TEST(Operator, Clone) { OperatorClone a("ABC", {}, {}, {}); - auto* b = a.Clone(); + auto b = a.Clone(); ASSERT_EQ(a.Type(), b->Type()); - delete b; } \ No newline at end of file diff --git a/paddle/operators/net_op.cc b/paddle/operators/net_op.cc index 896550f9d0c55665a9c9d6f133dc8f27bc5a07f1..77eb07e2f9018d966897f4b6eb1308b28394c03d 100644 --- a/paddle/operators/net_op.cc +++ b/paddle/operators/net_op.cc @@ -85,13 +85,13 @@ NetOp::NetOp(const std::string& type, const framework::OperatorBase::VarNameMap& inputs, const framework::OperatorBase::VarNameMap& outputs, const framework::AttributeMap& attrs) - : OperatorBase(type, inputs, outputs, attrs) {} + : framework::OperatorBase(type, inputs, outputs, attrs) {} -framework::OperatorBase* NetOp::Clone() const { +std::unique_ptr NetOp::Clone() const { PADDLE_ENFORCE( add_op_done_, "Must clone a sealed NetOp, invoke Net::CompleteAddOp before clone"); - return new NetOp(*this); + return std::unique_ptr(new NetOp(*this)); } } // namespace operators diff --git a/paddle/operators/net_op.h b/paddle/operators/net_op.h index deee543065ab716d9d802647745c3884c1e80380..743f0e67dbeaab2de97a6cf635aad0ee90b2cef1 100644 --- a/paddle/operators/net_op.h +++ b/paddle/operators/net_op.h @@ -109,7 +109,8 @@ class NetOp : public framework::OperatorBase { bool IsNetOp() const override; std::vector OutputVars(bool has_intermediate) const override; - framework::OperatorBase* Clone() const override; + + std::unique_ptr Clone() const override; std::vector> ops_; diff --git a/paddle/operators/net_op_test.cc b/paddle/operators/net_op_test.cc index 40e43f46df79fa311f7113b347a2d7c179c30eeb..6d6f8bd354817cd5cabd050b703e16bdeeaa623b 100644 --- a/paddle/operators/net_op_test.cc +++ b/paddle/operators/net_op_test.cc @@ -84,14 +84,13 @@ TEST(NetOp, Clone) { net.AddOp(std::shared_ptr(new EmptyOp{"empty", {}, {}, {}})); net.AddOp(std::shared_ptr(new EmptyOp{"empty2", {}, {}, {}})); net.CompleteAddOp(true); - auto* new_net_op = net.Clone(); + auto new_net_op = net.Clone(); ASSERT_NE(new_net_op, nullptr); ASSERT_TRUE(new_net_op->IsNetOp()); - auto* new_net = static_cast(new_net_op); + auto* new_net = static_cast(new_net_op.get()); ASSERT_EQ(2, new_net->ops_.size()); ASSERT_EQ(new_net->ops_[0]->Type(), "empty"); ASSERT_EQ(new_net->ops_[1]->Type(), "empty2"); - delete new_net; } } // namespace operators