From: Eric Niebler Date: Thu, 29 Dec 2016 19:50:57 +0000 (-0800) Subject: folly::FutureException inherits from std::logic_error, like std::future_error; become... X-Git-Tag: v2017.03.06.00~152 X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=42ba1a2241675cac61c0b61214d2a895ef0c2a3e folly::FutureException inherits from std::logic_error, like std::future_error; becomes smaller and nothrow-copy 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 --- diff --git a/folly/futures/FutureException.h b/folly/futures/FutureException.h index 8e10746a..7babb4b3 100644 --- a/folly/futures/FutureException.h +++ b/folly/futures/FutureException.h @@ -16,64 +16,42 @@ #pragma once -#include +#include #include 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 {