- Notifications
You must be signed in to change notification settings - Fork 5.9k
Adding Enforce to platform #2646
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
Conversation
dcc7d0c to 09dcc7a Compare Basically from caffe2::logging.h, but only expose `PADDLE_ENFORCE` interface.
09dcc7a to 3a119ef Compare paddle/platform/enforce.h Outdated
| @@ -0,0 +1,116 @@ | |||
| /* | |||
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.
The Copyright format is not right, please use this
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.
Done.
paddle/platform/enforce_test.cc Outdated
| @@ -0,0 +1,25 @@ | |||
| #include <gtest/gtest.h> | |||
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.
add Copyright info.
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.
Done.
paddle/platform/enforce.h Outdated
| all_msg_ = sout.str(); | ||
| } | ||
| | ||
| const char* what() const noexcept override { return all_msg_.c_str(); } |
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.
why not just return a std::string
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.
because it is the std::exception interface.
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.
got it! thanks~
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.
LGTM
paddle/platform/enforce.h Outdated
| | ||
| namespace details { | ||
| | ||
| inline void MakeStringInternal(std::ostringstream& stream) {} |
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.
We should replace MakeString by string::Printf in the near future. I have a PR working on string::Printf.
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.
Done, and wait for CI. After passing CI tests, this PR will be merged.
Basically from caffe2::logging.h, but only expose
PADDLE_ENFORCEinterface.