Remove some [[noreturn]] functions

Summary: [Folly] Remove some `[[noreturn]]` functions - many of them can be replaced with `throw_exception`.

Reviewed By: Orvid

Differential Revision: D9624966

fbshipit-source-id: e5e472c660677112edf99e30e36d78c1b1718b52

folly/Chrono.h

@@ -21,6 +21,7 @@
 #include <type_traits>
 
 #include <folly/Portability.h>
+#include <folly/lang/Exception.h>
 #include <folly/portability/Time.h>
 
 /***
@@ -159,11 +160,6 @@ constexpr std::chrono::time_point<Clock, To> round(
 
 namespace folly {
 namespace chrono {
-namespace detail {
-[[noreturn]] FOLLY_NOINLINE inline void throw_coarse_steady_clock_now_exn() {
-  throw std::runtime_error("Error using CLOCK_MONOTONIC_COARSE.");
-}
-} // namespace detail
 
 struct coarse_steady_clock {
   using rep = std::chrono::milliseconds::rep;
@@ -180,7 +176,8 @@ struct coarse_steady_clock {
     timespec ts;
     auto ret = clock_gettime(CLOCK_MONOTONIC_COARSE, &ts);
     if (ret != 0) {
-      detail::throw_coarse_steady_clock_now_exn();
+      throw_exception<std::runtime_error>(
+          "Error using CLOCK_MONOTONIC_COARSE.");
     }
     return time_point(std::chrono::duration_cast<duration>(
         std::chrono::seconds(ts.tv_sec) +
@@ -188,5 +185,6 @@ struct coarse_steady_clock {
 #endif
   }
 };
+
 } // namespace chrono
 } // namespace folly

folly/Format-inl.h

@@ -29,6 +29,7 @@
 #include <folly/FormatTraits.h>
 #include <folly/MapUtil.h>
 #include <folly/Traits.h>
+#include <folly/lang/Exception.h>
 #include <folly/portability/Windows.h>
 
 // Ignore -Wformat-nonliteral warnings within this file
@@ -184,7 +185,8 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
       p = q;
 
       if (p == end || *p != '}') {
-        throwBadFormatArg("folly::format: single '}' in format string");
+        throw_exception<BadFormatArg>(
+            "folly::format: single '}' in format string");
       }
       ++p;
     }
@@ -206,7 +208,8 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
     p = q + 1;
 
     if (p == end) {
-      throwBadFormatArg("folly::format: '}' at end of format string");
+      throw_exception<BadFormatArg>(
+          "folly::format: '}' at end of format string");
     }
 
     // "{{" -> "{"
@@ -219,7 +222,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
     // Format string
     q = static_cast<const char*>(memchr(p, '}', size_t(end - p)));
     if (q == nullptr) {
-      throwBadFormatArg("folly::format: missing ending '}'");
+      throw_exception<BadFormatArg>("folly::format: missing ending '}'");
     }
     FormatArg arg(StringPiece(p, q));
     p = q + 1;
@@ -268,7 +271,7 @@ void BaseFormatter<Derived, containerMode, Args...>::operator()(
     }
 
     if (hasDefaultArgIndex && hasExplicitArgIndex) {
-      throwBadFormatArg(
+      throw_exception<BadFormatArg>(
           "folly::format: may not have both default and explicit arg indexes");
     }
 
@@ -294,10 +297,10 @@ namespace format_value {
 template <class FormatCallback>
 void formatString(StringPiece val, FormatArg& arg, FormatCallback& cb) {
   if (arg.width != FormatArg::kDefaultWidth && arg.width < 0) {
-    throwBadFormatArg("folly::format: invalid width");
+    throw_exception<BadFormatArg>("folly::format: invalid width");
   }
   if (arg.precision != FormatArg::kDefaultPrecision && arg.precision < 0) {
-    throwBadFormatArg("folly::format: invalid precision");
+    throw_exception<BadFormatArg>("folly::format: invalid precision");
   }
 
   if (arg.precision != FormatArg::kDefaultPrecision &&
@@ -957,7 +960,7 @@ struct KeyableTraitsAssoc : public FormatTraitsBase {
     if (auto ptr = get_ptr(map, KeyFromStringPiece<key_type>::convert(key))) {
       return *ptr;
     }
-    detail::throwFormatKeyNotFoundException(key);
+    throw_exception<FormatKeyNotFoundException>(key);
   }
   static const value_type&
   at(const T& map, StringPiece key, const value_type& dflt) {

folly/Format.cpp

@@ -422,10 +422,4 @@ FormatKeyNotFoundException::FormatKeyNotFoundException(StringPiece key)
 
 constexpr StringPiece const FormatKeyNotFoundException::kMessagePrefix;
 
-namespace detail {
-[[noreturn]] void throwFormatKeyNotFoundException(StringPiece key) {
-  throw FormatKeyNotFoundException(key);
-}
-} // namespace detail
-
 } // namespace folly

folly/Format.h

@@ -331,10 +331,6 @@ class FOLLY_EXPORT FormatKeyNotFoundException : public std::out_of_range {
   static constexpr StringPiece const kMessagePrefix = "format key not found: ";
 };
 
-namespace detail {
-[[noreturn]] void throwFormatKeyNotFoundException(StringPiece key);
-} // namespace detail
-
 /**
  * Wrap a sequence or associative container so that out-of-range lookups
  * return a default value rather than throwing an exception.

folly/FormatArg.h

@@ -23,6 +23,7 @@
 #include <folly/Likely.h>
 #include <folly/Portability.h>
 #include <folly/Range.h>
+#include <folly/lang/Exception.h>
 
 namespace folly {
 
@@ -30,9 +31,6 @@ class FOLLY_EXPORT BadFormatArg : public std::invalid_argument {
   using invalid_argument::invalid_argument;
 };
 
-[[noreturn]] void throwBadFormatArg(char const* msg);
-[[noreturn]] void throwBadFormatArg(std::string const& msg);
-
 /**
  * Parsed format argument.
  */
