From f3f4bcfb67d95e56887204081c94141aae4c89bc Mon Sep 17 00:00:00 2001 From: Christopher Dykes Date: Tue, 13 Dec 2016 22:05:21 -0800 Subject: [PATCH] Allow building with -Wmissing-noreturn Summary: If your function doesn't return you should be explicit about it. Reviewed By: meyering Differential Revision: D4309893 fbshipit-source-id: ce275ec8f42e2cb3253a1e40e263934649f09d9e --- folly/FixedString.h | 28 +++++++------------ folly/MallctlHelper.cpp | 2 +- folly/MallctlHelper.h | 2 +- folly/detail/ThreadLocalDetail.h | 2 +- folly/experimental/DynamicParser.cpp | 2 +- folly/experimental/DynamicParser.h | 4 +-- .../exception_tracer/ExceptionTracerTest.cpp | 4 +-- .../test/ExceptionCounterTest.cpp | 12 ++++++-- folly/fibers/Fiber.cpp | 2 +- folly/fibers/Fiber.h | 2 +- folly/ssl/OpenSSLHash.cpp | 6 ++-- folly/ssl/OpenSSLHash.h | 7 +++-- folly/test/SafeAssertTest.cpp | 2 +- folly/test/ScopeGuardTest.cpp | 1 + .../test/SubprocessTestParentDeathHelper.cpp | 2 +- .../function_benchmark/test_functions.cpp | 1 + .../test/function_benchmark/test_functions.h | 2 +- 17 files changed, 42 insertions(+), 39 deletions(-) diff --git a/folly/FixedString.h b/folly/FixedString.h index 0f00e6ed..6291ffeb 100644 --- a/folly/FixedString.h +++ b/folly/FixedString.h @@ -102,19 +102,11 @@ constexpr std::size_t checkOverflowOrNpos(std::size_t i, std::size_t max) { } // Intentionally NOT constexpr. See note above for assertOutOfBounds -inline void assertOutOfBoundsNothrow() { - assert(false && "Array index out of bounds in BasicFixedString"); -} - -constexpr std::size_t checkOverflowNothrow(std::size_t i, std::size_t max) { - return i <= max ? i : (assertOutOfBoundsNothrow(), i); -} - -// Intentionally NOT constexpr. See note above for assertOutOfBounds -inline void assertNotNullTerminated() noexcept { +[[noreturn]] inline void assertNotNullTerminated() noexcept { assert( false && "Non-null terminated string used to initialize a BasicFixedString"); + std::terminate(); // Fail hard, fail fast. } // Parsing help for human readers: the following is a constexpr noexcept @@ -1153,7 +1145,7 @@ class BasicFixedString : private detail::fixedstring::FixedStringBase { #ifdef NDEBUG return data_[i]; #else - return data_[detail::fixedstring::checkOverflowNothrow(i, size_)]; + return data_[detail::fixedstring::checkOverflow(i, size_)]; #endif } @@ -1164,21 +1156,21 @@ class BasicFixedString : private detail::fixedstring::FixedStringBase { #ifdef NDEBUG return data_[i]; #else - return data_[detail::fixedstring::checkOverflowNothrow(i, size_)]; + return data_[detail::fixedstring::checkOverflow(i, size_)]; #endif } /** * \note Equivalent to `(*this)[0]` */ - FOLLY_CPP14_CONSTEXPR Char& front() noexcept(false) { + FOLLY_CPP14_CONSTEXPR Char& front() noexcept { return (*this)[0u]; } /** * \overload */ - constexpr const Char& front() const noexcept(false) { + constexpr const Char& front() const noexcept { return (*this)[0u]; } @@ -1186,22 +1178,22 @@ class BasicFixedString : private detail::fixedstring::FixedStringBase { * \note Equivalent to `at(size()-1)` * \pre `!empty()` */ - FOLLY_CPP14_CONSTEXPR Char& back() noexcept(false) { + FOLLY_CPP14_CONSTEXPR Char& back() noexcept { #ifdef NDEBUG return data_[size_ - 1u]; #else - return data_[size_ - detail::fixedstring::checkOverflowNothrow(1u, size_)]; + return data_[size_ - detail::fixedstring::checkOverflow(1u, size_)]; #endif } /** * \overload */ - constexpr const Char& back() const noexcept(false) { + constexpr const Char& back() const noexcept { #ifdef NDEBUG return data_[size_ - 1u]; #else - return data_[size_ - detail::fixedstring::checkOverflowNothrow(1u, size_)]; + return data_[size_ - detail::fixedstring::checkOverflow(1u, size_)]; #endif } diff --git a/folly/MallctlHelper.cpp b/folly/MallctlHelper.cpp index 7e224d08..a9c0c397 100644 --- a/folly/MallctlHelper.cpp +++ b/folly/MallctlHelper.cpp @@ -24,7 +24,7 @@ namespace folly { namespace detail { -void handleMallctlError(const char* cmd, int err) { +[[noreturn]] void handleMallctlError(const char* cmd, int err) { assert(err != 0); throw std::runtime_error( sformat("mallctl {}: {} ({})", cmd, errnoStr(err), err)); diff --git a/folly/MallctlHelper.h b/folly/MallctlHelper.h index 5c66c476..2d74205d 100644 --- a/folly/MallctlHelper.h +++ b/folly/MallctlHelper.h @@ -27,7 +27,7 @@ namespace folly { namespace detail { -void handleMallctlError(const char* cmd, int err); +[[noreturn]] void handleMallctlError(const char* cmd, int err); template void mallctlHelper(const char* cmd, T* out, T* in) { diff --git a/folly/detail/ThreadLocalDetail.h b/folly/detail/ThreadLocalDetail.h index c04fee1e..418889ff 100644 --- a/folly/detail/ThreadLocalDetail.h +++ b/folly/detail/ThreadLocalDetail.h @@ -256,7 +256,7 @@ struct StaticMetaBase { StaticMetaBase(ThreadEntry* (*threadEntry)(), bool strict); - ~StaticMetaBase() { + [[noreturn]] ~StaticMetaBase() { LOG(FATAL) << "StaticMeta lives forever!"; } diff --git a/folly/experimental/DynamicParser.cpp b/folly/experimental/DynamicParser.cpp index 4038af97..18dd25fd 100644 --- a/folly/experimental/DynamicParser.cpp +++ b/folly/experimental/DynamicParser.cpp @@ -168,7 +168,7 @@ folly::dynamic DynamicParser::ParserStack::releaseErrors() { return releaseErrorsImpl(); } -void DynamicParser::ParserStack::throwErrors() { +[[noreturn]] void DynamicParser::ParserStack::throwErrors() { throw DynamicParserParseError(releaseErrorsImpl()); } diff --git a/folly/experimental/DynamicParser.h b/folly/experimental/DynamicParser.h index 4124d8ca..8541313f 100644 --- a/folly/experimental/DynamicParser.h +++ b/folly/experimental/DynamicParser.h @@ -357,9 +357,9 @@ private: folly::dynamic releaseErrors(); // Invoked on error when using OnError::THROW. - void throwErrors(); + [[noreturn]] void throwErrors(); - private: + private: friend struct Pop; folly::dynamic releaseErrorsImpl(); // for releaseErrors() & throwErrors() diff --git a/folly/experimental/exception_tracer/ExceptionTracerTest.cpp b/folly/experimental/exception_tracer/ExceptionTracerTest.cpp index 2db6f9f8..25a66524 100644 --- a/folly/experimental/exception_tracer/ExceptionTracerTest.cpp +++ b/folly/experimental/exception_tracer/ExceptionTracerTest.cpp @@ -20,7 +20,7 @@ #include -void bar() { +[[noreturn]] void bar() { throw std::runtime_error("hello"); } @@ -45,7 +45,7 @@ void foo() { } } -void baz() { +[[noreturn]] void baz() { try { try { bar(); diff --git a/folly/experimental/exception_tracer/test/ExceptionCounterTest.cpp b/folly/experimental/exception_tracer/test/ExceptionCounterTest.cpp index 706177b8..6751396b 100644 --- a/folly/experimental/exception_tracer/test/ExceptionCounterTest.cpp +++ b/folly/experimental/exception_tracer/test/ExceptionCounterTest.cpp @@ -26,11 +26,17 @@ struct MyException {}; -void bar() { throw std::runtime_error("hello"); } +[[noreturn]] void bar() { + throw std::runtime_error("hello"); +} -void foo() { throw MyException(); } +[[noreturn]] void foo() { + throw MyException(); +} -void baz() { foo(); } +[[noreturn]] void baz() { + foo(); +} using namespace folly::exception_tracer; diff --git a/folly/fibers/Fiber.cpp b/folly/fibers/Fiber.cpp index de4124f7..6caa9d55 100644 --- a/folly/fibers/Fiber.cpp +++ b/folly/fibers/Fiber.cpp @@ -118,7 +118,7 @@ void Fiber::recordStackPosition() { VLOG(4) << "Stack usage: " << currentPosition; } -void Fiber::fiberFunc() { +[[noreturn]] void Fiber::fiberFunc() { #ifdef FOLLY_SANITIZE_ADDRESS fiberManager_.registerFinishSwitchStackWithAsan( nullptr, &asanMainStackBase_, &asanMainStackSize_); diff --git a/folly/fibers/Fiber.h b/folly/fibers/Fiber.h index c43894f2..82b73bc6 100644 --- a/folly/fibers/Fiber.h +++ b/folly/fibers/Fiber.h @@ -90,7 +90,7 @@ class Fiber { template void setFunctionFinally(F&& func, G&& finally); - void fiberFunc(); + [[noreturn]] void fiberFunc(); /** * Switch out of fiber context into the main context, diff --git a/folly/ssl/OpenSSLHash.cpp b/folly/ssl/OpenSSLHash.cpp index 9e3581aa..7b1c4394 100644 --- a/folly/ssl/OpenSSLHash.cpp +++ b/folly/ssl/OpenSSLHash.cpp @@ -21,12 +21,14 @@ namespace folly { namespace ssl { -void OpenSSLHash::check_out_size_throw(size_t size, MutableByteRange out) { +[[noreturn]] void OpenSSLHash::check_out_size_throw( + size_t size, + MutableByteRange out) { throw std::invalid_argument(folly::sformat( "expected out of size {} but was of size {}", size, out.size())); } -void OpenSSLHash::check_libssl_result_throw() { +[[noreturn]] void OpenSSLHash::check_libssl_result_throw() { throw std::runtime_error("openssl crypto function failed"); } diff --git a/folly/ssl/OpenSSLHash.h b/folly/ssl/OpenSSLHash.h index 5081faec..2a89e64b 100644 --- a/folly/ssl/OpenSSLHash.h +++ b/folly/ssl/OpenSSLHash.h @@ -173,7 +173,9 @@ class OpenSSLHash { } check_out_size_throw(size, out); } - static void check_out_size_throw(size_t size, MutableByteRange out); + [[noreturn]] static void check_out_size_throw( + size_t size, + MutableByteRange out); static inline void check_libssl_result(int expected, int result) { if (LIKELY(result == expected)) { @@ -181,8 +183,7 @@ class OpenSSLHash { } check_libssl_result_throw(); } - static void check_libssl_result_throw(); - + [[noreturn]] static void check_libssl_result_throw(); }; } diff --git a/folly/test/SafeAssertTest.cpp b/folly/test/SafeAssertTest.cpp index 0cb59b96..f2015ce7 100644 --- a/folly/test/SafeAssertTest.cpp +++ b/folly/test/SafeAssertTest.cpp @@ -23,7 +23,7 @@ using namespace folly; -void fail() { +[[noreturn]] void fail() { FOLLY_SAFE_CHECK(0 + 0, "hello"); } diff --git a/folly/test/ScopeGuardTest.cpp b/folly/test/ScopeGuardTest.cpp index 892a4352..b3eb981b 100644 --- a/folly/test/ScopeGuardTest.cpp +++ b/folly/test/ScopeGuardTest.cpp @@ -295,6 +295,7 @@ TEST(ScopeGuard, TEST_THROWING_CLEANUP_ACTION) { struct ThrowingCleanupAction { explicit ThrowingCleanupAction(int& scopeExitExecuted) : scopeExitExecuted_(scopeExitExecuted) {} + [[noreturn]] ThrowingCleanupAction(const ThrowingCleanupAction& other) : scopeExitExecuted_(other.scopeExitExecuted_) { throw std::runtime_error("whoa"); diff --git a/folly/test/SubprocessTestParentDeathHelper.cpp b/folly/test/SubprocessTestParentDeathHelper.cpp index 68a601d9..6dfa896e 100644 --- a/folly/test/SubprocessTestParentDeathHelper.cpp +++ b/folly/test/SubprocessTestParentDeathHelper.cpp @@ -60,7 +60,7 @@ void runChild(const char* file) { CHECK_ERR(creat(file, 0600)); } -void runParent(const char* file) { +[[noreturn]] void runParent(const char* file) { std::vector args {"/proc/self/exe", "--child", file}; Subprocess proc( args, diff --git a/folly/test/function_benchmark/test_functions.cpp b/folly/test/function_benchmark/test_functions.cpp index 7da5df99..0818b468 100644 --- a/folly/test/function_benchmark/test_functions.cpp +++ b/folly/test/function_benchmark/test_functions.cpp @@ -36,6 +36,7 @@ class Exception : public std::exception { void doNothing() { } +[[noreturn]] void throwException() { throw Exception("this is a test"); } diff --git a/folly/test/function_benchmark/test_functions.h b/folly/test/function_benchmark/test_functions.h index e248aa63..c6d82fe0 100644 --- a/folly/test/function_benchmark/test_functions.h +++ b/folly/test/function_benchmark/test_functions.h @@ -24,7 +24,7 @@ void doNothing(); -void throwException(); +[[noreturn]] void throwException(); std::exception_ptr returnExceptionPtr(); void exceptionPtrReturnParam(std::exception_ptr* excReturn); std::string returnString(); -- 2.34.1