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

[via cfe-commits]

=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


DonatNagyE wrote:

The issue was really trivial to fix, I created a PR for it: 
https://github.com/llvm/llvm-project/pull/75216

Sorry for breaking check-clang, I _did_ ran the tests before pressing merge on 
the github GUI, but I forgot to consider that the GUI button _rebases_ the 
commit onto the current main tip before merging it, and while the testcase was 
passing locally, it started to fail when it was rebased onto a version that 
contained https://github.com/llvm/llvm-project/pull/74503 (which was also added 
by me).


https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nico Weber via cfe-commits]

=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


nico wrote:

This breaks check-clang everywhere, e.g.:

https://lab.llvm.org/buildbot/#/builders/139/builds/55404
http://45.33.8.238/linux/125647/step_7.txt

Please take a look and revert for now if it takes a while to fix. (Also, please 
run tests before committing.)

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


https://github.com/DonatNagyE closed 
https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via cfe-commits]

https://github.com/Xazax-hun approved this pull request.


https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

Sounds good!

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


DonatNagyE wrote:

The limitation is emphasized in the documentation, and I also added a `TODO` 
comment that mentions it in the source code.

I'll merge this change as it is now (because I don't want to leave it open for 
even more time), but @whisperity is interested in this "bitflag enum" case and 
he'll address these issues when he'll have time for it (in January?).

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