@@ -215,7 +213,7 @@ inline std::string FormatArg::errorStr(Args&&... args) const {
 
 template <typename... Args>
 [[noreturn]] inline void FormatArg::error(Args&&... args) const {
-  throwBadFormatArg(errorStr(std::forward<Args>(args)...));
+  throw_exception<BadFormatArg>(errorStr(std::forward<Args>(args)...));
 }
 
 template <bool emptyOk>

folly/Poly.cpp

@@ -22,8 +22,9 @@
 
 namespace folly {
 namespace detail {
-[[noreturn]] void throwBadPolyAccess() { throw BadPolyAccess(); }
-[[noreturn]] void throwBadPolyCast() { throw BadPolyCast(); }
+
+// empty
+
 } // namespace detail
 } // namespace folly
 

folly/Poly.h

@@ -47,6 +47,7 @@
 #define FOLLY_INLINE_CONSTEXPR inline constexpr
 #endif
 
+#include <folly/PolyException.h>
 #include <folly/detail/PolyDetail.h>
 
 namespace folly {
@@ -173,27 +174,6 @@ struct PolyMembers {};
 
 #endif
 
-/**
- * Exception type that is thrown on invalid access of an empty `Poly` object.
- */
-struct FOLLY_EXPORT BadPolyAccess : std::exception {
-  BadPolyAccess() = default;
-  char const* what() const noexcept override {
-    return "BadPolyAccess";
-  }
-};
-
-/**
- * Exception type that is thrown when attempting to extract from a `Poly` a
- * value of the wrong type.
- */
-struct FOLLY_EXPORT BadPolyCast : std::bad_cast {
-  BadPolyCast() = default;
-  char const* what() const noexcept override {
-    return "BadPolyCast";
-  }
-};
-
 /**
  * Used in the definition of a `Poly` interface to say that the current
  * interface is an extension of a set of zero or more interfaces.

folly/FormatArg.cpp → folly/PolyException.h

@@ -14,14 +14,33 @@
  * limitations under the License.
  */
 
-#include <folly/FormatArg.h>
+#pragma once
+
+#include <exception>
+
+#include <folly/CPortability.h>
 
 namespace folly {
 
-[[noreturn]] void throwBadFormatArg(char const* msg) {
-  throw BadFormatArg(msg);
-}
-[[noreturn]] void throwBadFormatArg(std::string const& msg) {
-  throw BadFormatArg(msg);
-}
+/**
+ * Exception type that is thrown on invalid access of an empty `Poly` object.
+ */
+struct FOLLY_EXPORT BadPolyAccess : std::exception {
+  BadPolyAccess() = default;
+  char const* what() const noexcept override {
+    return "BadPolyAccess";
+  }
+};
+
+/**
+ * Exception type that is thrown when attempting to extract from a `Poly` a
+ * value of the wrong type.
+ */
+struct FOLLY_EXPORT BadPolyCast : std::bad_cast {
+  BadPolyCast() = default;
+  char const* what() const noexcept override {
+    return "BadPolyCast";
+  }
+};
+
 } // namespace folly

folly/Singleton.cpp

@@ -184,10 +184,6 @@ void singletonPrintDestructionStackTrace(const TypeDescriptor& type) {
       type.name());
 }
 
-[[noreturn]] void SingletonVaultState::throwUnexpectedState(const char* msg) {
-  throw std::logic_error(msg);
-}
-
 } // namespace detail
 
 namespace {

folly/Singleton.h

@@ -129,6 +129,7 @@
 #include <folly/detail/StaticSingletonManager.h>
 #include <folly/experimental/ReadMostlySharedPtr.h>
 #include <folly/hash/Hash.h>
+#include <folly/lang/Exception.h>
 #include <folly/synchronization/Baton.h>
 #include <folly/synchronization/RWSpinLock.h>
 
@@ -271,11 +272,9 @@ struct SingletonVaultState {
       Type expected,
       const char* msg = "Unexpected singleton state change") const {
     if (expected != state) {
-      throwUnexpectedState(msg);
+      throw_exception<std::logic_error>(msg);
     }
   }
-
-  [[noreturn]] static void throwUnexpectedState(const char* msg);
 };
 
 // This interface is used by SingletonVault to interact with SingletonHolders.

