From 4824fb8374d19f411676eb5dfefb507dd3e747fd Mon Sep 17 00:00:00 2001 From: Christopher Dykes Date: Tue, 13 Dec 2016 10:01:00 -0800 Subject: [PATCH] Enable -Wunreachable-code-return Summary: The most common place this happened was in tests where it was being used to force the return type of a lambda, but there were a couple of places in the main code that triggered this as well. Reviewed By: yfeldblum Differential Revision: D4310187 fbshipit-source-id: e3701cff9827eacaf3be8d28296441466eb2fa11 --- folly/IPAddress.cpp | 1 - folly/Subprocess.cpp | 4 +-- folly/experimental/symbolizer/test/Crash.cpp | 1 - folly/fibers/test/FibersTest.cpp | 6 ++-- folly/futures/test/FutureTest.cpp | 29 ++++++++------------ folly/io/async/AsyncTimeout.cpp | 4 +-- folly/io/async/SSLContext.cpp | 2 -- folly/test/TryTest.cpp | 4 +-- 8 files changed, 17 insertions(+), 34 deletions(-) diff --git a/folly/IPAddress.cpp b/folly/IPAddress.cpp index bb733743..b3ce9ad7 100644 --- a/folly/IPAddress.cpp +++ b/folly/IPAddress.cpp @@ -420,7 +420,6 @@ IPAddress::longestCommonPrefix(const CIDRNetwork& one, const CIDRNetwork& two) { } else { throw std::invalid_argument("Unknown address family"); } - return {IPAddress(0), uint8_t(0)}; } [[noreturn]] void IPAddress::asV4Throw() const { diff --git a/folly/Subprocess.cpp b/folly/Subprocess.cpp index 93e3f577..412a3431 100644 --- a/folly/Subprocess.cpp +++ b/folly/Subprocess.cpp @@ -34,6 +34,7 @@ #include +#include #include #include #include @@ -106,8 +107,7 @@ std::string ProcessReturnCode::str() const { return to("killed by signal ", killSignal(), (coreDumped() ? " (core dumped)" : "")); } - CHECK(false); // unreached - return ""; // silence GCC warning + assume_unreachable(); } CalledProcessError::CalledProcessError(ProcessReturnCode rc) diff --git a/folly/experimental/symbolizer/test/Crash.cpp b/folly/experimental/symbolizer/test/Crash.cpp index a84d0763..92a4a37e 100644 --- a/folly/experimental/symbolizer/test/Crash.cpp +++ b/folly/experimental/symbolizer/test/Crash.cpp @@ -19,5 +19,4 @@ int main() { folly::symbolizer::installFatalSignalHandler(); __builtin_trap(); - return 0; } diff --git a/folly/fibers/test/FibersTest.cpp b/folly/fibers/test/FibersTest.cpp index 9a828ad4..d1b60cee 100644 --- a/folly/fibers/test/FibersTest.cpp +++ b/folly/fibers/test/FibersTest.cpp @@ -678,12 +678,11 @@ TEST(FiberManager, collectNThrow) { manager.addTask([&]() { std::vector> funcs; for (size_t i = 0; i < 3; ++i) { - funcs.push_back([i, &pendingFibers]() { + funcs.push_back([i, &pendingFibers]() -> size_t { await([&pendingFibers](Promise promise) { pendingFibers.push_back(std::move(promise)); }); throw std::runtime_error("Runtime"); - return i * 2 + 1; }); } @@ -2029,9 +2028,8 @@ TEST(FiberManager, ABD_UserProvidedBatchDispatchThrowsTest) { // Testing that exception is set if user provided batch dispatch throws // dispatchFunc = [](std::vector&& inputs) -> std::vector { - auto results = userDispatchFunc(std::move(inputs)); + (void)userDispatchFunc(std::move(inputs)); throw std::runtime_error("Unexpected exception in user dispatch function"); - return results; }; auto atomicBatchDispatcher = createAtomicBatchDispatcher(std::move(dispatchFunc)); diff --git a/folly/futures/test/FutureTest.cpp b/folly/futures/test/FutureTest.cpp index 5dc02dd2..b2485a90 100644 --- a/folly/futures/test/FutureTest.cpp +++ b/folly/futures/test/FutureTest.cpp @@ -237,18 +237,16 @@ TEST(Future, onError) { // Returned value propagates { - auto f = makeFuture().then([] { + auto f = makeFuture().then([]() -> int { throw eggs; - return 0; }).onError([&](eggs_t& /* e */) { return 42; }); EXPECT_EQ(42, f.value()); } // Returned future propagates { - auto f = makeFuture().then([] { + auto f = makeFuture().then([]() -> int { throw eggs; - return 0; }).onError([&](eggs_t& /* e */) { return makeFuture(42); }); EXPECT_EQ(42, f.value()); } @@ -256,15 +254,15 @@ TEST(Future, onError) { // Throw in callback { auto f = makeFuture() - .then([] { throw eggs; return 0; }) - .onError([&] (eggs_t& e) { throw e; return -1; }); + .then([]() -> int { throw eggs; }) + .onError([&] (eggs_t& e) -> int { throw e; }); EXPECT_THROW(f.value(), eggs_t); } { auto f = makeFuture() - .then([] { throw eggs; return 0; }) - .onError([&] (eggs_t& e) { throw e; return makeFuture(-1); }); + .then([]() -> int { throw eggs; }) + .onError([&] (eggs_t& e) -> Future { throw e; }); EXPECT_THROW(f.value(), eggs_t); } @@ -283,14 +281,12 @@ TEST(Future, onError) { // exception_wrapper, return Future but throw { auto f = makeFuture() - .then([] { + .then([]() -> int { throw eggs; - return 0; }) - .onError([&](exception_wrapper /* e */) { + .onError([&](exception_wrapper /* e */) -> Future { flag(); throw eggs; - return makeFuture(-1); }); EXPECT_FLAG(); EXPECT_THROW(f.value(), eggs_t); @@ -299,9 +295,8 @@ TEST(Future, onError) { // exception_wrapper, return T { auto f = makeFuture() - .then([] { + .then([]() -> int { throw eggs; - return 0; }) .onError([&](exception_wrapper /* e */) { flag(); @@ -314,14 +309,12 @@ TEST(Future, onError) { // exception_wrapper, return T but throw { auto f = makeFuture() - .then([] { + .then([]() -> int { throw eggs; - return 0; }) - .onError([&](exception_wrapper /* e */) { + .onError([&](exception_wrapper /* e */) -> int { flag(); throw eggs; - return -1; }); EXPECT_FLAG(); EXPECT_THROW(f.value(), eggs_t); diff --git a/folly/io/async/AsyncTimeout.cpp b/folly/io/async/AsyncTimeout.cpp index aa2e2837..73641042 100644 --- a/folly/io/async/AsyncTimeout.cpp +++ b/folly/io/async/AsyncTimeout.cpp @@ -128,9 +128,7 @@ void AsyncTimeout::detachTimeoutManager() { // currently installed. if (isScheduled()) { // Programmer bug. Abort the program. - LOG(ERROR) << "detachEventBase() called on scheduled timeout; aborting"; - abort(); - return; + LOG(FATAL) << "detachEventBase() called on scheduled timeout; aborting"; } if (timeoutManager_) { diff --git a/folly/io/async/SSLContext.cpp b/folly/io/async/SSLContext.cpp index cd7ad6e4..43703f2f 100644 --- a/folly/io/async/SSLContext.cpp +++ b/folly/io/async/SSLContext.cpp @@ -169,12 +169,10 @@ void SSLContext::setServerECCurve(const std::string& curveName) { nid = OBJ_sn2nid(curveName.c_str()); if (nid == 0) { LOG(FATAL) << "Unknown curve name:" << curveName.c_str(); - return; } ecdh = EC_KEY_new_by_curve_name(nid); if (ecdh == nullptr) { LOG(FATAL) << "Unable to create curve:" << curveName.c_str(); - return; } SSL_CTX_set_tmp_ecdh(ctx_, ecdh); diff --git a/folly/test/TryTest.cpp b/folly/test/TryTest.cpp index e9b600a3..b00c195b 100644 --- a/folly/test/TryTest.cpp +++ b/folly/test/TryTest.cpp @@ -64,9 +64,8 @@ TEST(Try, makeTryWith) { } TEST(Try, makeTryWithThrow) { - auto func = []() { + auto func = []() -> std::unique_ptr { throw std::runtime_error("Runtime"); - return folly::make_unique(1); }; auto result = makeTryWith(func); @@ -85,7 +84,6 @@ TEST(Try, makeTryWithVoid) { TEST(Try, makeTryWithVoidThrow) { auto func = []() { throw std::runtime_error("Runtime"); - return; }; auto result = makeTryWith(func); -- 2.34.1