=?utf-8?q?Donát?= Nagy ,
=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


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?= 
Date: Fri, 22 Sep 2023 17:22:53 +0200
Subject: [PATCH 1/3] [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 `_, and
   `#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(3);  // OK
+   WidgetKind x = static_cast(99); // OK
+   WidgetKind d = static_cast(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;
 // 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;
 
 // 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<"EnumCa

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

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

I am a bit concerned about the bad user experience with bitwise enums, but I 
think that is probably OK since this is an optin check. That being said, I 
think:
* It would be easy to address this, there is already a clang tidy check for 
bitwise enums, the heuristics to recognize them could be reused from there and 
used to suppress bogus warnings.
* As long as this limitation is documented, I am happy to get this merged as is.

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


DonatNagyE wrote:

PR https://github.com/llvm/llvm-project/pull/74503 was merged, so I think all 
the review comments are addressed. @haoNoQ @Xazax-hun What do you think about 
merging this?

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


DonatNagyE wrote:

> > Before merging this PR, the diagnostics of the EnumCast checker should be 
> > updated to mention the name of the `enum` type in question. @gamesh411 
> > could you create a PR that implements this improvement (as we discussed)?
> 
> Yes, it should call out the enum, call out the value, and then _track_ the 
> value (with `trackExpressionValue()`) in order to explain where it's coming 
> from and why we think it's out of range.

These suggestions were mostly implemented in 
https://github.com/llvm/llvm-project/pull/68191/ , I created PR 
https://github.com/llvm/llvm-project/pull/74503 to implement the last missing 
part ("call out the value").

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Balazs Benics via cfe-commits]

=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


steakhal wrote:

> (Force pushed the branch because I wanted to rebase it onto a recent main and 
> fix the merge conflicts. Is there a better workflow than this?)

I think git also offers conflict resolution for `git merge origin/main`. It 
should put the resolution into the merge commit.
That being said, I usually just do the rebase/resolve approach and force-push; 
only after all conversations are resolved and I don't expect anyone reviewing 
at the time. But yea, a merge commit should be the way for GitHub AFAIK.

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

=?utf-8?q?Donát?= Nagy 
Message-ID:
In-Reply-To: 


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?= 
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 `_, and
   `#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(3);  // OK
+   WidgetKind x = static_cast(99); // OK
+   WidgetKind d = static_cast(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;
 // 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;
 
 // 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">,
+  HelpTe

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

[via cfe-commits]

DonatNagyE wrote:

(Force pushed the branch because I wanted to rebase it onto a recent main and 
fix the merge conflicts. Is there a better workflow than this?)

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

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?= 
Date: Fri, 22 Sep 2023 17:22:53 +0200
Subject: [PATCH] [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 828dd10e3d6db9..082dbf6ab3c2b7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1027,6 +1027,9 @@ Static Analyzer
   `#65888 `_, and
   `#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 f7b48e64e324f0..cc5eb31d664803 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(3);  // OK
+   WidgetKind x = static_cast(99); // OK
+   WidgetKind d = static_cast(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 1d224786372e82..e7774e5a9392d2 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;
 // 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;
 
 // 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

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

[via cfe-commits]

DonatNagyE wrote:

@Xazax-hun This checker doesn't accept the use of bitwise enums (like the one 
in your example), so projects that use bitwise enums should not enable this 
checker. (Perhaps this limitation should be mentioned in the docs? @gamesh411)

It would be possible to write a more complex checker that recognizes and 
accepts enums that are used as flags; but I'm not sure that it's a good 
investment of developer-hours and I think that the current simple&stupid 
checker is already useful for some projects. 

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gábor Horváth via cfe-commits]

Xazax-hun wrote:

How well does this check work with bitwise enums? 

People often write code like:
```
enum AnimalFlags
{
HasClaws   = 1,
CanFly = 2,
EatsFish   = 4,
Endangered = 8
};

AnimalFlags operator|(AnimalFlags a, AnimalFlags b)
{
return static_cast(static_cast(a) | static_cast(b));
}

auto flags = HasClaws | CanFly;
```

Would this warning emit false positives for these patterns? If yes, can/should 
we suppress them?

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via cfe-commits]

haoNoQ wrote:

> Before merging this PR, the diagnostics of the EnumCast checker should be 
> updated to mention the name of the `enum` type in question. @gamesh411 could 
> you create a PR that implements this improvement (as we discussed)?

Yes, it should call out the enum, call out the value, and then *track* the 
value (with `trackExpressionValue()`) in order to explain where it's coming 
from and why we think it's out of range.

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

DonatNagyE wrote:

Before merging this PR, the diagnostics of the EnumCast checker should be 
updated to mention the name of the `enum` type in question. @gamesh411 could 
you create a PR that implements this improvement (as we discussed)?

https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Endre Fülöp via cfe-commits]

https://github.com/gamesh411 approved this pull request.


https://github.com/llvm/llvm-project/pull/67157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Endre Fülöp via cfe-commits]

gamesh411 wrote:

| OpenSource Project name | New Reports | Reports Lost | Evaluation of reports |
|||||
| memcached_1.6.8 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_baseline&newcheck=memcached_1.6.8_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=memcached_1.6.8_baseline&newcheck=memcached_1.6.8_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | no reports |
| tmux_2.6 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_baseline&newcheck=tmux_2.6_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tmux_2.6_baseline&newcheck=tmux_2.6_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | no reports |
| curl_curl-7_66_0 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_baseline&newcheck=curl_curl-7_66_0_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_baseline&newcheck=curl_curl-7_66_0_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | 18 reports, all seem valid, even if some of them are just plain ugly macro 
expansion-wrapped madness |
| twin_v0.8.1 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_baseline&newcheck=twin_v0.8.1_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=twin_v0.8.1_baseline&newcheck=twin_v0.8.1_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | 2 reports, 1 is flag type usage, so this project would want NOT want to opt 
into this checker
| vim_v8.2.1920 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_baseline&newcheck=vim_v8.2.1920_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=vim_v8.2.1920_baseline&newcheck=vim_v8.2.1920_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | 29 reports, valid, but not really useful for this project |
| openssl_openssl-3.0.0 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_baseline&newcheck=openssl_openssl-3.0.0-alpha7_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=openssl_openssl-3.0.0-alpha7_baseline&newcheck=openssl_openssl-3.0.0-alpha7_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | no reports |
| sqlite_version-3.33.0 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_baseline&newcheck=sqlite_version-3.33.0_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=sqlite_version-3.33.0_baseline&newcheck=sqlite_version-3.33.0_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | no reports |
| ffmpeg_n4.3.1 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_baseline&newcheck=ffmpeg_n4.3.1_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=ffmpeg_n4.3.1_baseline&newcheck=ffmpeg_n4.3.1_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | 39 reports, not really useful or understandable |
| postgres_REL_13_0 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_baseline&newcheck=postgres_REL_13_0_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=postgres_REL_13_0_baseline&newcheck=postgres_REL_13_0_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | 16 reports, they valid from the coding style enforcing POV |
| tinyxml2_8.0.0 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tinyxml2_8.0.0_baseline&newcheck=tinyxml2_8.0.0_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=tinyxml2_8.0.0_baseline&newcheck=tinyxml2_8.0.0_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | no reports |
| libwebm_libwebm-1.0.0.27 | [New 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_baseline&newcheck=libwebm_libwebm-1.0.0.27_with_enum_cast&is-unique=on&diff-mode=New)
 | [Lost 
reports](https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_baseline&newcheck=libwebm_libwebm-1.0.0.27_with_enum_cast&is-unique=on&diff-mode=Resolved)
 | no reports |
| xerces_v3.2.3 | [New 
reports](https://codechecker-de

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

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang


Changes

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 `_)
 
+- 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(3);  // OK
+   WidgetKind x = static_cast(99); // OK
+   WidgetKind d = static_cast(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;
 // 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;
 
 // 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;
+
+} // end "optin.core"
+
 
//===--===//
 // Unix API checkers.
 
//===---

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

[via cfe-commits]

https://github.com/DonatNagyE created 
https://github.com/llvm/llvm-project/pull/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.

>From 9fd8def3d6ed3ca533efb63005d4748c8bd62687 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= 
Date: Fri, 22 Sep 2023 17:22:53 +0200
Subject: [PATCH] [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 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 `_)
 
+- 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(3);  // OK
+   WidgetKind x = static_cast(99); // OK
+   WidgetKind d = static_cast(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;
 // 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