Skip to content

Commit 42179bb

Browse files
committed
[clang-tidy] Add check for possibly incomplete switch statements
While clang warns about a possibly incomplete switch statement when switching over an enum variable and failing to cover all enum values (either explicitly or with a default case), no such warning is emitted if a plain integer variable is used as switch variable. Add a clang-tidy check to diagnose these scenarios. No fixit hint is provided since there are multiple possible solutions. Differential Revision: https://reviews.llvm.org/D4784
1 parent 7ce4e93 commit 42179bb

File tree

8 files changed

+232
-0
lines changed

8 files changed

+232
-0
lines changed

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
#include "SuspiciousSemicolonCheck.h"
6666
#include "SuspiciousStringCompareCheck.h"
6767
#include "SwappedArgumentsCheck.h"
68+
#include "SwitchMissingDefaultCaseCheck.h"
6869
#include "TerminatingContinueCheck.h"
6970
#include "ThrowKeywordMissingCheck.h"
7071
#include "TooSmallLoopVariableCheck.h"
@@ -116,6 +117,8 @@ class BugproneModule : public ClangTidyModule {
116117
"bugprone-implicit-widening-of-multiplication-result");
117118
CheckFactories.registerCheck<InaccurateEraseCheck>(
118119
"bugprone-inaccurate-erase");
120+
CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>(
121+
"bugprone-switch-missing-default-case");
119122
CheckFactories.registerCheck<IncorrectRoundingsCheck>(
120123
"bugprone-incorrect-roundings");
121124
CheckFactories.registerCheck<InfiniteLoopCheck>("bugprone-infinite-loop");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ add_clang_library(clangTidyBugproneModule
2121
ForwardingReferenceOverloadCheck.cpp
2222
ImplicitWideningOfMultiplicationResultCheck.cpp
2323
InaccurateEraseCheck.cpp
24+
SwitchMissingDefaultCaseCheck.cpp
2425
IncorrectRoundingsCheck.cpp
2526
InfiniteLoopCheck.cpp
2627
IntegerDivisionCheck.cpp
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
//===--- SwitchMissingDefaultCaseCheck.cpp - clang-tidy -------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "SwitchMissingDefaultCaseCheck.h"
10+
#include "clang/AST/ASTContext.h"
11+
12+
using namespace clang::ast_matchers;
13+
14+
namespace clang::tidy::bugprone {
15+
16+
namespace {
17+
18+
AST_MATCHER(SwitchStmt, hasDefaultCase) {
19+
const SwitchCase *Case = Node.getSwitchCaseList();
20+
while (Case) {
21+
if (DefaultStmt::classof(Case))
22+
return true;
23+
24+
Case = Case->getNextSwitchCase();
25+
}
26+
return false;
27+
}
28+
} // namespace
29+
30+
void SwitchMissingDefaultCaseCheck::registerMatchers(MatchFinder *Finder) {
31+
Finder->addMatcher(
32+
switchStmt(hasCondition(expr(unless(isInstantiationDependent()),
33+
hasType(qualType(hasCanonicalType(
34+
unless(hasDeclaration(enumDecl()))))))),
35+
unless(hasDefaultCase()))
36+
.bind("switch"),
37+
this);
38+
}
39+
40+
void SwitchMissingDefaultCaseCheck::check(
41+
const ast_matchers::MatchFinder::MatchResult &Result) {
42+
const auto *Switch = Result.Nodes.getNodeAs<SwitchStmt>("switch");
43+
44+
diag(Switch->getSwitchLoc(), "switching on non-enum value without "
45+
"default case may not cover all cases");
46+
}
47+
} // namespace clang::tidy::bugprone
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//===--- SwitchMissingDefaultCaseCheck.h - clang-tidy -----------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SWITCHMISSINGDEFAULTCASECHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SWITCHMISSINGDEFAULTCASECHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
#include "clang/ASTMatchers/ASTMatchFinder.h"
14+
#include "clang/ASTMatchers/ASTMatchers.h"
15+
16+
namespace clang::tidy::bugprone {
17+
18+
/// Ensures that switch statements without default cases are flagged, focuses
19+
/// only on covering cases with non-enums where the compiler may not issue
20+
/// warnings.
21+
///
22+
/// For the user-facing documentation see:
23+
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/switch-missing-default-case.html
24+
class SwitchMissingDefaultCaseCheck : public ClangTidyCheck {
25+
public:
26+
SwitchMissingDefaultCaseCheck(StringRef Name, ClangTidyContext *Context)
27+
: ClangTidyCheck(Name, Context) {}
28+
29+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
30+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
31+
std::optional<TraversalKind> getCheckTraversalKind() const override {
32+
return TK_IgnoreUnlessSpelledInSource;
33+
}
34+
};
35+
36+
} // namespace clang::tidy::bugprone
37+
38+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SWITCHMISSINGDEFAULTCASECHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,12 @@ New checks
121121
Detect implicit and explicit casts of ``enum`` type into ``bool`` where ``enum`` type
122122
doesn't have a zero-value enumerator.
123123

124+
- New :doc:`bugprone-switch-missing-default-case
125+
<clang-tidy/checks/bugprone/switch-missing-default-case>` check.
126+
127+
Ensures that switch statements without default cases are flagged, focuses only
128+
on covering cases with non-enums where the compiler may not issue warnings.
129+
124130
- New :doc:`bugprone-unique-ptr-array-mismatch
125131
<clang-tidy/checks/bugprone/unique-ptr-array-mismatch>` check.
126132

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
.. title:: clang-tidy - bugprone-switch-missing-default-case
2+
3+
bugprone-switch-missing-default-case
4+
====================================
5+
6+
Ensures that switch statements without default cases are flagged, focuses only
7+
on covering cases with non-enums where the compiler may not issue warnings.
8+
9+
Switch statements without a default case can lead to unexpected
10+
behavior and incomplete handling of all possible cases. When a switch statement
11+
lacks a default case, if a value is encountered that does not match any of the
12+
specified cases, the program will continue execution without any defined
13+
behavior or handling.
14+
15+
This check helps identify switch statements that are missing a default case,
16+
allowing developers to ensure that all possible cases are handled properly.
17+
Adding a default case allows for graceful handling of unexpected or unmatched
18+
values, reducing the risk of program errors and unexpected behavior.
19+
20+
Example:
21+
22+
.. code-block:: c++
23+
24+
// Example 1:
25+
// warning: switching on non-enum value without default case may not cover all cases
26+
switch (i) {
27+
case 0:
28+
break;
29+
}
30+
31+
// Example 2:
32+
enum E { eE1 };
33+
E e = eE1;
34+
switch (e) { // no-warning
35+
case eE1:
36+
break;
37+
}
38+
39+
// Example 3:
40+
int i = 0;
41+
switch (i) { // no-warning
42+
case 0:
43+
break;
44+
default:
45+
break;
46+
}
47+
48+
.. note::
49+
Enum types are already covered by compiler warnings (comes under -Wswitch)
50+
when a switch statement does not handle all enum values. This check focuses
51+
on non-enum types where the compiler warnings may not be present.
52+
53+
.. seealso::
54+
The `CppCoreGuideline ES.79 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-default>`_
55+
provide guidelines on switch statements, including the recommendation to
56+
always provide a default case.

clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ Clang-Tidy Checks
9292
`bugprone-forwarding-reference-overload <bugprone/forwarding-reference-overload.html>`_,
9393
`bugprone-implicit-widening-of-multiplication-result <bugprone/implicit-widening-of-multiplication-result.html>`_, "Yes"
9494
`bugprone-inaccurate-erase <bugprone/inaccurate-erase.html>`_, "Yes"
95+
`bugprone-switch-missing-default-case <bugprone/switch-missing-default-case.html>`_,
9596
`bugprone-incorrect-roundings <bugprone/incorrect-roundings.html>`_,
9697
`bugprone-infinite-loop <bugprone/infinite-loop.html>`_,
9798
`bugprone-integer-division <bugprone/integer-division.html>`_,
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// RUN: %check_clang_tidy %s bugprone-switch-missing-default-case %t -- -- -fno-delayed-template-parsing
2+
3+
typedef int MyInt;
4+
enum EnumType { eE2 };
5+
typedef EnumType MyEnum;
6+
7+
void positive() {
8+
int I1 = 0;
9+
// CHECK-MESSAGES: [[@LINE+1]]:3: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
10+
switch (I1) {
11+
case 0:
12+
break;
13+
}
14+
15+
MyInt I2 = 0;
16+
// CHECK-MESSAGES: [[@LINE+1]]:3: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
17+
switch (I2) {
18+
case 0:
19+
break;
20+
}
21+
22+
int getValue(void);
23+
// CHECK-MESSAGES: [[@LINE+1]]:3: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
24+
switch (getValue()) {
25+
case 0:
26+
break;
27+
}
28+
}
29+
30+
void negative() {
31+
enum E { eE1 };
32+
E E1 = eE1;
33+
switch (E1) { // no-warning
34+
case eE1:
35+
break;
36+
}
37+
38+
MyEnum E2 = eE2;
39+
switch (E2) { // no-warning
40+
case eE2:
41+
break;
42+
}
43+
44+
int I1 = 0;
45+
switch (I1) { // no-warning
46+
case 0:
47+
break;
48+
default:
49+
break;
50+
}
51+
52+
MyInt I2 = 0;
53+
switch (I2) { // no-warning
54+
case 0:
55+
break;
56+
default:
57+
break;
58+
}
59+
60+
int getValue(void);
61+
switch (getValue()) { // no-warning
62+
case 0:
63+
break;
64+
default:
65+
break;
66+
}
67+
}
68+
69+
template<typename T>
70+
void testTemplate(T Value) {
71+
switch (Value) {
72+
case 0:
73+
break;
74+
}
75+
}
76+
77+
void exampleUsage() {
78+
testTemplate(5);
79+
testTemplate(EnumType::eE2);
80+
}

0 commit comments

Comments
 (0)