folly/detail/PolyDetail.h

@@ -27,6 +27,9 @@
 #include <folly/Utility.h>
 #include <folly/detail/TypeList.h>
 #include <folly/functional/Invoke.h>
+#include <folly/lang/Exception.h>
+
+#include <folly/PolyException.h>
 
 namespace folly {
 /// \cond
@@ -243,9 +246,6 @@ using MembersOf = typename I::template Members<Uncvref<T>>;
 template <class I, class T>
 using InterfaceOf = typename I::template Interface<T>;
 
-[[noreturn]] void throwBadPolyAccess();
-[[noreturn]] void throwBadPolyCast();
-
 #if !defined(__cpp_template_auto)
 template <class T, T V>
 using Member = std::integral_constant<T, V>;
@@ -440,7 +440,7 @@ struct ThrowThunk {
   constexpr /* implicit */ operator FnPtr<R, Args...>() const noexcept {
     struct _ {
       static R call(Args...) {
-        throwBadPolyAccess();
+        throw_exception<BadPolyAccess>();
       }
     };
     return &_::call;
@@ -598,7 +598,7 @@ void* execOnHeap(Op op, Data* from, void* to) {
       if (*static_cast<std::type_info const*>(to) == typeid(T)) {
         return from->pobj_;
       }
-      throwBadPolyCast();
+      throw_exception<BadPolyCast>();
     case Op::eRefr:
       return vtableForRef<I, Uncvref<T>>(
           static_cast<RefType>(reinterpret_cast<std::uintptr_t>(to)));
@@ -629,7 +629,7 @@ void* execOnHeap(Op op, Data* from, void* to) {
       if (*static_cast<std::type_info const*>(to) == typeid(T)) {
         return from->pobj_;
       }
-      throwBadPolyCast();
+      throw_exception<BadPolyCast>();
     case Op::eRefr:
       return vtableForRef<I, Uncvref<T>>(
           static_cast<RefType>(reinterpret_cast<std::uintptr_t>(to)));
@@ -660,7 +660,7 @@ void* execInSitu(Op op, Data* from, void* to) {
       if (*static_cast<std::type_info const*>(to) == typeid(T)) {
         return &from->buff_;
       }
-      throwBadPolyCast();
+      throw_exception<BadPolyCast>();
     case Op::eRefr:
       return vtableForRef<I, Uncvref<T>>(
           static_cast<RefType>(reinterpret_cast<std::uintptr_t>(to)));
@@ -670,7 +670,7 @@ void* execInSitu(Op op, Data* from, void* to) {
 
 inline void* noopExec(Op op, Data*, void*) {
   if (op == Op::eAddr)
-    throwBadPolyAccess();
+    throw_exception<BadPolyAccess>();
   return const_cast<void*>(static_cast<void const*>(&typeid(void)));
 }
 

folly/dynamic-inl.h

@@ -22,6 +22,7 @@
 #include <folly/Format.h>
 #include <folly/Likely.h>
 #include <folly/detail/Iterators.h>
+#include <folly/lang/Exception.h>
 
 //////////////////////////////////////////////////////////////////////
 
@@ -93,14 +94,6 @@ struct FOLLY_EXPORT TypeError : std::runtime_error {
   ~TypeError() override;
 };
 
-[[noreturn]] void throwTypeError_(
-    std::string const& expected,
-    dynamic::Type actual);
-[[noreturn]] void throwTypeError_(
-    std::string const& expected,
-    dynamic::Type actual1,
-    dynamic::Type actual2);
-
 //////////////////////////////////////////////////////////////////////
 
 namespace detail {
@@ -122,7 +115,7 @@ struct Destroy {
 template <template <class> class Op>
 dynamic numericOp(dynamic const& a, dynamic const& b) {
   if (!a.isNumber() || !b.isNumber()) {
-    throwTypeError_("numeric", a.type(), b.type());
+    throw_exception<TypeError>("numeric", a.type(), b.type());
   }
   if (a.type() != b.type()) {
     auto& integ = a.isInt() ? a : b;
@@ -556,13 +549,13 @@ inline dynamic& dynamic::operator/=(dynamic const& o) {
   return *this;
 }
 
-#define FB_DYNAMIC_INTEGER_OP(op)                          \
-  inline dynamic& dynamic::operator op(dynamic const& o) { \
-    if (!isInt() || !o.isInt()) {                          \
-      throwTypeError_("int64", type(), o.type());          \
-    }                                                      \
-    *getAddress<int64_t>() op o.asInt();                   \
-    return *this;                                          \
+#define FB_DYNAMIC_INTEGER_OP(op)                            \
+  inline dynamic& dynamic::operator op(dynamic const& o) {   \
+    if (!isInt() || !o.isInt()) {                            \
+      throw_exception<TypeError>("int64", type(), o.type()); \
+    }                                                        \
+    *getAddress<int64_t>() op o.asInt();                     \
+    return *this;                                            \
   }
 
 FB_DYNAMIC_INTEGER_OP(%=)
@@ -650,7 +643,7 @@ template <class K, class V> inline void dynamic::insert(K&& key, V&& val) {
 
 inline void dynamic::update(const dynamic& mergeObj) {
   if (!isObject() || !mergeObj.isObject()) {
-    throwTypeError_("object", type(), mergeObj.type());
+    throw_exception<TypeError>("object", type(), mergeObj.type());
   }
 
   for (const auto& pair : mergeObj.items()) {
@@ -660,7 +653,7 @@ inline void dynamic::update(const dynamic& mergeObj) {
 
 inline void dynamic::update_missing(const dynamic& mergeObj1) {
   if (!isObject() || !mergeObj1.isObject()) {
-    throwTypeError_("object", type(), mergeObj1.type());
+    throw_exception<TypeError>("object", type(), mergeObj1.type());
   }
 
   // Only add if not already there
@@ -802,7 +795,7 @@ T dynamic::asImpl() const {
   case STRING:
     return to<T>(*get_nothrow<std::string>());
   default:
-    throwTypeError_("int/double/bool/string", type());
+    throw_exception<TypeError>("int/double/bool/string", type());
   }
 }
 
@@ -872,7 +865,7 @@ T& dynamic::get() {
   if (auto* p = get_nothrow<T>()) {
     return *p;
   }
-  throwTypeError_(TypeInfo<T>::name, type());
+  throw_exception<TypeError>(TypeInfo<T>::name, type());
 }
 
 template <class T>

folly/dynamic.cpp

@@ -72,19 +72,6 @@ TypeError& TypeError::operator=(TypeError&&) noexcept(
     std::is_nothrow_move_assignable<std::runtime_error>::value) = default;
 TypeError::~TypeError() = default;
 
-[[noreturn]] void throwTypeError_(
-    std::string const& expected,
-    dynamic::Type actual) {
-  throw TypeError(expected, actual);
-}
-
-[[noreturn]] void throwTypeError_(
-    std::string const& expected,
-    dynamic::Type actual1,
-    dynamic::Type actual2) {
-  throw TypeError(expected, actual1, actual2);
-}
-
 // This is a higher-order preprocessor macro to aid going from runtime
 // types to the compile time type system.
 #define FB_DYNAMIC_APPLY(type, apply) \
@@ -119,7 +106,7 @@ TypeError::~TypeError() = default;
 
 bool dynamic::operator<(dynamic const& o) const {
   if (UNLIKELY(type_ == OBJECT || o.type_ == OBJECT)) {
-    throwTypeError_("object", type_);
+    throw_exception<TypeError>("object", type_);
   }
   if (type_ != o.type_) {
     return type_ < o.type_;
@@ -182,7 +169,7 @@ dynamic& dynamic::operator=(dynamic&& o) noexcept {
 
 dynamic& dynamic::operator[](dynamic const& k) & {
   if (!isObject() && !isArray()) {
-    throwTypeError_("object/array", type());
+    throw_exception<TypeError>("object/array", type());
   }
   if (isArray()) {
     return at(k);
@@ -229,7 +216,7 @@ dynamic dynamic::getDefault(const dynamic& k, dynamic&& v) && {
 const dynamic* dynamic::get_ptr(dynamic const& idx) const& {
   if (auto* parray = get_nothrow<Array>()) {
     if (!idx.isInt()) {
-      throwTypeError_("int64", idx.type());
+      throw_exception<TypeError>("int64", idx.type());
     }
     if (idx < 0 || idx >= parray->size()) {
       return nullptr;
@@ -242,19 +229,14 @@ const dynamic* dynamic::get_ptr(dynamic const& idx) const& {
     }
     return &it->second;
   } else {
-    throwTypeError_("object/array", type());
+    throw_exception<TypeError>("object/array", type());
   }
 }
 
-[[noreturn]] static void throwOutOfRangeAtMissingKey(dynamic const& idx) {
-  auto msg = sformat("couldn't find key {} in dynamic object", idx.asString());
-  throw_exception<std::out_of_range>(msg);
-}
-
 dynamic const& dynamic::at(dynamic const& idx) const& {
   if (auto* parray = get_nothrow<Array>()) {
     if (!idx.isInt()) {
-      throwTypeError_("int64", idx.type());
+      throw_exception<TypeError>("int64", idx.type());
     }
     if (idx < 0 || idx >= parray->size()) {
       throw_exception<std::out_of_range>("out of range in dynamic array");
@@ -263,11 +245,14 @@ dynamic const& dynamic::at(dynamic const& idx) const& {
   } else if (auto* pobject = get_nothrow<ObjectImpl>()) {
     auto it = pobject->find(idx);
     if (it == pobject->end()) {
-      throwOutOfRangeAtMissingKey(idx);
+      invoke_noreturn_cold([&] {
+        throw_exception<std::out_of_range>(
+            sformat("couldn't find key {} in dynamic object", idx.asString()));
+      });
     }
     return it->second;
   } else {
-    throwTypeError_("object/array", type());
+    throw_exception<TypeError>("object/array", type());
   }
 }
 
@@ -281,7 +266,7 @@ std::size_t dynamic::size() const {
   if (auto* str = get_nothrow<std::string>()) {
     return str->size();
   }
-  throwTypeError_("array/object", type());
+  throw_exception<TypeError>("array/object", type());
 }
 
 dynamic::iterator dynamic::erase(const_iterator first, const_iterator last) {
@@ -384,7 +369,7 @@ const dynamic* dynamic::get_ptr(json_pointer const& jsonPtr) const& {
         dyn = dyn->get_ptr("");
         continue;
       }
-      throwTypeError_("object", dyn->type());
+      throw_exception<TypeError>("object", dyn->type());
     }
     if (auto* parray = dyn->get_nothrow<dynamic::Array>()) {
       if (token.size() > 1 && token.at(0) == '0') {
@@ -405,7 +390,7 @@ const dynamic* dynamic::get_ptr(json_pointer const& jsonPtr) const& {
       dyn = it != pobject->end() ? &it->second : nullptr;
       continue;
     }
-    throwTypeError_("object/array", dyn->type());
+    throw_exception<TypeError>("object/array", dyn->type());
   }
   return dyn;
 }

folly/ssl/OpenSSLHash.cpp

@@ -28,8 +28,5 @@ namespace ssl {
       "expected out of size {} but was of size {}", size, out.size()));
 }
 
-[[noreturn]] void OpenSSLHash::check_libssl_result_throw() {
-  throw std::runtime_error("openssl crypto function failed");
-}
 } // namespace ssl
 } // namespace folly

folly/ssl/OpenSSLHash.h

@@ -177,9 +177,9 @@ class OpenSSLHash {
     if (LIKELY(result == expected)) {
       return;
     }
-    check_libssl_result_throw();
+    throw_exception<std::runtime_error>("openssl crypto function failed");
   }
-  [[noreturn]] static void check_libssl_result_throw();
 };
+
 } // namespace ssl
 } // namespace folly

folly/test/function_benchmark/main.cpp

@@ -231,7 +231,7 @@ BENCHMARK_DRAW_LINE();
 BENCHMARK(throw_exception, iters) {
   for (size_t n = 0; n < iters; ++n) {
     try {
-      throwException();
+      folly::throw_exception<Exception>("this is a test");
     } catch (const std::exception& ex) {
     }
   }

folly/test/function_benchmark/test_functions.cpp

@@ -16,33 +16,17 @@
 
 #include <folly/test/function_benchmark/test_functions.h>
 
+#include <folly/lang/Exception.h>
+
 /*
  * These functions are defined in a separate file so that
  * gcc won't be able to inline them.
  */
 
 
-class Exception : public std::exception {
- public:
-  explicit Exception(const std::string& value) : value_(value) {}
-  ~Exception() noexcept override {}
-
-  const char* what() const noexcept override {
-    return value_.c_str();
-  }
-
- private:
-  std::string value_;
-};
-
 void doNothing() {
 }
 
-[[noreturn]]
-void throwException() {
-  throw Exception("this is a test");
-}
-
 std::exception_ptr returnExceptionPtr() {
   Exception ex("this is a test");
   return std::make_exception_ptr(ex);

folly/test/function_benchmark/test_functions.h

@@ -22,9 +22,21 @@
 
 #include <folly/Function.h>
 
+class Exception : public std::exception {
+ public:
+  explicit Exception(const std::string& value) : value_(value) {}
+  ~Exception() noexcept override {}
+
+  const char* what() const noexcept override {
+    return value_.c_str();
+  }
+
+ private:
+  std::string value_;
+};
+
 void doNothing();
 
-[[noreturn]] void throwException();
 std::exception_ptr returnExceptionPtr();
 void exceptionPtrReturnParam(std::exception_ptr* excReturn);
 std::string returnString();