[clang-tools-extra] [clang-tidy] Move part of bugprone-unhandled-code-paths to a new check readability-trivial-switch (PR #198237)
Yanzuo Liu via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 28 07:29:01 PDT 2026
https://github.com/zwuis updated https://github.com/llvm/llvm-project/pull/198237
>From 0445e94537811e0943838124bf810e2f22c647d4 Mon Sep 17 00:00:00 2001
From: Yanzuo Liu <zwuis at outlook.com>
Date: Mon, 18 May 2026 14:15:05 +0800
Subject: [PATCH 1/2] Move part of bugprone-unhandled-code-paths to a new check
readability-trivial-switch
---
.../bugprone/UnhandledCodePathsCheck.cpp | 43 ++-------------
.../bugprone/UnhandledCodePathsCheck.h | 1 -
.../clang-tidy/readability/CMakeLists.txt | 1 +
.../readability/ReadabilityTidyModule.cpp | 3 ++
.../readability/TrivialSwitchCheck.cpp | 48 +++++++++++++++++
.../readability/TrivialSwitchCheck.h | 33 ++++++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 6 ++-
.../checks/bugprone/unhandled-code-paths.rst | 33 ------------
.../docs/clang-tidy/checks/list.rst | 1 +
.../checks/readability/trivial-switch.rst | 36 +++++++++++++
.../bugprone/unhandled-code-paths.cpp | 52 ++-----------------
.../checkers/readability/trivial-switch.cpp | 44 ++++++++++++++++
12 files changed, 178 insertions(+), 123 deletions(-)
create mode 100644 clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/trivial-switch.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/trivial-switch.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnhandledCodePathsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnhandledCodePathsCheck.cpp
index 1a7d907bac6ec..c02dcd50f10f3 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledCodePathsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledCodePathsCheck.cpp
@@ -98,39 +98,10 @@ void UnhandledCodePathsCheck::check(const MatchFinder::MatchResult &Result) {
bool SwitchHasDefault = false;
std::tie(SwitchCaseCount, SwitchHasDefault) = countCaseLabels(Switch);
- // Checks the sanity of 'switch' statements that actually do define
- // a default branch but might be degenerated by having no or only one case.
- if (SwitchHasDefault) {
- handleSwitchWithDefault(Switch, SwitchCaseCount);
+ if (SwitchHasDefault || SwitchCaseCount <= 1)
return;
- }
- // Checks all 'switch' statements that do not define a default label.
- // Here the heavy lifting happens.
- if (!SwitchHasDefault && SwitchCaseCount > 0) {
- handleSwitchWithoutDefault(Switch, SwitchCaseCount, Result);
- return;
- }
- // Warns for degenerated 'switch' statements that neither define a case nor
- // a default label.
- // FIXME: Evaluate, if emitting a fix-it to simplify that statement is
- // reasonable.
- if (!SwitchHasDefault && SwitchCaseCount == 0) {
- diag(Switch->getBeginLoc(),
- "switch statement without labels has no effect");
- return;
- }
- llvm_unreachable("matched a case, that was not explicitly handled");
-}
-void UnhandledCodePathsCheck::handleSwitchWithDefault(const SwitchStmt *Switch,
- std::size_t CaseCount) {
- assert(CaseCount > 0 && "Switch statement with supposedly one default "
- "branch did not contain any case labels");
- if (CaseCount == 1 || CaseCount == 2)
- diag(Switch->getBeginLoc(),
- CaseCount == 1
- ? "degenerated switch with default label only"
- : "switch could be better written as an if/else statement");
+ handleSwitchWithoutDefault(Switch, SwitchCaseCount, Result);
}
void UnhandledCodePathsCheck::handleSwitchWithoutDefault(
@@ -146,11 +117,7 @@ void UnhandledCodePathsCheck::handleSwitchWithoutDefault(
// and duplicating case labels is not allowed this number represents
// the number of codepaths. It can be directly compared to 'MaxPathsPossible'
// to see if some cases are missing.
- // CaseCount == 0 is caught in DegenerateSwitch. Necessary because the
- // matcher used for here does not match on degenerate 'switch'.
- assert(CaseCount > 0 && "Switch statement without any case found. This case "
- "should be excluded by the matcher and is handled "
- "separately.");
+ assert(CaseCount > 1 && "Switch statement with fewer than two cases found.");
const std::size_t MaxPathsPossible = [&]() {
if (const auto *GeneralCondition =
Result.Nodes.getNodeAs<DeclRefExpr>("non-enum-condition")) {
@@ -165,10 +132,8 @@ void UnhandledCodePathsCheck::handleSwitchWithoutDefault(
return static_cast<std::size_t>(0);
}();
- // FIXME: Transform the 'switch' into an 'if' for CaseCount == 1.
if (CaseCount < MaxPathsPossible)
diag(Switch->getBeginLoc(),
- CaseCount == 1 ? "switch with only one case; use an if statement"
- : "potential uncovered code path; add a default label");
+ "potential uncovered code path; add a default label");
}
} // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnhandledCodePathsCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnhandledCodePathsCheck.h
index 051f1fd66dd63..bb4dfdffb9840 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnhandledCodePathsCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnhandledCodePathsCheck.h
@@ -29,7 +29,6 @@ class UnhandledCodePathsCheck : public ClangTidyCheck {
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
private:
- void handleSwitchWithDefault(const SwitchStmt *Switch, std::size_t CaseCount);
void handleSwitchWithoutDefault(
const SwitchStmt *Switch, std::size_t CaseCount,
const ast_matchers::MatchFinder::MatchResult &Result);
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 0dfdbe69c880d..c5ee081c351ed 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -61,6 +61,7 @@ add_clang_library(clangTidyReadabilityModule STATIC
StringCompareCheck.cpp
SuspiciousCallArgumentCheck.cpp
TrailingCommaCheck.cpp
+ TrivialSwitchCheck.cpp
UniqueptrDeleteReleaseCheck.cpp
UppercaseLiteralSuffixCheck.cpp
UseAnyOfAllOfCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 86e4f4dcc1de1..3aca2ce7e066d 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -63,6 +63,7 @@
#include "StringCompareCheck.h"
#include "SuspiciousCallArgumentCheck.h"
#include "TrailingCommaCheck.h"
+#include "TrivialSwitchCheck.h"
#include "UniqueptrDeleteReleaseCheck.h"
#include "UppercaseLiteralSuffixCheck.h"
#include "UseAnyOfAllOfCheck.h"
@@ -186,6 +187,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-suspicious-call-argument");
CheckFactories.registerCheck<TrailingCommaCheck>(
"readability-trailing-comma");
+ CheckFactories.registerCheck<TrivialSwitchCheck>(
+ "readability-trivial-switch");
CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
"readability-uniqueptr-delete-release");
CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>(
diff --git a/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.cpp b/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.cpp
new file mode 100644
index 0000000000000..e26a30de4ead1
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.cpp
@@ -0,0 +1,48 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TrivialSwitchCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void TrivialSwitchCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(switchStmt().bind("switch"), this);
+}
+
+void TrivialSwitchCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Switch = Result.Nodes.getNodeAs<SwitchStmt>("switch");
+ std::size_t CaseCount = 0;
+ bool HasDefault = false;
+
+ for (const SwitchCase *CurrentCase = Switch->getSwitchCaseList(); CurrentCase;
+ CurrentCase = CurrentCase->getNextSwitchCase()) {
+ ++CaseCount;
+ if (isa<DefaultStmt>(CurrentCase))
+ HasDefault = true;
+ }
+
+ // FIXME: Try to add fixit for each case.
+ switch (SourceLocation Loc = Switch->getBeginLoc(); CaseCount) {
+ case 0:
+ diag(Loc, "switch statement without labels has no effect");
+ return;
+ case 1:
+ if (HasDefault)
+ diag(Loc, "switch with default label only");
+ else
+ diag(Loc, "switch with only one case; use an if statement");
+ return;
+ case 2:
+ if (HasDefault)
+ diag(Loc, "switch could be better written as an if-else statement");
+ }
+}
+
+} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.h b/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.h
new file mode 100644
index 0000000000000..9eff86475932d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.h
@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_TRIVIALSWITCHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_TRIVIALSWITCHCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Finds trivial switch statements that can be written more clearly.
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/readability/trivial-switch.html
+class TrivialSwitchCheck : public ClangTidyCheck {
+public:
+ TrivialSwitchCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_TRIVIALSWITCHCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 89fb1684bba7c..5d561b62a2412 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -83,8 +83,10 @@ Potentially Breaking Changes
<clang-tidy/checks/cppcoreguidelines/pro-type-member-init>`
``hicpp-move-const-arg`` :doc:`performance-move-const-arg
<clang-tidy/checks/performance/move-const-arg>`
- ``hicpp-multiway-paths-covered`` :doc:`bugprone-unhandled-code-paths
- <clang-tidy/checks/bugprone/unhandled-code-paths>`
+ ``hicpp-multiway-paths-covered`` | :doc:`bugprone-unhandled-code-paths
+ <clang-tidy/checks/bugprone/unhandled-code-paths>`
+ | :doc:`readability-trivial-switch
+ <clang-tidy/checks/readability/trivial-switch>`
``hicpp-named-parameter`` :doc:`readability-named-parameter
<clang-tidy/checks/readability/named-parameter>`
``hicpp-new-delete-operators`` :doc:`misc-new-delete-overloads
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unhandled-code-paths.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unhandled-code-paths.rst
index 0e3082afe38f7..01208244989c4 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unhandled-code-paths.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unhandled-code-paths.rst
@@ -4,8 +4,6 @@ bugprone-unhandled-code-paths
=============================
This check discovers situations where code paths are not fully-covered.
-It furthermore suggests using ``if`` instead of ``switch`` if the code
-will be more clear.
``if-else if`` chains that miss a final ``else`` branch might lead to
unexpected program execution and be the result of a logical error.
@@ -55,37 +53,6 @@ possible code paths.
// Other cases missing
}
-
-Every ``switch`` statement should have at least two ``case`` labels
-other than a `default` label.
-Otherwise, the ``switch`` could be better expressed with an ``if`` statement.
-Degenerated ``switch`` statements without any labels are caught as well.
-
-.. code-block:: c++
-
- // Degenerated switch that could be better written as `if`
- int i = 42;
- switch(i) {
- case 1: // do something here
- default: // do something else here
- }
-
- // Should rather be the following:
- if (i == 1) {
- // do something here
- }
- else {
- // do something here
- }
-
-
-.. code-block:: c++
-
- // A completely degenerated switch will be diagnosed.
- int i = 42;
- switch(i) {}
-
-
Options
-------
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 937b80da9b601..0029b83aace91 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -432,6 +432,7 @@ Clang-Tidy Checks
:doc:`readability-string-compare <readability/string-compare>`, "Yes"
:doc:`readability-suspicious-call-argument <readability/suspicious-call-argument>`,
:doc:`readability-trailing-comma <readability/trailing-comma>`, "Yes"
+ :doc:`readability-trivial-switch <readability/trivial-switch>`,
:doc:`readability-uniqueptr-delete-release <readability/uniqueptr-delete-release>`, "Yes"
:doc:`readability-uppercase-literal-suffix <readability/uppercase-literal-suffix>`, "Yes"
:doc:`readability-use-anyofallof <readability/use-anyofallof>`,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/trivial-switch.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/trivial-switch.rst
new file mode 100644
index 0000000000000..efcee2f0c7caf
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/trivial-switch.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - readability-trivial-switch
+
+readability-trivial-switch
+==========================
+
+Finds trivial ``switch`` statements that can be written more clearly.
+
+Every ``switch`` statement should have at least two ``case`` labels other than a ``default`` label.
+Otherwise, the ``switch`` can be better expressed with an ``if`` statement.
+``switch`` statements without any labels are diagnosed as well.
+
+.. code-block:: c++
+
+ int i = 42;
+
+ switch (i) {
+ case 1:
+ doSomething();
+ break;
+ default:
+ doSomethingElse();
+ break;
+ }
+
+ // The switch can be written more clearly as:
+ if (i == 1) {
+ doSomething();
+ } else {
+ doSomethingElse();
+ }
+
+.. code-block:: c++
+
+ // The switch without any labels will be diagnosed.
+ int i = 42;
+ switch (i) {}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-code-paths.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-code-paths.cpp
index 0c3b459ef06fd..b5206db852964 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-code-paths.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unhandled-code-paths.cpp
@@ -12,11 +12,6 @@ struct Bitfields {
int return_integer() { return 42; }
void bad_switch(int i) {
- switch (i) {
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
- case 0:
- break;
- }
// No default in this switch
switch (i) {
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
@@ -28,11 +23,6 @@ void bad_switch(int i) {
break;
}
- // degenerate, maybe even warning
- switch (i) {
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch statement without labels has no effect
- }
-
switch (int j = return_integer()) {
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
case 0:
@@ -41,22 +31,6 @@ void bad_switch(int i) {
break;
}
- // Degenerated, only default case.
- switch (i) {
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
- default:
- break;
- }
-
- // Degenerated, only one case label and default case -> Better as if-stmt.
- switch (i) {
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as an if/else statement
- case 0:
- break;
- default:
- break;
- }
-
unsigned long long BigNumber = 0;
switch (BigNumber) {
// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
@@ -90,7 +64,10 @@ void bad_switch(int i) {
case 1:
break;
}
- // All paths explicitly covered.
+}
+
+void unproblematic_switch(unsigned char c) {
+ Bitfields Bf;
switch (Bf.UInt) {
case 0:
case 1:
@@ -102,13 +79,6 @@ void bad_switch(int i) {
case 7:
break;
}
- // SInt has 1 bit size, so this is somewhat degenerated.
- switch (Bf.SInt) {
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
- case 0:
- break;
- }
- // All paths explicitly covered.
switch (Bf.SInt) {
case 0:
case 1:
@@ -116,18 +86,6 @@ void bad_switch(int i) {
}
bool Flag = false;
- switch (Flag) {
- // CHECK-MESSAGES:[[@LINE-1]]:3: warning: switch with only one case; use an if statement
- case true:
- break;
- }
-
- switch (Flag) {
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
- default:
- break;
- }
-
// This `switch` will create a frontend warning from '-Wswitch-bool' but is
// ok for this check.
switch (Flag) {
@@ -136,9 +94,7 @@ void bad_switch(int i) {
case false:
break;
}
-}
-void unproblematic_switch(unsigned char c) {
//
switch (c) {
case 0:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/trivial-switch.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/trivial-switch.cpp
new file mode 100644
index 0000000000000..bc693df6599c4
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/trivial-switch.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy -std=c++98-or-later %s readability-trivial-switch %t
+
+void bad(int I) {
+ switch (I) {
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch statement without labels has no effect [readability-trivial-switch]
+ }
+
+ switch (I) {
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with default label only [readability-trivial-switch]
+ default:
+ break;
+ }
+
+ switch (I) {
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement [readability-trivial-switch]
+ case 0:
+ break;
+ }
+
+ switch (I) {
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as an if-else statement [readability-trivial-switch]
+ case 0:
+ break;
+ default:
+ break;
+ }
+}
+
+void good(int I) {
+ switch (I) {
+ case 0:
+ case 1:
+ break;
+ }
+
+ switch (I) {
+ case 0:
+ break;
+ case 1:
+ break;
+ default:
+ break;
+ }
+}
>From ce59f0bf5d7d12bc96dc6f06ebf8a863a0d237bd Mon Sep 17 00:00:00 2001
From: Yanzuo Liu <zwuis at outlook.com>
Date: Mon, 18 May 2026 14:44:20 +0800
Subject: [PATCH 2/2] Make CI happy
---
.../clang-tidy/readability/TrivialSwitchCheck.cpp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.cpp b/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.cpp
index e26a30de4ead1..5b5fe4c48724b 100644
--- a/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/TrivialSwitchCheck.cpp
@@ -28,8 +28,8 @@ void TrivialSwitchCheck::check(const MatchFinder::MatchResult &Result) {
HasDefault = true;
}
- // FIXME: Try to add fixit for each case.
- switch (SourceLocation Loc = Switch->getBeginLoc(); CaseCount) {
+ // FIXME: Try to add fix-it for each case.
+ switch (const SourceLocation Loc = Switch->getBeginLoc(); CaseCount) {
case 0:
diag(Loc, "switch statement without labels has no effect");
return;
@@ -42,6 +42,9 @@ void TrivialSwitchCheck::check(const MatchFinder::MatchResult &Result) {
case 2:
if (HasDefault)
diag(Loc, "switch could be better written as an if-else statement");
+ [[fallthrough]];
+ default:
+ break;
}
}
More information about the cfe-commits
mailing list