folly::FutureException inherits from std::logic_error, like std::future_error; become...
authorEric Niebler <eniebler@fb.com>
Thu, 29 Dec 2016 19:50:57 +0000 (11:50 -0800)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Thu, 29 Dec 2016 20:03:04 +0000 (12:03 -0800)
Summary:
folly::FutureException was inheriting from std::exception and managing its own message in a std::string data member.
That is suboptimal for the following reasons:

1) The analagous std:: type, std::future_error, inherits from std::logic_error. According to the Principle of Least
   Astonishment, folly::FutureExpception should follow suit.
2) std::logic_error has a nothrow copy constructor. (This is typically implemented with a ref-counted string.)
   std::string does not. By explicitly managing its own message, folly::FutureException picks up a throwing copy
   constructor. Exception classes should try to be nothrow copyable.
3) With most stdlib implementations, std::string is larger by a lot than the std:: exceptions. By changing
   folly::FutureException as suggested, its size drops from 40 bytes to 16 on clang and gcc >=5.0.

Also, I took the liberty of fixing some copy-pastas that gave some of these exception types explicit default
constructors.

Reviewed By: yfeldblum

Differential Revision: D4367909

fbshipit-source-id: 1639d404237493f929db6116a760c2d0e599877d

folly/futures/FutureException.h

index 8e10746ae19d00c16d5e8f8763211b4ecaa4b821..7babb4b35b7aebaa113c291f051abd259fb04b49 100644 (file)
 
 #pragma once
 
-#include <exception>
+#include <stdexcept>
 #include <string>
 
 namespace folly {
 
-class FutureException : public std::exception {
-
-public:
-
-  explicit FutureException(std::string message_arg)
-    : message(message_arg) {}
-
-  ~FutureException() throw(){}
-
-  virtual const char *what() const throw() {
-    return message.c_str();
-  }
-
-  bool operator==(const FutureException &other) const{
-    return other.message == this->message;
-  }
-
-  bool operator!=(const FutureException &other) const{
-    return !(*this == other);
-  }
-
-  protected:
-    std::string message;
+class FutureException : public std::logic_error {
+ public:
+  using std::logic_error::logic_error;
 };
 
 class BrokenPromise : public FutureException {
-  public:
-    explicit BrokenPromise(std::string type) :
-      FutureException(
-          (std::string("Broken promise for type name `") + type) + '`') { }
+ public:
+  explicit BrokenPromise(const std::string& type)
+      : FutureException("Broken promise for type name `" + type + '`') {}
+
+  explicit BrokenPromise(const char* type) : BrokenPromise(std::string(type)) {}
 };
 
 class NoState : public FutureException {
 public:
-    explicit NoState() : FutureException("No state") { }
+ public:
+  NoState() : FutureException("No state") {}
 };
 
 class PromiseAlreadySatisfied : public FutureException {
-  public:
-    explicit PromiseAlreadySatisfied() :
-      FutureException("Promise already satisfied") { }
+ public:
+  PromiseAlreadySatisfied() : FutureException("Promise already satisfied") {}
 };
 
 class FutureNotReady : public FutureException {
-  public:
-    explicit FutureNotReady() :
-      FutureException("Future not ready") { }
+ public:
+  FutureNotReady() : FutureException("Future not ready") {}
 };
 
 class FutureAlreadyRetrieved : public FutureException {
-  public:
-    explicit FutureAlreadyRetrieved () :
-      FutureException("Future already retrieved") { }
+ public:
+  FutureAlreadyRetrieved() : FutureException("Future already retrieved") {}
 };
 
 class FutureCancellation : public FutureException {