Skip to content

Commit

Permalink
Fix a nuisance compiler warning from clang (#12144)
Browse files Browse the repository at this point in the history
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 <atomic>
void do_something();
void test(int v) {
    switch (v) {
        case 1:
            if constexpr (std::atomic<long>::is_always_lock_free) {
                return;
            } else {
                do_something();
                [[fallthrough]];
            }
        case 2:
            return;
    }
}
```

Pull Request resolved: #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
  • Loading branch information
pdillinger committed Dec 14, 2023
1 parent 0efb576 commit fcd07e0
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions cache/clock_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit fcd07e0

Please sign in to comment.