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

2019-08-09 Thread 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/

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

2019-08-09 Thread 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

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

2019-08-09 Thread 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

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

2019-08-09 Thread 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

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

2019-08-08 Thread 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

[PATCH] D65912: Add new check for math constants

2019-08-07 Thread 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

[PATCH] D65912: Add new check for math constants

2019-08-07 Thread 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

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

2019-05-08 Thread 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

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

2019-05-07 Thread 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

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

2019-05-02 Thread 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

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

2018-11-04 Thread 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

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

2018-11-04 Thread 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: >

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

2018-11-04 Thread 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: >

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

2018-11-04 Thread 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: >

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

2018-11-04 Thread 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

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

2018-11-04 Thread 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: >

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

2018-11-01 Thread 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

2018-11-01 Thread 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

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

2018-10-29 Thread 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

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

2018-10-29 Thread 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

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

2018-10-29 Thread 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

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

2018-10-29 Thread 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 +

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

2018-10-29 Thread 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`: >

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

2018-10-29 Thread 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

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

2018-10-29 Thread 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

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

2018-10-29 Thread 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

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

2018-10-29 Thread 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

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

2018-10-28 Thread 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

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

2018-10-28 Thread 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

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

2018-10-28 Thread 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

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

2018-10-10 Thread 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