[clang] [analyzer] Move alpha checker EnumCastOutOfRange to optin (PR #67157)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 5 08:39:43 PST 2023
=?utf-8?q?Donát?= Nagy <donat.nagy at ericsson.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/67157 at github.com>
https://github.com/DonatNagyE updated https://github.com/llvm/llvm-project/pull/67157
>From 5c42d3e5286e041e22776fa496d884454640ffec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Fri, 22 Sep 2023 17:22:53 +0200
Subject: [PATCH 1/2] [analyzer] Move alpha checker EnumCastOutOfRange to optin
The checker EnumCastOutOfRange verifies the (helpful, but not
standard-mandated) design rule that integer to enum casts should not
produce values that don't have a corresponding enumerator. As it was
improved and cleaned up by recent changes, this commit renames it from
`alpha.cplusplus.EnumCastOutOfRange` to `optin.core.EnumCastOutOfRange`
to reflect that it's no longer alpha quality.
As this checker handles a basic language feature (which is also present
in plain C), I moved it to a "core" subpackage within "optin".
In addition to the renaming, this commit cleans up the documentation
in `checkers.rst` and adds the new example code to a test file to ensure
that it's indeed producing the behavior claimend in the documentation.
---
clang/docs/ReleaseNotes.rst | 3 ++
clang/docs/analyzer/checkers.rst | 39 +++++++++++--------
.../clang/StaticAnalyzer/Checkers/Checkers.td | 17 ++++++--
clang/test/Analysis/enum-cast-out-of-range.c | 2 +-
.../test/Analysis/enum-cast-out-of-range.cpp | 13 ++++++-
clang/www/analyzer/alpha_checks.html | 19 ---------
6 files changed, 51 insertions(+), 42 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 828dd10e3d6db..082dbf6ab3c2b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1027,6 +1027,9 @@ Static Analyzer
`#65888 <https://github.com/llvm/llvm-project/pull/65888>`_, and
`#65887 <https://github.com/llvm/llvm-project/pull/65887>`_
+- Move checker ``alpha.cplusplus.EnumCastOutOfRange`` out of the ``alpha``
+ package to ``optin.core.EnumCastOutOfRange``.
+
.. _release-notes-sanitizers:
Sanitizers
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index f7b48e64e324f..cc5eb31d66480 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -535,6 +535,28 @@ optin
Checkers for portability, performance or coding style specific rules.
+.. _optin-core-EnumCastOutOfRange:
+
+optin.core.EnumCastOutOfRange (C, C++)
+""""""""""""""""""""""""""""""""""""""
+Check for integer to enumeration casts that would produce a value with no
+corresponding enumerator. This is not necessarily undefined behavior, but can
+lead to nasty surprises, so projects may decide to use a coding standard that
+disallows these "unusual" conversions.
+
+Note that no warnings are produced when the enum type (e.g. `std::byte`) has no
+enumerators at all.
+
+.. code-block:: cpp
+
+ enum WidgetKind { A=1, B, C, X=99 };
+
+ void foo() {
+ WidgetKind c = static_cast<WidgetKind>(3); // OK
+ WidgetKind x = static_cast<WidgetKind>(99); // OK
+ WidgetKind d = static_cast<WidgetKind>(4); // warn
+ }
+
.. _optin-cplusplus-UninitializedObject:
optin.cplusplus.UninitializedObject (C++)
@@ -2113,23 +2135,6 @@ Reports destructions of polymorphic objects with a non-virtual destructor in the
// destructor
}
-.. _alpha-cplusplus-EnumCastOutOfRange:
-
-alpha.cplusplus.EnumCastOutOfRange (C++)
-""""""""""""""""""""""""""""""""""""""""
-Check for integer to enumeration casts that could result in undefined values.
-
-.. code-block:: cpp
-
- enum TestEnum {
- A = 0
- };
-
- void foo() {
- TestEnum t = static_cast(-1);
- // warn: the value provided to the cast expression is not in
- // the valid range of values for the enum
-
.. _alpha-cplusplus-InvalidatedIterator:
alpha.cplusplus.InvalidatedIterator (C++)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 1d224786372e8..e7774e5a9392d 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -36,6 +36,7 @@ def CoreAlpha : Package<"core">, ParentPackage<Alpha>;
// Note: OptIn is *not* intended for checkers that are too noisy to be on by
// default. Such checkers belong in the alpha package.
def OptIn : Package<"optin">;
+def CoreOptIn : Package<"core">, ParentPackage<OptIn>;
// In the Portability package reside checkers for finding code that relies on
// implementation-defined behavior. Such checks are wanted for cross-platform
@@ -439,6 +440,18 @@ def UndefinedNewArraySizeChecker : Checker<"NewArraySize">,
} // end "core.uninitialized"
+//===----------------------------------------------------------------------===//
+// Optin checkers for core language features
+//===----------------------------------------------------------------------===//
+
+let ParentPackage = CoreOptIn in {
+
+def EnumCastOutOfRangeChecker : Checker<"EnumCastOutOfRange">,
+ HelpText<"Check integer to enumeration casts for out of range values">,
+ Documentation<HasDocumentation>;
+
+} // end "optin.core"
+
//===----------------------------------------------------------------------===//
// Unix API checkers.
//===----------------------------------------------------------------------===//
@@ -774,10 +787,6 @@ def DeleteWithNonVirtualDtorChecker : Checker<"DeleteWithNonVirtualDtor">,
"destructor in their base class">,
Documentation<HasDocumentation>;
-def EnumCastOutOfRangeChecker : Checker<"EnumCastOutOfRange">,
- HelpText<"Check integer to enumeration casts for out of range values">,
- Documentation<HasDocumentation>;
-
def IteratorModeling : Checker<"IteratorModeling">,
HelpText<"Models iterators of C++ containers">,
Dependencies<[ContainerModeling]>,
diff --git a/clang/test/Analysis/enum-cast-out-of-range.c b/clang/test/Analysis/enum-cast-out-of-range.c
index 4e5c9bb9ffdec..a6eef92f418d1 100644
--- a/clang/test/Analysis/enum-cast-out-of-range.c
+++ b/clang/test/Analysis/enum-cast-out-of-range.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
+// RUN: -analyzer-checker=core,optin.core.EnumCastOutOfRange \
// RUN: -analyzer-output text \
// RUN: -verify %s
diff --git a/clang/test/Analysis/enum-cast-out-of-range.cpp b/clang/test/Analysis/enum-cast-out-of-range.cpp
index 0eb740664ecdc..84fc23ab4d857 100644
--- a/clang/test/Analysis/enum-cast-out-of-range.cpp
+++ b/clang/test/Analysis/enum-cast-out-of-range.cpp
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 \
-// RUN: -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
+// RUN: -analyzer-checker=core,optin.core.EnumCastOutOfRange \
// RUN: -std=c++11 -verify %s
// expected-note at +1 + {{enum declared here}}
@@ -219,3 +219,14 @@ void empty_enums_init_with_zero_should_not_warn() {
ignore_unused(eu, ef, efu);
}
+
+//Test the example from checkers.rst:
+enum WidgetKind { A=1, B, C, X=99 }; // expected-note {{enum declared here}}
+
+void foo() {
+ WidgetKind c = static_cast<WidgetKind>(3); // OK
+ WidgetKind x = static_cast<WidgetKind>(99); // OK
+ WidgetKind d = static_cast<WidgetKind>(4); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'WidgetKind'}}
+
+ ignore_unused(c, x, d);
+}
diff --git a/clang/www/analyzer/alpha_checks.html b/clang/www/analyzer/alpha_checks.html
index cff0284777bc7..11ef7d405dd4c 100644
--- a/clang/www/analyzer/alpha_checks.html
+++ b/clang/www/analyzer/alpha_checks.html
@@ -370,25 +370,6 @@ <h3 id="cplusplus_alpha_checkers">C++ Alpha Checkers</h3>
}
</pre></div></div></td></tr>
-<tr><td><a id="alpha.cplusplus.EnumCastOutOfRange"><div class="namedescr expandable"><span class="name">
-alpha.cplusplus.EnumCastOutOfRange</span><span class="lang">
-(C++)</span><div class="descr">
- Check for integer to enumeration casts that could result in undefined values.
-</div></div></a></td>
- <td><div class="exampleContainer expandable">
- <div class="example"><pre>
-enum TestEnum {
- A = 0
-};
-
-void foo() {
- TestEnum t = static_cast<TestEnum>(-1);
- // warn: the value provided to the cast expression is not in
- the valid range of values for the enum
-}
-</pre></div></div></td></tr>
-
-
<tr><td><a id="alpha.cplusplus.InvalidatedIterator"><div class="namedescr expandable"><span class="name">
alpha.cplusplus.InvalidatedIterator</span><span class="lang">
(C++)</span><div class="descr">
>From e293c3ef9c8d05a616aa4087f60bceb716b8c685 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Tue, 5 Dec 2023 17:38:11 +0100
Subject: [PATCH 2/2] Describe the limitation that the checker doesn't accept
bitwise enums
---
clang/docs/analyzer/checkers.rst | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index cc5eb31d66480..81d40395067c9 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -557,6 +557,30 @@ enumerators at all.
WidgetKind d = static_cast<WidgetKind>(4); // warn
}
+**Limitations**
+
+This checker does not accept the coding pattern where an enum type is used to
+store combinations of flag values:
+
+.. code-block:: cpp
+
+ enum AnimalFlags
+ {
+ HasClaws = 1,
+ CanFly = 2,
+ EatsFish = 4,
+ Endangered = 8
+ };
+
+ AnimalFlags operator|(AnimalFlags a, AnimalFlags b)
+ {
+ return static_cast<AnimalFlags>(static_cast<int>(a) | static_cast<int>(b));
+ }
+
+ auto flags = HasClaws | CanFly;
+
+Projects that use this pattern should not enable this optin checker.
+
.. _optin-cplusplus-UninitializedObject:
optin.cplusplus.UninitializedObject (C++)
More information about the cfe-commits
mailing list