From c74531b1d291985d3c1c6d074a771d81cab48658 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Wed, 13 Dec 2023 15:58:46 -0800 Subject: [PATCH] Fix a nuisance compiler warning from clang (#12144) Summary: Example: ``` cache/clock_cache.cc:56:7: error: fallthrough annotation in unreachable code [-Werror,-Wimplicit-fallthrough] FALLTHROUGH_INTENDED; ^ ./port/lang.h:10:30: note: expanded from macro 'FALLTHROUGH_INTENDED' ^ ``` In clang < 14, this is annoyingly generated from -Wimplicit-fallthrough, but was changed to -Wunreachable-code-fallthrough (implied by -Wunreachable-code) in clang 14. See https://reviews.llvm.org/D107933 for how this nuisance pattern generated false positives similar to ours in the Linux kernel. Just to underscore the ridiculousness of this warning, here an error is reported on the annotation, not the call to do_something(), depending on the constexpr value (https://godbolt.org/z/EvxqdPTdr): ``` #include void do_something(); void test(int v) { switch (v) { case 1: if constexpr (std::atomic::is_always_lock_free) { return; } else { do_something(); [[fallthrough]]; } case 2: return; } } ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/12144 Test Plan: Added the warning to our Makefile for USE_CLANG, which reproduced the warning-as-error as shown above, but is now fixed. Reviewed By: jaykorean Differential Revision: D52139615 Pulled By: pdillinger fbshipit-source-id: ba967ae700c0916d1a478bc465cf917633e337d9 --- Makefile | 2 +- cache/clock_cache.cc | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 50dddc97606..d9ebbb72b17 100644 --- a/Makefile +++ b/Makefile @@ -539,7 +539,7 @@ endif ifdef USE_CLANG # Used by some teams in Facebook - WARNING_FLAGS += -Wshift-sign-overflow -Wambiguous-reversed-operator + WARNING_FLAGS += -Wshift-sign-overflow -Wambiguous-reversed-operator -Wimplicit-fallthrough endif ifeq ($(PLATFORM), OS_OPENBSD) diff --git a/cache/clock_cache.cc b/cache/clock_cache.cc index fd330d90d83..e37c03fb502 100644 --- a/cache/clock_cache.cc +++ b/cache/clock_cache.cc @@ -51,14 +51,15 @@ inline uint64_t GetInitialCountdown(Cache::Priority priority) { switch (priority) { case Cache::Priority::HIGH: return ClockHandle::kHighCountdown; - default: - assert(false); - FALLTHROUGH_INTENDED; case Cache::Priority::LOW: return ClockHandle::kLowCountdown; case Cache::Priority::BOTTOM: return ClockHandle::kBottomCountdown; } + // Switch should have been exhaustive. + assert(false); + // For release build, fall back on something reasonable. + return ClockHandle::kLowCountdown; } inline void MarkEmpty(ClockHandle& h) {