Skip to content

Commit

Permalink
[clang] Expose unreachable fallthrough annotation warning
Browse files Browse the repository at this point in the history
The Linux kernel has a macro called IS_ENABLED(), which evaluates to a
constant 1 or 0 based on Kconfig selections, allowing C code to be
unconditionally enabled or disabled at build time. For example:

int foo(struct *a, int b) {
    switch (b) {
    case 1:
        if (a->flag || !IS_ENABLED(CONFIG_64BIT))
            return 1;
        __attribute__((fallthrough));
    case 2:
        return 2;
    default:
        return 3;
    }
}

There is an unreachable warning about the fallthrough annotation in the
first case because !IS_ENABLED(CONFIG_64BIT) can be evaluated to 1,
which looks like

        return 1;
        __attribute__((fallthrough));

to clang.

This type of warning is pointless for the Linux kernel because it does
this trick all over the place due to the sheer number of configuration
options that it has.

Add -Wunreachable-code-fallthrough, enabled under -Wunreachable-code, so
that projects that want to warn on unreachable code get this warning but
projects that do not care about unreachable code can still use
-Wimplicit-fallthrough without having to make changes to their code
base.

Fixes PR51094.

Reviewed By: aaron.ballman, nickdesaulniers

Differential Revision: https://reviews.llvm.org/D107933
  • Loading branch information
nathanchance committed Aug 17, 2021
1 parent e2c97d4 commit 9ed4a94
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 7 deletions.
4 changes: 3 additions & 1 deletion clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -821,8 +821,10 @@ def ReservedIdentifier : DiagGroup<"reserved-identifier",
// under separate flags.
//
def UnreachableCodeLoopIncrement : DiagGroup<"unreachable-code-loop-increment">;
def UnreachableCodeFallthrough : DiagGroup<"unreachable-code-fallthrough">;
def UnreachableCode : DiagGroup<"unreachable-code",
[UnreachableCodeLoopIncrement]>;
[UnreachableCodeLoopIncrement,
UnreachableCodeFallthrough]>;
def UnreachableCodeBreak : DiagGroup<"unreachable-code-break">;
def UnreachableCodeReturn : DiagGroup<"unreachable-code-return">;
def UnreachableCodeAggressive : DiagGroup<"unreachable-code-aggressive",
Expand Down
6 changes: 3 additions & 3 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,9 @@ def warn_unreachable_return : Warning<
def warn_unreachable_loop_increment : Warning<
"loop will run at most once (loop increment never executed)">,
InGroup<UnreachableCodeLoopIncrement>, DefaultIgnore;
def warn_unreachable_fallthrough_attr : Warning<
"fallthrough annotation in unreachable code">,
InGroup<UnreachableCodeFallthrough>, DefaultIgnore;
def note_unreachable_silence : Note<
"silence by adding parentheses to mark code as explicitly dead">;

Expand Down Expand Up @@ -9578,9 +9581,6 @@ def err_fallthrough_attr_outside_switch : Error<
"fallthrough annotation is outside switch statement">;
def err_fallthrough_attr_invalid_placement : Error<
"fallthrough annotation does not directly precede switch label">;
def warn_fallthrough_attr_unreachable : Warning<
"fallthrough annotation in unreachable code">,
InGroup<ImplicitFallthrough>, DefaultIgnore;

def warn_unreachable_default : Warning<
"default label in switch which covers all enumeration values">,
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/AnalysisBasedWarnings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,7 @@ namespace {
// unreachable in all instantiations of the template.
if (!IsTemplateInstantiation)
S.Diag(AS->getBeginLoc(),
diag::warn_fallthrough_attr_unreachable);
diag::warn_unreachable_fallthrough_attr);
markFallthroughVisited(AS);
++AnnotatedCnt;
break;
Expand Down
2 changes: 1 addition & 1 deletion clang/test/SemaCXX/P30636.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough -Wunreachable-code-fallthrough %s
// expected-no-diagnostics

template<bool param>
Expand Down
22 changes: 21 additions & 1 deletion clang/test/SemaCXX/switch-implicit-fallthrough.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough %s
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wimplicit-fallthrough -Wunreachable-code-fallthrough %s


int fallthrough(int n) {
Expand Down Expand Up @@ -193,6 +193,26 @@ int fallthrough_position(int n) {
;
}

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wunreachable-code-fallthrough"
switch (n) {
n += 300;
[[clang::fallthrough]]; // no warning here
case 221:
return 1;
[[clang::fallthrough]]; // no warning here
case 222:
return 2;
__attribute__((fallthrough)); // no warning here
case 223:
if (1)
return 3;
__attribute__((fallthrough)); // no warning here
case 224:
n += 400;
}
#pragma clang diagnostic pop

long p = static_cast<long>(n) * n;
switch (sizeof(p)) {
case 9:
Expand Down

0 comments on commit 9ed4a94

Please sign in to comment.