[clang] [analyzer] Move alpha checker EnumCastOutOfRange to optin (PR #67157)

via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 22 08:47:43 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/67157.diff


6 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/docs/analyzer/checkers.rst (+22-17) 
- (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+13-4) 
- (modified) clang/test/Analysis/enum-cast-out-of-range.c (+1-1) 
- (modified) clang/test/Analysis/enum-cast-out-of-range.cpp (+12-1) 
- (modified) clang/www/analyzer/alpha_checks.html (-19) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 477a40630f11097..522272a81c088f1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -491,6 +491,9 @@ Static Analyzer
   Read the PR for the details.
   (`#66086 <https://github.com/llvm/llvm-project/pull/66086>`_)
 
+- 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 dbd6d7787823530..552f7676a328371 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++)
@@ -1853,23 +1875,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 65c1595eb6245dd..1c859c39cf44d91 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
@@ -433,6 +434,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.
 //===----------------------------------------------------------------------===//
@@ -763,10 +776,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 3282cba653d7125..328b8b28ca70003 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:   -verify %s
 
 enum En_t {
diff --git a/clang/test/Analysis/enum-cast-out-of-range.cpp b/clang/test/Analysis/enum-cast-out-of-range.cpp
index abc1431e5be140f..33790f726c5ff8a 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
 
 enum unscoped_unspecified_t {
@@ -215,3 +215,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 };
+
+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 the enum}}
+
+  ignore_unused(c, x, d);
+}
diff --git a/clang/www/analyzer/alpha_checks.html b/clang/www/analyzer/alpha_checks.html
index 181ce1b9de591b3..a00e330a4e99579 100644
--- a/clang/www/analyzer/alpha_checks.html
+++ b/clang/www/analyzer/alpha_checks.html
@@ -350,25 +350,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">

``````````

</details>


https://github.com/llvm/llvm-project/pull/67157


More information about the cfe-commits mailing list