Skip to content

Commit c873f77

Browse files
authored
[analyzer] Move alpha checker EnumCastOutOfRange to optin (llvm#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.
1 parent 6d46337 commit c873f77

File tree

7 files changed

+78
-42
lines changed

7 files changed

+78
-42
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,9 @@ Static Analyzer
10731073
`#65888 <https://github.com/llvm/llvm-project/pull/65888>`_, and
10741074
`#65887 <https://github.com/llvm/llvm-project/pull/65887>`_
10751075

1076+
- Move checker ``alpha.cplusplus.EnumCastOutOfRange`` out of the ``alpha``
1077+
package to ``optin.core.EnumCastOutOfRange``.
1078+
10761079
.. _release-notes-sanitizers:
10771080

10781081
Sanitizers

clang/docs/analyzer/checkers.rst

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,52 @@ optin
535535
536536
Checkers for portability, performance or coding style specific rules.
537537
538+
.. _optin-core-EnumCastOutOfRange:
539+
540+
optin.core.EnumCastOutOfRange (C, C++)
541+
""""""""""""""""""""""""""""""""""""""
542+
Check for integer to enumeration casts that would produce a value with no
543+
corresponding enumerator. This is not necessarily undefined behavior, but can
544+
lead to nasty surprises, so projects may decide to use a coding standard that
545+
disallows these "unusual" conversions.
546+
547+
Note that no warnings are produced when the enum type (e.g. `std::byte`) has no
548+
enumerators at all.
549+
550+
.. code-block:: cpp
551+
552+
enum WidgetKind { A=1, B, C, X=99 };
553+
554+
void foo() {
555+
WidgetKind c = static_cast<WidgetKind>(3); // OK
556+
WidgetKind x = static_cast<WidgetKind>(99); // OK
557+
WidgetKind d = static_cast<WidgetKind>(4); // warn
558+
}
559+
560+
**Limitations**
561+
562+
This checker does not accept the coding pattern where an enum type is used to
563+
store combinations of flag values:
564+
565+
.. code-block:: cpp
566+
567+
enum AnimalFlags
568+
{
569+
HasClaws = 1,
570+
CanFly = 2,
571+
EatsFish = 4,
572+
Endangered = 8
573+
};
574+
575+
AnimalFlags operator|(AnimalFlags a, AnimalFlags b)
576+
{
577+
return static_cast<AnimalFlags>(static_cast<int>(a) | static_cast<int>(b));
578+
}
579+
580+
auto flags = HasClaws | CanFly;
581+
582+
Projects that use this pattern should not enable this optin checker.
583+
538584
.. _optin-cplusplus-UninitializedObject:
539585
540586
optin.cplusplus.UninitializedObject (C++)
@@ -2113,23 +2159,6 @@ Reports destructions of polymorphic objects with a non-virtual destructor in the
21132159
// destructor
21142160
}
21152161
2116-
.. _alpha-cplusplus-EnumCastOutOfRange:
2117-
2118-
alpha.cplusplus.EnumCastOutOfRange (C++)
2119-
""""""""""""""""""""""""""""""""""""""""
2120-
Check for integer to enumeration casts that could result in undefined values.
2121-
2122-
.. code-block:: cpp
2123-
2124-
enum TestEnum {
2125-
A = 0
2126-
};
2127-
2128-
void foo() {
2129-
TestEnum t = static_cast(-1);
2130-
// warn: the value provided to the cast expression is not in
2131-
// the valid range of values for the enum
2132-
21332162
.. _alpha-cplusplus-InvalidatedIterator:
21342163
21352164
alpha.cplusplus.InvalidatedIterator (C++)

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ def CoreAlpha : Package<"core">, ParentPackage<Alpha>;
3636
// Note: OptIn is *not* intended for checkers that are too noisy to be on by
3737
// default. Such checkers belong in the alpha package.
3838
def OptIn : Package<"optin">;
39+
def CoreOptIn : Package<"core">, ParentPackage<OptIn>;
3940

4041
// In the Portability package reside checkers for finding code that relies on
4142
// implementation-defined behavior. Such checks are wanted for cross-platform
@@ -439,6 +440,18 @@ def UndefinedNewArraySizeChecker : Checker<"NewArraySize">,
439440

440441
} // end "core.uninitialized"
441442

443+
//===----------------------------------------------------------------------===//
444+
// Optin checkers for core language features
445+
//===----------------------------------------------------------------------===//
446+
447+
let ParentPackage = CoreOptIn in {
448+
449+
def EnumCastOutOfRangeChecker : Checker<"EnumCastOutOfRange">,
450+
HelpText<"Check integer to enumeration casts for out of range values">,
451+
Documentation<HasDocumentation>;
452+
453+
} // end "optin.core"
454+
442455
//===----------------------------------------------------------------------===//
443456
// Unix API checkers.
444457
//===----------------------------------------------------------------------===//
@@ -774,10 +787,6 @@ def DeleteWithNonVirtualDtorChecker : Checker<"DeleteWithNonVirtualDtor">,
774787
"destructor in their base class">,
775788
Documentation<HasDocumentation>;
776789

777-
def EnumCastOutOfRangeChecker : Checker<"EnumCastOutOfRange">,
778-
HelpText<"Check integer to enumeration casts for out of range values">,
779-
Documentation<HasDocumentation>;
780-
781790
def IteratorModeling : Checker<"IteratorModeling">,
782791
HelpText<"Models iterators of C++ containers">,
783792
Dependencies<[ContainerModeling]>,

clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,9 @@ void EnumCastOutOfRangeChecker::checkPreStmt(const CastExpr *CE,
159159
// Every initialization an enum with a fixed underlying type but without any
160160
// enumerators would produce a warning if we were to continue at this point.
161161
// The most notable example is std::byte in the C++17 standard library.
162+
// TODO: Create heuristics to bail out when the enum type is intended to be
163+
// used to store combinations of flag values (to mitigate the limitation
164+
// described in the docs).
162165
if (DeclValues.size() == 0)
163166
return;
164167

clang/test/Analysis/enum-cast-out-of-range.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// RUN: %clang_analyze_cc1 \
2-
// RUN: -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
2+
// RUN: -analyzer-checker=core,optin.core.EnumCastOutOfRange \
33
// RUN: -analyzer-output text \
44
// RUN: -verify %s
55

clang/test/Analysis/enum-cast-out-of-range.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// RUN: %clang_analyze_cc1 \
2-
// RUN: -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
2+
// RUN: -analyzer-checker=core,optin.core.EnumCastOutOfRange \
33
// RUN: -std=c++11 -verify %s
44

55
// expected-note@+1 + {{enum declared here}}
@@ -219,3 +219,14 @@ void empty_enums_init_with_zero_should_not_warn() {
219219

220220
ignore_unused(eu, ef, efu);
221221
}
222+
223+
//Test the example from checkers.rst:
224+
enum WidgetKind { A=1, B, C, X=99 }; // expected-note {{enum declared here}}
225+
226+
void foo() {
227+
WidgetKind c = static_cast<WidgetKind>(3); // OK
228+
WidgetKind x = static_cast<WidgetKind>(99); // OK
229+
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'}}
230+
231+
ignore_unused(c, x, d);
232+
}

clang/www/analyzer/alpha_checks.html

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -370,25 +370,6 @@ <h3 id="cplusplus_alpha_checkers">C++ Alpha Checkers</h3>
370370
}
371371
</pre></div></div></td></tr>
372372

373-
<tr><td><a id="alpha.cplusplus.EnumCastOutOfRange"><div class="namedescr expandable"><span class="name">
374-
alpha.cplusplus.EnumCastOutOfRange</span><span class="lang">
375-
(C++)</span><div class="descr">
376-
Check for integer to enumeration casts that could result in undefined values.
377-
</div></div></a></td>
378-
<td><div class="exampleContainer expandable">
379-
<div class="example"><pre>
380-
enum TestEnum {
381-
A = 0
382-
};
383-
384-
void foo() {
385-
TestEnum t = static_cast<TestEnum>(-1);
386-
// warn: the value provided to the cast expression is not in
387-
the valid range of values for the enum
388-
}
389-
</pre></div></div></td></tr>
390-
391-
392373
<tr><td><a id="alpha.cplusplus.InvalidatedIterator"><div class="namedescr expandable"><span class="name">
393374
alpha.cplusplus.InvalidatedIterator</span><span class="lang">
394375
(C++)</span><div class="descr">

0 commit comments

Comments
 (0)