From 42ba1a2241675cac61c0b61214d2a895ef0c2a3e Mon Sep 17 00:00:00 2001 From: Eric Niebler Date: Thu, 29 Dec 2016 11:50:57 -0800 Subject: [PATCH] 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 --- folly/futures/FutureException.h | 56 ++++++++++----------------------- 1 file changed, 17 insertions(+), 39 deletions(-) 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 { -- 2.34.1