Author: DonatNagyE Date: 2023-12-12T16:29:37+01:00 New Revision: c873f77e87a9ebd02f94d6b9d46e84d43e1ceb38
URL: https://github.com/llvm/llvm-project/commit/c873f77e87a9ebd02f94d6b9d46e84d43e1ceb38 DIFF: https://github.com/llvm/llvm-project/commit/c873f77e87a9ebd02f94d6b9d46e84d43e1ceb38.diff LOG: [analyzer] Move alpha checker EnumCastOutOfRange to optin (#67157) 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. Added: Modified: clang/docs/ReleaseNotes.rst clang/docs/analyzer/checkers.rst clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp clang/test/Analysis/enum-cast-out-of-range.c clang/test/Analysis/enum-cast-out-of-range.cpp clang/www/analyzer/alpha_checks.html Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b62293a33bb9f..066e4ac5b9e54 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -1073,6 +1073,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..81d40395067c9 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -535,6 +535,52 @@ 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 + } + +**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++) @@ -2113,23 +2159,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/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp index 14433d06c2d04..7c51673422a0a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp @@ -159,6 +159,9 @@ void EnumCastOutOfRangeChecker::checkPreStmt(const CastExpr *CE, // Every initialization an enum with a fixed underlying type but without any // enumerators would produce a warning if we were to continue at this point. // The most notable example is std::byte in the C++17 standard library. + // TODO: Create heuristics to bail out when the enum type is intended to be + // used to store combinations of flag values (to mitigate the limitation + // described in the docs). if (DeclValues.size() == 0) return; 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 09835d420672b..82086b7305787 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@+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"> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits