[PATCH] D65912: [clang-tidy] Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

> ... then I have a script that runs clang-tidy over all the compilation units 
> in a compilation database

Can you share this script please? :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65912/new/

https://reviews.llvm.org/D65912



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65912: [clang-tidy] Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

> My point regarding statistics is that the check needs to pull its own weight 
> -- if it doesn't find many true positives, it's not of much value to a broad 
> community, or if it has a lot of false positives, we may need to tweak the 
> check before releasing it to the public, etc. So definitely do the 
> implementation work, but part of that work should be testing it over large 
> code bases and reporting back the results.

Okay. Do we have any infrastructure for doing such testing? Or I should do it 
manually: prepare some large codebases, run over them clang-tidy with the check 
and parse the result?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65912/new/

https://reviews.llvm.org/D65912



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65912: [clang-tidy] Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

@aaron.ballman Sure. I agree with you that epsilon should be configurable. I 
think we can collect some statistics later. Now I am going to work on 
implementation and tests. Later, of course, will be good to run the check on 
some codebases. I will be happy to hear that we already have some 
infrastructure for doing stuff like this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65912/new/

https://reviews.llvm.org/D65912



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65912: [clang-tidy] Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

> One thing I would be interested in knowing is how often the check behaves 
> when run over some large, real-world code bases. Does it catch any true 
> positives? Does it have false positives?

I have no such statistics for this check. But I have statistics for identical 
check from PVS-Studio - https://www.viva64.com/en/examples/v624/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65912/new/

https://reviews.llvm.org/D65912



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65912: [clang-tidy] Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

The main reason why I've created this differential - asking to you about 
usefulness of this check for clang-tidy. I understand that there are a some 
TODO and formatting issues - it's ok for now.

As far I understand your remarks - you are not against this check so I can 
continue my work on this path. Great.

Let's discuss about some open questions:

1. Should we support C language and  header file? I think - yes, it's 
quite easy to implement.
2. Should we support converting to Boost.Constants? I don't think so.
3. In header file instead of defining math constants directly should we use 
them from  as much as possible?




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65912/new/

https://reviews.llvm.org/D65912



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65912: Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK updated this revision to Diff 214043.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65912/new/

https://reviews.llvm.org/D65912

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp
  clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
  clang-tools-extra/test/clang-tidy/misc-math-constants.cpp

Index: clang-tools-extra/test/clang-tidy/misc-math-constants.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-math-constants.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s misc-math-constants %t
+
+// FIXME: Add something that triggers the check here.
+void f();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [misc-math-constants]
+
+// FIXME: Verify the applied fix.
+//   * Make the CHECK patterns specific enough and try to make verified lines
+// unique to avoid incorrect matches.
+//   * Use {{}} for regular expressions.
+// CHECK-FIXES: {{^}}void awesome_f();{{$}}
+
+// FIXME: Add something that doesn't trigger the check here.
+void foo()
+{
+double pi = 3.14;
+double p_2 = 1.57;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - misc-math-constants
+
+misc-math-constants
+===
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -276,6 +276,7 @@
llvm-prefer-isa-or-dyn-cast-in-conditionals
llvm-twine-local
misc-definitions-in-headers
+   misc-math-constants
misc-misplaced-const
misc-new-delete-overloads
misc-non-copyable-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -85,6 +85,11 @@
 
 The improvements are...
 
+- New :doc:`misc-math-constants
+  ` check.
+
+  FIXME: add release notes.
+
 Improvements to clang-include-fixer
 ---
 
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "DefinitionsInHeadersCheck.h"
+#include "MathConstantsCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
 #include "NonCopyableObjects.h"
@@ -32,6 +33,8 @@
   void addCheckFactories(ClangTidyCheckFactories ) override {
 CheckFactories.registerCheck(
 "misc-definitions-in-headers");
+CheckFactories.registerCheck(
+"misc-math-constants");
 CheckFactories.registerCheck("misc-misplaced-const");
 CheckFactories.registerCheck(
 "misc-new-delete-overloads");
Index: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h
@@ -0,0 +1,59 @@
+//===--- MathConstantsCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MATHCONSTANTSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MATHCONSTANTSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+#include "../utils/IncludeInserter.h"
+
+#include "llvm/ADT/Optional.h"
+
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-math-constants.html
+class MathConstantsCheck : public ClangTidyCheck {
+public:
+  MathConstantsCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void registerPPCallbacks(const SourceManager , 

[PATCH] D65912: Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK created this revision.
ZaMaZaN4iK added reviewers: JonasToth, Szelethus, aaron.ballman, lebedev.ri.
ZaMaZaN4iK added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Hello.

I found quite interesting and useful FMPOV check for Clang Tidy - description 
, examples 
. Whole sense is in using standard 
(from cmath or math.h or math headers) math constants instead of written from 
scratch.

This patch is in status WIP. The main question is - do we want to see such 
check in Clang Tidy and should I continue work on it?

Thank you.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D65912

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp
  clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
  clang-tools-extra/test/clang-tidy/misc-math-constants.cpp

Index: clang-tools-extra/test/clang-tidy/misc-math-constants.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-math-constants.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s misc-math-constants %t
+
+// FIXME: Add something that triggers the check here.
+void f();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [misc-math-constants]
+
+// FIXME: Verify the applied fix.
+//   * Make the CHECK patterns specific enough and try to make verified lines
+// unique to avoid incorrect matches.
+//   * Use {{}} for regular expressions.
+// CHECK-FIXES: {{^}}void awesome_f();{{$}}
+
+// FIXME: Add something that doesn't trigger the check here.
+void foo()
+{
+double pi = 3.14;
+double p_2 = 1.57;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - misc-math-constants
+
+misc-math-constants
+===
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -4,7 +4,6 @@
 =
 
 .. toctree::
-
abseil-duration-addition
abseil-duration-comparison
abseil-duration-conversion-cast
@@ -103,87 +102,87 @@
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) 
-   clang-analyzer-core.CallAndMessage
-   clang-analyzer-core.DivideZero
+   clang-analyzer-core.CallAndMessage (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.DivideZero (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.DynamicTypePropagation
-   clang-analyzer-core.NonNullParamChecker
-   clang-analyzer-core.NullDereference
-   clang-analyzer-core.StackAddressEscape
-   clang-analyzer-core.UndefinedBinaryOperatorResult
-   clang-analyzer-core.VLASize
-   clang-analyzer-core.uninitialized.ArraySubscript
-   clang-analyzer-core.uninitialized.Assign
-   clang-analyzer-core.uninitialized.Branch
+   clang-analyzer-core.NonNullParamChecker (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.NullDereference (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.StackAddressEscape (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.UndefinedBinaryOperatorResult (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.VLASize (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.uninitialized.ArraySubscript (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.uninitialized.Assign (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.uninitialized.Branch (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.uninitialized.CapturedBlockVariable
-   clang-analyzer-core.uninitialized.UndefReturn
+   clang-analyzer-core.uninitialized.UndefReturn (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-cplusplus.InnerPointer
-   clang-analyzer-cplusplus.Move
-   clang-analyzer-cplusplus.NewDelete
-   clang-analyzer-cplusplus.NewDeleteLeaks
-   clang-analyzer-deadcode.DeadStores
-   clang-analyzer-nullability.NullPassedToNonnull

[PATCH] D61475: Update an information about ReSharper C++ and clang-tidy custom binary integration

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a subscriber: Szelethus.
ZaMaZaN4iK added a comment.

@JonasToth @NoQ @Szelethus can anyone merge it?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61475/new/

https://reviews.llvm.org/D61475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61475: Update an information about ReSharper C++ and clang-tidy custom binary integration

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

Can anyone merge it? I have no commit access


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61475/new/

https://reviews.llvm.org/D61475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61475: Update an information about ReSharper C++ and clang-tidy custom binary integration

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK created this revision.
ZaMaZaN4iK added reviewers: NoQ, JonasToth.
ZaMaZaN4iK added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

According to the release notes from JetBrains 
(https://www.jetbrains.com/resharper-cpp/whatsnew/#v2019-1-clang-tidy) now it's 
possible to set custom clang-tidy binary


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D61475

Files:
  clang-tools-extra/docs/clang-tidy/Integrations.rst


Index: clang-tools-extra/docs/clang-tidy/Integrations.rst
===
--- clang-tools-extra/docs/clang-tidy/Integrations.rst
+++ clang-tools-extra/docs/clang-tidy/Integrations.rst
@@ -34,7 +34,7 @@
 
+--++-+--+-+--+
 |Qt Creator IDE| \+\|  
 \+\   |   \-\| \-\ 
|   \+\|
 
+--++-+--+-+--+
-|ReSharper C++ for Visual Studio   | \+\|  
 \+\   |   \-\| \+\ 
|   \-\|
+|ReSharper C++ for Visual Studio   | \+\|  
 \+\   |   \-\| \+\ 
|   \+\|
 
+--++-+--+-+--+
 |Syntastic for Vim | \+\|  
 \-\   |   \-\| \-\ 
|   \+\|
 
+--++-+--+-+--+


Index: clang-tools-extra/docs/clang-tidy/Integrations.rst
===
--- clang-tools-extra/docs/clang-tidy/Integrations.rst
+++ clang-tools-extra/docs/clang-tidy/Integrations.rst
@@ -34,7 +34,7 @@
 +--++-+--+-+--+
 |Qt Creator IDE| \+\|   \+\   |   \-\| \-\ |   \+\|
 +--++-+--+-+--+
-|ReSharper C++ for Visual Studio   | \+\|   \+\   |   \-\| \+\ |   \-\|
+|ReSharper C++ for Visual Studio   | \+\|   \+\   |   \-\| \+\ |   \+\|
 +--++-+--+-+--+
 |Syntastic for Vim | \+\|   \-\   |   \-\| \-\ |   \+\|
 +--++-+--+-+--+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK updated this revision to Diff 172516.
ZaMaZaN4iK added a comment.

Fix using `auto` in case where it leads to worse readability


https://reviews.llvm.org/D33672

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  test/Analysis/enum-cast-out-of-range.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -348,6 +348,24 @@
 }
 
 
+
+alpha.cplusplus.EnumCastOutOfRange
+(C++)
+  Check for integer to enumeration casts that could result in undefined values.
+
+  
+
+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
Index: test/Analysis/enum-cast-out-of-range.cpp
===
--- /dev/null
+++ test/Analysis/enum-cast-out-of-range.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:   -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
+// RUN:   -std=c++11 -verify %s
+
+enum unscoped_unspecified_t {
+  unscoped_unspecified_0 = -4,
+  unscoped_unspecified_1,
+  unscoped_unspecified_2 = 1,
+  unscoped_unspecified_3,
+  unscoped_unspecified_4 = 4
+};
+
+enum unscoped_specified_t : int {
+  unscoped_specified_0 = -4,
+  unscoped_specified_1,
+  unscoped_specified_2 = 1,
+  unscoped_specified_3,
+  unscoped_specified_4 = 4
+};
+
+enum class scoped_unspecified_t {
+  scoped_unspecified_0 = -4,
+  scoped_unspecified_1,
+  scoped_unspecified_2 = 1,
+  scoped_unspecified_3,
+  scoped_unspecified_4 = 4
+};
+
+enum class scoped_specified_t : int {
+  scoped_specified_0 = -4,
+  scoped_specified_1,
+  scoped_specified_2 = 1,
+  scoped_specified_3,
+  scoped_specified_4 = 4
+};
+
+struct S {
+  unscoped_unspecified_t E : 5;
+};
+
+void unscopedUnspecified() {
+  unscoped_unspecified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_unspecified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_unspecified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_unspecified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_unspecified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_unspecified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+
+void unscopedSpecified() {
+  unscoped_specified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_specified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_specified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_specified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_specified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_specified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of 

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96
+  // Get the value of the expression to cast.
+  const auto ValueToCastOptional =
+  C.getSVal(CE->getSubExpr()).getAs();

lebedev.ri wrote:
> ZaMaZaN4iK wrote:
> > lebedev.ri wrote:
> > > ZaMaZaN4iK wrote:
> > > > lebedev.ri wrote:
> > > > > ZaMaZaN4iK wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > `const auto *`
> > > > > > Why do we need this change here? If I understand correctly, with 
> > > > > > `const auto*` we also need change initializer to 
> > > > > > `C.getSVal(CE->getSubExpr()).getAs().getPointer()`.
> > > > > >  But I don't understand why we need this.
> > > > > Is `ValueToCastOptional` a pointer, a reference, or just an actual 
> > > > > `DefinedOrUnknownSVal`? I can't tell.
> > > > > (sidenote: would be great to have a clang-tidy check for this.)
> > > > `ValueToCastOptional` is `llvm::Optional`
> > > See, all my guesses were wrong. That is why it should not be `auto` at 
> > > all.
> > I don't agree with you for this case. Honestly it's like a yet another 
> > holywar question. If we are talking only about this case - here you can see 
> > `getAs` part of the expression. this means clearly 
> > (at least for me) that we get something like `DefinedOrUnknownSVal`. What 
> > we get? I just press hotkey in my favourite IDE/text editor and see that 
> > `getAs` returns `llvm::Optional`. From my point of 
> > view it's clear enough here.
> > 
> > If we are talking more generally about question "When should we use `auto` 
> > at all? " - we can talk, but not here, I think :)
> https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
>  comes to mind.
> > What we get? I just press hotkey in my favourite IDE/text editor and see 
> > that getAs returns llvm::Optional
> Which hotkey do i need to press to see this here, in the phabricator?
> 
> This really shouldn't be `auto`, if you have to explain that in the 
> variable's name, justify it in review comments.
Ok, didn't know about such LLVM coding standard. Of course, with this 
information I will fix using `auto` here. Thank you.


https://reviews.llvm.org/D33672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96
+  // Get the value of the expression to cast.
+  const auto ValueToCastOptional =
+  C.getSVal(CE->getSubExpr()).getAs();

lebedev.ri wrote:
> ZaMaZaN4iK wrote:
> > lebedev.ri wrote:
> > > ZaMaZaN4iK wrote:
> > > > aaron.ballman wrote:
> > > > > `const auto *`
> > > > Why do we need this change here? If I understand correctly, with `const 
> > > > auto*` we also need change initializer to 
> > > > `C.getSVal(CE->getSubExpr()).getAs().getPointer()`.
> > > >  But I don't understand why we need this.
> > > Is `ValueToCastOptional` a pointer, a reference, or just an actual 
> > > `DefinedOrUnknownSVal`? I can't tell.
> > > (sidenote: would be great to have a clang-tidy check for this.)
> > `ValueToCastOptional` is `llvm::Optional`
> See, all my guesses were wrong. That is why it should not be `auto` at all.
I don't agree with you for this case. Honestly it's like a yet another holywar 
question. If we are talking only about this case - here you can see 
`getAs` part of the expression. this means clearly (at 
least for me) that we get something like `DefinedOrUnknownSVal`. What we get? I 
just press hotkey in my favourite IDE/text editor and see that `getAs` returns 
`llvm::Optional`. From my point of view it's clear enough 
here.

If we are talking more generally about question "When should we use `auto` at 
all? " - we can talk, but not here, I think :)


https://reviews.llvm.org/D33672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96
+  // Get the value of the expression to cast.
+  const auto ValueToCastOptional =
+  C.getSVal(CE->getSubExpr()).getAs();

lebedev.ri wrote:
> ZaMaZaN4iK wrote:
> > aaron.ballman wrote:
> > > `const auto *`
> > Why do we need this change here? If I understand correctly, with `const 
> > auto*` we also need change initializer to 
> > `C.getSVal(CE->getSubExpr()).getAs().getPointer()`. 
> > But I don't understand why we need this.
> Is `ValueToCastOptional` a pointer, a reference, or just an actual 
> `DefinedOrUnknownSVal`? I can't tell.
> (sidenote: would be great to have a clang-tidy check for this.)
`ValueToCastOptional` is `llvm::Optional`


https://reviews.llvm.org/D33672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK updated this revision to Diff 172515.
ZaMaZaN4iK added a comment.

1. Fix indentation in test file
2. Return back capitalization for the checker description


https://reviews.llvm.org/D33672

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  test/Analysis/enum-cast-out-of-range.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -348,6 +348,24 @@
 }
 
 
+
+alpha.cplusplus.EnumCastOutOfRange
+(C++)
+  Check for integer to enumeration casts that could result in undefined values.
+
+  
+
+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
Index: test/Analysis/enum-cast-out-of-range.cpp
===
--- /dev/null
+++ test/Analysis/enum-cast-out-of-range.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:   -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
+// RUN:   -std=c++11 -verify %s
+
+enum unscoped_unspecified_t {
+  unscoped_unspecified_0 = -4,
+  unscoped_unspecified_1,
+  unscoped_unspecified_2 = 1,
+  unscoped_unspecified_3,
+  unscoped_unspecified_4 = 4
+};
+
+enum unscoped_specified_t : int {
+  unscoped_specified_0 = -4,
+  unscoped_specified_1,
+  unscoped_specified_2 = 1,
+  unscoped_specified_3,
+  unscoped_specified_4 = 4
+};
+
+enum class scoped_unspecified_t {
+  scoped_unspecified_0 = -4,
+  scoped_unspecified_1,
+  scoped_unspecified_2 = 1,
+  scoped_unspecified_3,
+  scoped_unspecified_4 = 4
+};
+
+enum class scoped_specified_t : int {
+  scoped_specified_0 = -4,
+  scoped_specified_1,
+  scoped_specified_2 = 1,
+  scoped_specified_3,
+  scoped_specified_4 = 4
+};
+
+struct S {
+  unscoped_unspecified_t E : 5;
+};
+
+void unscopedUnspecified() {
+  unscoped_unspecified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_unspecified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_unspecified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_unspecified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_unspecified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_unspecified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+
+void unscopedSpecified() {
+  unscoped_specified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_specified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_specified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_specified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_specified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_specified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is 

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:96
+  // Get the value of the expression to cast.
+  const auto ValueToCastOptional =
+  C.getSVal(CE->getSubExpr()).getAs();

aaron.ballman wrote:
> `const auto *`
Why do we need this change here? If I understand correctly, with `const auto*` 
we also need change initializer to 
`C.getSVal(CE->getSubExpr()).getAs().getPointer()`. But I 
don't understand why we need this.


https://reviews.llvm.org/D33672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

Which other changes and/or approvals are required for merging into trunk?


https://reviews.llvm.org/D33672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53974: [clang-tidy] new checker: bugprone-too-small-loop-variable

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

Hmm, i thought Clang has some warning for this, but I was wrong... Did you 
think to implement this check as Clang warning?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.

In https://reviews.llvm.org/D33672#1279305, @NoQ wrote:

> Thanks! I like where this is going. Let's land the patch and continue 
> developing it incrementally in trunk.
>
> The next steps for this checker are, in my opinion:
>
> - Do the visitor thingy that i requested in inline-311373 
> . I think it's a necessary 
> thing to do, but don't jump into implementing it right away: i already have 
> some code for this that i want to share.
> - Play nicely with typedefs. For now i believe the checker ignores them 
> because you can't cast `TypedefType` to `EnumType`. Once this is done, it 
> will be worth it to include the name of the enum in the warning message.
> - Optimize the code using `assumeInclusiveRange`. Because `assume` is an 
> expensive operation, i'd like to avoid doing it O(n) times for contiguous 
> enums in which just 2 `assume`s are enough (or, even better, as single 
> `assumeInclusiveRange`).
> - See how this checker performs on real code, fix crashes and false positives 
> if any.


Thank you for the roadmap! Honestly I am not so familiar with Clang AST and 
Clang Static Analyzer details (all my experience is writing some simple 
checkers for clang-tidy and watching some videos about Clang/Clang Static 
Analyzers), so I even don't understand of your words :) So don't rely on quick 
fixes from my side. But I think I can write something useful after couple of 
tries (slowly, but I can). Thank you for the help!


https://reviews.llvm.org/D33672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added inline comments.



Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:119
+   const LangOptions ) {
+  std::size_t DeclCount = std::distance(DS->decl_begin(), DS->decl_end());
+  if (DeclCount < 2)

Mark `DeclCount` as const


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53654: [clang] Improve ctor initializer completions.

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:5135
+  };
+  auto AddDefaultCtorInit = [&](const char *TypedName,
+const char *TypeName,

Is it good idea to capture ALL by reference? Probably will be better to capture 
only required things by reference


Repository:
  rC Clang

https://reviews.llvm.org/D53654



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53814: Allow the analyzer to output to a SARIF file

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added inline comments.



Comment at: StaticAnalyzer/Core/CMakeLists.txt:43
   PlistDiagnostics.cpp
+  SarifDiagnostics.cpp
   ProgramState.cpp

Sort alphabetically



Comment at: StaticAnalyzer/Core/SarifDiagnostics.cpp:88
+  // URI encode the part.
+  for (char C : Component) {
+// RFC 3986 claims alpha, numeric, and this handful of

I don't know about const corectness policy in LLVM. But I prefer here const 
char C .



Comment at: StaticAnalyzer/Core/SarifDiagnostics.cpp:94
+// reserved character.
+if (llvm::isAlnum(C) ||
+StringRef::npos !=

Probably this piece of code will be better to write separately as function


https://reviews.llvm.org/D53814



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:36
+  const ProgramStateRef PS;
+  SValBuilder 
+

Szelethus wrote:
> You can acquire `SValBuilder` from `ProgramState`:
> `PS->getStateManager()->getSvalBuilder()`
Is there any difference? Is it critical to get `SValBuilder` from 
`CheckerContext' ?


https://reviews.llvm.org/D33672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK updated this revision to Diff 171506.
ZaMaZaN4iK marked 2 inline comments as done.
ZaMaZaN4iK added a comment.

Fix typedef -> using


https://reviews.llvm.org/D33672

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  test/Analysis/enum-cast-out-of-range.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -348,6 +348,24 @@
 }
 
 
+
+alpha.cplusplus.EnumCastOutOfRange
+(C++)
+  Check for integer to enumeration casts that could result in undefined values.
+
+  
+
+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
Index: test/Analysis/enum-cast-out-of-range.cpp
===
--- /dev/null
+++ test/Analysis/enum-cast-out-of-range.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:   -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
+// RUN:   -std=c++11 -verify %s
+
+enum unscoped_unspecified_t {
+  unscoped_unspecified_0 = -4,
+  unscoped_unspecified_1,
+  unscoped_unspecified_2 = 1,
+  unscoped_unspecified_3,
+  unscoped_unspecified_4 = 4
+};
+
+enum unscoped_specified_t : int {
+  unscoped_specified_0 = -4,
+  unscoped_specified_1,
+  unscoped_specified_2 = 1,
+  unscoped_specified_3,
+  unscoped_specified_4 = 4
+};
+
+enum class scoped_unspecified_t {
+  scoped_unspecified_0 = -4,
+  scoped_unspecified_1,
+  scoped_unspecified_2 = 1,
+  scoped_unspecified_3,
+  scoped_unspecified_4 = 4
+};
+
+enum class scoped_specified_t : int {
+  scoped_specified_0 = -4,
+  scoped_specified_1,
+  scoped_specified_2 = 1,
+  scoped_specified_3,
+  scoped_specified_4 = 4
+};
+
+struct S {
+unscoped_unspecified_t E : 5;
+};
+
+void unscopedUnspecified() {
+  unscoped_unspecified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_unspecified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_unspecified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_unspecified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_unspecified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_unspecified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+
+void unscopedSpecified() {
+  unscoped_specified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_specified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_specified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_specified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_specified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_specified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid 

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK updated this revision to Diff 171503.
ZaMaZaN4iK added a comment.

Add new test for enum bit field


https://reviews.llvm.org/D33672

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  test/Analysis/enum-cast-out-of-range.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -348,6 +348,24 @@
 }
 
 
+
+alpha.cplusplus.EnumCastOutOfRange
+(C++)
+  Check for integer to enumeration casts that could result in undefined values.
+
+  
+
+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
Index: test/Analysis/enum-cast-out-of-range.cpp
===
--- /dev/null
+++ test/Analysis/enum-cast-out-of-range.cpp
@@ -0,0 +1,192 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:   -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
+// RUN:   -std=c++11 -verify %s
+
+enum unscoped_unspecified_t {
+  unscoped_unspecified_0 = -4,
+  unscoped_unspecified_1,
+  unscoped_unspecified_2 = 1,
+  unscoped_unspecified_3,
+  unscoped_unspecified_4 = 4
+};
+
+enum unscoped_specified_t : int {
+  unscoped_specified_0 = -4,
+  unscoped_specified_1,
+  unscoped_specified_2 = 1,
+  unscoped_specified_3,
+  unscoped_specified_4 = 4
+};
+
+enum class scoped_unspecified_t {
+  scoped_unspecified_0 = -4,
+  scoped_unspecified_1,
+  scoped_unspecified_2 = 1,
+  scoped_unspecified_3,
+  scoped_unspecified_4 = 4
+};
+
+enum class scoped_specified_t : int {
+  scoped_specified_0 = -4,
+  scoped_specified_1,
+  scoped_specified_2 = 1,
+  scoped_specified_3,
+  scoped_specified_4 = 4
+};
+
+struct S {
+unscoped_unspecified_t E : 5;
+};
+
+void unscopedUnspecified() {
+  unscoped_unspecified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_unspecified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_unspecified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_unspecified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_unspecified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_unspecified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+
+void unscopedSpecified() {
+  unscoped_specified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_specified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_specified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_specified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_specified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_specified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK marked 3 inline comments as done.
ZaMaZaN4iK added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:19
+//   of casting an integer value that is out of range
+//===--===//
+

Szelethus wrote:
> ZaMaZaN4iK wrote:
> > aaron.ballman wrote:
> > > If this check is intended to conform to CERT's INT50-CPP rule, you should 
> > > put a link to the wiki entry for it here as well.
> > Which wiki entry do you mean?
> Maybe this one? 
> https://wiki.sei.cmu.edu/confluence/display/cplusplus/INT50-CPP.+Do+not+cast+to+an+out-of-range+enumeration+value
Added a note about CERT's coding standard recommendaation. Didn't add the 
reference to web page because web pages can become broken. I think standard and 
check names are quite good explanation for here


https://reviews.llvm.org/D33672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK updated this revision to Diff 171472.
ZaMaZaN4iK added a comment.

Add the reference to CERT page


https://reviews.llvm.org/D33672

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  test/Analysis/enum-cast-out-of-range.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -348,6 +348,24 @@
 }
 
 
+
+alpha.cplusplus.EnumCastOutOfRange
+(C++)
+  Check for integer to enumeration casts that could result in undefined values.
+
+  
+
+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
Index: test/Analysis/enum-cast-out-of-range.cpp
===
--- /dev/null
+++ test/Analysis/enum-cast-out-of-range.cpp
@@ -0,0 +1,182 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:   -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
+// RUN:   -std=c++11 -verify %s
+
+enum unscoped_unspecified_t {
+  unscoped_unspecified_0 = -4,
+  unscoped_unspecified_1,
+  unscoped_unspecified_2 = 1,
+  unscoped_unspecified_3,
+  unscoped_unspecified_4 = 4
+};
+
+enum unscoped_specified_t : int {
+  unscoped_specified_0 = -4,
+  unscoped_specified_1,
+  unscoped_specified_2 = 1,
+  unscoped_specified_3,
+  unscoped_specified_4 = 4
+};
+
+enum class scoped_unspecified_t {
+  scoped_unspecified_0 = -4,
+  scoped_unspecified_1,
+  scoped_unspecified_2 = 1,
+  scoped_unspecified_3,
+  scoped_unspecified_4 = 4
+};
+
+enum class scoped_specified_t : int {
+  scoped_specified_0 = -4,
+  scoped_specified_1,
+  scoped_specified_2 = 1,
+  scoped_specified_3,
+  scoped_specified_4 = 4
+};
+
+void unscopedUnspecified() {
+  unscoped_unspecified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_unspecified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_unspecified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_unspecified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_unspecified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_unspecified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+
+void unscopedSpecified() {
+  unscoped_specified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_specified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_specified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_specified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_specified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_specified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+
+void scopedUnspecified() {
+  scoped_unspecified_t 

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK updated this revision to Diff 171443.

https://reviews.llvm.org/D33672

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  test/Analysis/enum-cast-out-of-range.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -348,6 +348,24 @@
 }
 
 
+
+alpha.cplusplus.EnumCastOutOfRange
+(C++)
+  Check for integer to enumeration casts that could result in undefined values.
+
+  
+
+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
Index: test/Analysis/enum-cast-out-of-range.cpp
===
--- /dev/null
+++ test/Analysis/enum-cast-out-of-range.cpp
@@ -0,0 +1,182 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:   -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
+// RUN:   -std=c++11 -verify %s
+
+enum unscoped_unspecified_t {
+  unscoped_unspecified_0 = -4,
+  unscoped_unspecified_1,
+  unscoped_unspecified_2 = 1,
+  unscoped_unspecified_3,
+  unscoped_unspecified_4 = 4
+};
+
+enum unscoped_specified_t : int {
+  unscoped_specified_0 = -4,
+  unscoped_specified_1,
+  unscoped_specified_2 = 1,
+  unscoped_specified_3,
+  unscoped_specified_4 = 4
+};
+
+enum class scoped_unspecified_t {
+  scoped_unspecified_0 = -4,
+  scoped_unspecified_1,
+  scoped_unspecified_2 = 1,
+  scoped_unspecified_3,
+  scoped_unspecified_4 = 4
+};
+
+enum class scoped_specified_t : int {
+  scoped_specified_0 = -4,
+  scoped_specified_1,
+  scoped_specified_2 = 1,
+  scoped_specified_3,
+  scoped_specified_4 = 4
+};
+
+void unscopedUnspecified() {
+  unscoped_unspecified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_unspecified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_unspecified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_unspecified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_unspecified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_unspecified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+
+void unscopedSpecified() {
+  unscoped_specified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_specified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_specified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_specified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_specified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_specified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+
+void scopedUnspecified() {
+  scoped_unspecified_t InvalidBeforeRangeBegin = static_cast(-5); // 

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK updated this revision to Diff 171438.
ZaMaZaN4iK added a comment.

1. Changed std::transform to llvm::transform
2. Described the check in .html file
3. Fixed RUN command for the test file


https://reviews.llvm.org/D33672

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  test/Analysis/enum-cast-out-of-range.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -348,6 +348,24 @@
 }
 
 
+
+alpha.cplusplus.EnumCastOutOfRange
+(C++)
+  Check for integer to enumeration casts that could result in undefined values.
+
+  
+
+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
Index: test/Analysis/enum-cast-out-of-range.cpp
===
--- /dev/null
+++ test/Analysis/enum-cast-out-of-range.cpp
@@ -0,0 +1,182 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:   -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
+// RUN:   -std=c++11 -verify %s
+
+enum unscoped_unspecified_t {
+  unscoped_unspecified_0 = -4,
+  unscoped_unspecified_1,
+  unscoped_unspecified_2 = 1,
+  unscoped_unspecified_3,
+  unscoped_unspecified_4 = 4
+};
+
+enum unscoped_specified_t : int {
+  unscoped_specified_0 = -4,
+  unscoped_specified_1,
+  unscoped_specified_2 = 1,
+  unscoped_specified_3,
+  unscoped_specified_4 = 4
+};
+
+enum class scoped_unspecified_t {
+  scoped_unspecified_0 = -4,
+  scoped_unspecified_1,
+  scoped_unspecified_2 = 1,
+  scoped_unspecified_3,
+  scoped_unspecified_4 = 4
+};
+
+enum class scoped_specified_t : int {
+  scoped_specified_0 = -4,
+  scoped_specified_1,
+  scoped_specified_2 = 1,
+  scoped_specified_3,
+  scoped_specified_4 = 4
+};
+
+void unscopedUnspecified() {
+  unscoped_unspecified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_unspecified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_unspecified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_unspecified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_unspecified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_unspecified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_unspecified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+}
+
+void unscopedSpecified() {
+  unscoped_specified_t InvalidBeforeRangeBegin = static_cast(-5); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidNegativeValue1 = static_cast(-4); // OK.
+  unscoped_specified_t ValidNegativeValue2 = static_cast(-3); // OK.
+  unscoped_specified_t InvalidInsideRange1 = static_cast(-2); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange2 = static_cast(-1); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t InvalidInsideRange3 = static_cast(0); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue1 = static_cast(1); // OK.
+  unscoped_specified_t ValidPositiveValue2 = static_cast(2); // OK.
+  unscoped_specified_t InvalidInsideRange4 = static_cast(3); // expected-warning {{the value provided to the cast expression is not in the valid range of values for the enum}}
+  unscoped_specified_t ValidPositiveValue3 = static_cast(4); // OK.
+  unscoped_specified_t InvalidAfterRangeEnd = static_cast(5); // expected-warning {{the value provided to the cast expression is not in the valid 

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK commandeered this revision.
ZaMaZaN4iK added a reviewer: gamesh411.
ZaMaZaN4iK added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:19
+//   of casting an integer value that is out of range
+//===--===//
+

aaron.ballman wrote:
> If this check is intended to conform to CERT's INT50-CPP rule, you should put 
> a link to the wiki entry for it here as well.
Which wiki entry do you mean?


https://reviews.llvm.org/D33672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46027: [clang-tidy] Fix PR35824

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK added a comment.
Herald added a subscriber: Szelethus.

What is the status of the PR?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46027



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits