-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clean OpProtoAndCheckerMaker #10486
Clean OpProtoAndCheckerMaker #10486
Conversation
Do not use ctor * Reduce line of codes. * We can use virtual function for Maker now. * The implementation does not care what maker holds, it is easier to refactor later.
095fd3b
to
0e78cb6
Compare
… feature/clean_op_maker
|
||
virtual ~OpProtoAndCheckerMaker() { | ||
PADDLE_ENFORCE(validated_, "should call Validate after build"); | ||
CHECK(validated_) << "should call Validate after build"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exception in desturctor will only crash and cannot be catched. Change here to CHECK
} | ||
|
||
void SetProto(proto::OpProto *proto) { proto_ = proto; } | ||
|
||
void SetChecker(OpAttrChecker *attr_checker) { op_checker_ = attr_checker; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not use constructor to avoid all inherited classes to have a constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This replacement looks great.
Do not use ctor
refactor later.