Make Error interface cleaner

Author: reyoung

paddle/gserver/activations/ActivationFunction.cpp

@@ -169,7 +169,7 @@ Argument argument_;
 public:
 Error __must_check forward(Argument& act) {
  if (act.value->getWidth() != 1UL) {
- return ErrorF(
+ return Error(
  "Input width for each timestep of sequence softmax should be 1");
  }
 
@@ -193,7 +193,7 @@ Error __must_check forward(Argument& act) {
 
 Error __must_check backward(Argument& act) {
  if (act.value->getWidth() != 1UL) {
- return ErrorF(
+ return Error(
  "Input width for each timestep of sequence softmax should be 1");
  }
 
@@ -208,7 +208,7 @@ Error __must_check backward(Argument& act) {
  argument_.grad->setData(act.grad->getData() + offset, 1UL, size);
 
  Error status = softmax_.backward(argument_);
- if (!status.isOK()) return status;
+ if (!status) return status;
  }
  return Error();
 }

paddle/utils/Error.h

@@ -23,14 +23,12 @@ limitations under the License. */
 namespace paddle {
 
 /**
- * Status is Paddle error code. It only contain a std::string as error message.
- * Although Status inherits the std::exception, but do not throw it except you
- * know what you are doing.
+ * Error is Paddle error code. It only contain a std::string as error message.
  *
  *
- * There are two styles to return status in Paddle.
+ * There are two styles to return error in Paddle.
  *
- * 1. Return Status
+ * 1. Return Error
  * When method return a status, the return must use `__must_check` attribute.
  * Example as below.
  * @code{cpp}
@@ -39,29 +37,29 @@ namespace paddle {
  * Error __must_check bar() {
  * // do something.
  * Status s = foo(); // invoke other method return status.
- * if (!s.isOK()) return s;
+ * if (!s) return s;
  * // do something else.
  * return Status();
  * }
  * @endcode{cpp}
  *
  * 2. Return by parameter.
- * It is another way to return a status, by using a pointer parameter.
+ * It is another way to return an error, by using a pointer parameter.
  * Example as below.
  *
  * @code{cpp}
  * Error bar();
  *
- * int foo(Error* status) {
+ * int foo(Error* error) {
  * // Do something.
- * Status s = bar();
- * if (!s.isOK()) {
- * *status = s;
+ * Error s = bar();
+ * if (!s) {
+ * *error = s;
  * return 0;
  * }
  * // Do something else.
  * if (someInternalErrorHappend) {
- * *status = ErrorF("Some dimension is too large, %d", dimension);
+ * *error = Error("Some dimension is too large, %d", dimension);
  * return 0;
  * }
  * // End of method.
@@ -72,7 +70,7 @@ namespace paddle {
  * Error s;
  * // do something.
  * foo(&s);
- * if (!s.isOK()) return s;
+ * if (!s) return s;
  * }
  * @endcode{cpp}
  *
@@ -81,17 +79,31 @@ namespace paddle {
  * use log(FATAL) or CHECK to make program exit before. When we clean all
  * log(FATAL) and CHECK in Paddle, 'check' method will be removed.
  */
-class Error final : public std::exception {
+class Error final {
 public:
  /**
  * Default Status. OK
  */
- Error() noexcept {}
+ inline Error() {}
 
  /**
- * @brief what will return the error message. If status is OK, return nullptr.
+ * @brief Create an Error use printf syntax.
  */
- const char* what() const noexcept override {
+ inline explicit Error(const char* fmt, ...) {
+ va_list ap;
+ va_start(ap, fmt);
+ constexpr size_t kBufferSize = 1024;
+ this->errMsg_.reset(new std::string(kBufferSize, 0));
+ auto sz = vsnprintf(&(*errMsg_)[0], kBufferSize, fmt, ap);
+ this->errMsg_->resize(sz);
+ this->errMsg_->shrink_to_fit();
+ va_end(ap);
+ }
+
+ /**
+ * @brief what will return the error message. If no error, return nullptr.
+ */
+ inline const char* msg() const {
  if (errMsg_) {
  return errMsg_->data();
  } else {
@@ -100,58 +112,18 @@ class Error final : public std::exception {
  }
 
  /**
- * @brief isOK
- * @return true if OK.
+ * @brief operator bool, return True if there is no error.
  */
- inline bool isOK() const noexcept { return errMsg_ == nullptr; }
-
+ inline operator bool() const { return !errMsg_; }
  /**
  * @brief check this status by glog.
  * @note It is a temp method used during cleaning Paddle code. It will be
  * removed later.
  */
- inline void check() const { CHECK(isOK()) << what(); }
-
- /**
- * friend method to create Error.
- */
- template <typename... ARGS>
- friend Error __must_check ErrorF(const char* fmt, ARGS... args);
+ inline void check() const { CHECK(*this) << msg(); }
 
 private:
  std::shared_ptr<std::string> errMsg_;
 };
 
-/**
- * ErrorF will create an Error by printf syntax.
- *
- * Specialize this method because clang will give a warning when use printf(fmt)
- * without arguments.
- */
-template <>
-inline Error __must_check ErrorF(const char* msg) {
- Error e;
- e.errMsg_.reset(new std::string(msg));
- return e;
-}
-
-/**
- * ErrorF will create an Error by printf syntax.
- *
- * Examples:
- * @code{cpp}
- * auto err = ErrorF("SomeError");
- * auto err2 = ErrorF("SomeErrorWithParameter %f %d", real_val, int_val);
- * @endcode{cpp}
- */
-template <typename... ARGS>
-inline Error __must_check ErrorF(const char* fmt, ARGS... args) {
- constexpr size_t kBufferSize = 1024;
- char buffer[kBufferSize];
- snprintf(buffer, kBufferSize, fmt, args...);
- Error e;
- e.errMsg_.reset(new std::string(buffer));
- return e;
-}
-
 } // namespace paddle

paddle/utils/tests/test_Error.cpp

@@ -18,17 +18,17 @@ limitations under the License. */
 
 TEST(Error, testAll) {
  paddle::Error error;
- ASSERT_TRUE(error.isOK());
- error = paddle::ErrorF("I'm the error");
- ASSERT_FALSE(error.isOK());
- ASSERT_STREQ("I'm the error", error.what());
+ ASSERT_TRUE(error);
+ error = paddle::Error("I'm the error");
+ ASSERT_FALSE(error);
+ ASSERT_STREQ("I'm the error", error.msg());
 
- error = paddle::ErrorF("error2");
- ASSERT_FALSE(error.isOK());
- ASSERT_STREQ("error2", error.what());
+ error = paddle::Error("error2");
+ ASSERT_FALSE(error);
+ ASSERT_STREQ("error2", error.msg());
 
  int i = 3;
- auto error3 = paddle::ErrorF("error%d", i);
- ASSERT_FALSE(error3.isOK());
- ASSERT_STREQ("error3", error3.what());
+ auto error3 = paddle::Error("error%d", i);
+ ASSERT_FALSE(error3);
+ ASSERT_STREQ("error3", error3.msg());
 }