[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 92775. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. No changes compared to v2, just correctly rebased the master branch now. https://reviews.llvm.org/D31252 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 92774. lebedev.ri added a comment. Addressing review notes. https://reviews.llvm.org/D31252 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/CompoundStatementSizeCheck.cpp clang-tidy/readability/CompoundStatementSizeCheck.h

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-03-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D31252#708209, @Eugene.Zelenko wrote: > Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). Done > Will be good idea to run Clang-tidy modernize and readability checks over new > code. I did run them

[PATCH] D7375: [clang-tidy] Assert related checkers

2017-03-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/clang-tidy/misc-assert-side-effect.cpp:67 + + assert(freeFunction()); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() with side effect Is it intentional that the check also warns on free functions

[PATCH] D31252: Clang-tidy: add readability-compound-statement-size check.

2017-03-22 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang-tools-extra. Herald added a subscriber: mgorny. This check is quite similar to the readability-function-size check. But it is more generic. It works for each compound statement, excluding the ones directly related to the

[PATCH] D31252: Clang-tidy: add readability-compound-statement-size check.

2017-03-22 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Do note that clang-tidy/readability/CompoundStatementSizeCheck.cpp is *very* similar to the clang-tidy/readability/FunctionSizeCheck.cpp I'm not sure if it makes sense to have such duplication. But i'm also not sure yet how to deduplicate them. But in the long run, i

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

2017-07-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. PR #33771 filled. IMHO the problems with this diagnostic should be resolved before 5.0 https://reviews.llvm.org/D32914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35472: Implement P0463R1: "Endian just Endian"

2017-07-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: include/type_traits:4740 +#if _LIBCPP_STD_VER > 14 +enum class endian (Apologies for double commenting, did not notice that it was in phab until after replying) I'm probably wrong, but if this is a C++2a

[PATCH] D33365: [clang-tidy] misc-assertion-count: A New Check

2017-07-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D33365#775916, @alexfh wrote: > In https://reviews.llvm.org/D33365#775880, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D33365#775860, @alexfh wrote: > > > > > I guess, this check should go to `readability` or elsewhere, but > > >

[PATCH] D35068: Detect usages of unsafe I/O functions

2017-07-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This does not do anything more than traversing the AST, shouldn't this be a clang-tidy check? Also, i suspect CERT-MSC24-C might be relevant

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. How does this relate to the gcc behavior? I suspect not everyone would want to have this relaxed `-Wshadow` mode. Perhaps it could be hidden under some new flag, which is not going to be automatically enabled by `-Weverything`. Like, `-Wshadow-in-macros` which does

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-07-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Will land this shortly.. Repository: rL LLVM https://reviews.llvm.org/D33102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-07-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 105103. lebedev.ri added a comment. Rebased before commit. Repository: rL LLVM https://reviews.llvm.org/D33102 Files: docs/ReleaseNotes.rst lib/Sema/SemaCast.cpp test/Sema/warn-cast-qual.c test/SemaCXX/warn-cast-qual.cpp Index:

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

2017-07-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. What will this check say about the following code? c++ void function(void) { char c = 'x'; int *ip = reinterpret_cast(); } The clang'g `-Wcast-align` does not warn: https://godbolt.org/g/hVwD5S Repository: rL LLVM https://reviews.llvm.org/D33826

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

2017-07-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. @thakis ping? https://reviews.llvm.org/D32914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31252: [clang-tidy] add readability-compound-statement-size check.

2017-04-27 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri abandoned this revision. lebedev.ri added a comment. Thank you all! After some thought, i have come to conclusion that it is best to start with something less controversial, and simpler. https://reviews.llvm.org/D31252 ___ cfe-commits

[PATCH] D33365: [clang-tidy] misc-assertion-count: A New Check

2017-08-01 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D33365#802884, @lebedev.ri wrote: > In https://reviews.llvm.org/D33365#775916, @alexfh wrote: > > > I think, this kind of a check needs some prior research (not necessarily in > > the sense of a printed paper, but at least a thoughtful

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-08-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D36836#845417, @JonasToth wrote: > Did I run it over clang or athletic like that? I would be interested in an > overall output. No, not yet. But if it is to be enabled for LLVM repos by default, then the `Threshold` will most

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-08-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 111731. lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. Address review notes: - `constexpr const` -> `constexpr` - moved `test/clang-tidy/check_clang_tidy.py` changes into https://reviews.llvm.org/D36892 Repository: rL LLVM

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2017-08-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang-tools-extra. Herald added subscribers: xazax.hun, JDevlieghere. Currently, there is two configured prefixes: `CHECK-FIXES` and `CHECK-MESSAGES` `CHECK-MESSAGES` checks that there are no test output lines with `warning:|error:`,

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

2017-08-22 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Please add the following test: (and make sure that it does the right thing :)) bool f_with_preproc_condition() { auto test = 42; assert(test == 42); return test; } I.e. if `-DNDEBUG` is present, variable is not needed, but if `-DNDEBUG` is *NOT*

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

2017-08-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D37014#850064, @tbourvon wrote: > In https://reviews.llvm.org/D37014#849157, @lebedev.ri wrote: > > > Please add the following test: (and make sure that it does the right thing > > :)) > > > > bool f_with_preproc_condition() { > >

[PATCH] D37014: [clang-tidy] Add a checker to remove useless intermediate variables before return statements with comparisons

2017-08-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/UnnecessaryIntermediateVarCheck.cpp:376 +// expression wouldn't really benefit readability. Therefore we abort. +if (NewReturnLength > MaximumLineLength) { + return; Is there

[PATCH] D33365: [clang-tidy] misc-assertion-count: A New Check

2017-06-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D33365#775860, @alexfh wrote: > I guess, this check should go to `readability` or elsewhere, but definitely > not to `misc`. Hmm, `misc` may be a bad place for this, but i think `readability` is even worse fit. The best guess would be

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. So i'm trying to analyze that stage2 warning. The testcase //seems// to be: (autogenerated all the variants) void test_nop() { unsigned char **ptr1 = 0; void **ptr2 = (void **)ptr1; } void test_bad() { unsigned char **ptr1 = 0; const void **ptr2

[PATCH] D33365: [clang-tidy] misc-assertion-count: A New Check

2017-05-29 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. I will obviously try to add lambda handling, but until then, the general idea and design should be pretty much done, so i'd love to hear any feedback about the current diff :) Repository: rL LLVM https://reviews.llvm.org/D33365

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

2017-05-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Relevant https://reviews.llvm.org/D31370, https://reviews.llvm.org/D19201 https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

2017-06-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/clang-tidy/cert-avoid-pointer-cast-to-more-strict-alignment.c:17 + + tmp = (struct foo_header *)(data + offset); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not cast pointers into more strictly aligned pointer types

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 101232. lebedev.ri marked an inline comment as done. lebedev.ri added a subscriber: cfe-commits. lebedev.ri added a comment. Address review notes. Repository: rL LLVM https://reviews.llvm.org/D33102 Files: docs/ReleaseNotes.rst

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D33102#767291, @dblaikie wrote: > Still seems like an awful lot more testing than I'd expect for this warning. > (eg: testing all the many variations of C++ const_cast - when they all > provide basically the same coverage I think, that

[PATCH] D33827: [clang-tidy] misc-static-assert shouldn't flag assert(!"msg")

2017-06-02 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. While not directly related to this Differential, i wonder if it would make sense to also not warn on calls to (especially non-member) functions marked with `__attribute__((const))`, maybe `__attribute__((pure))` too. At least i'm not sure what side-effects such calls

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D33102#773296, @dblaikie wrote: > I still feel like that's more testing than would be ideal (does the context > of the cast matter? Wether it's dereferenced, a struct member access, > assigned, initialized, etc - it doesn't look like it

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. (tested with: gcc version 6.3.0 20170516 (Debian 6.3.0-18) ```) Repository: rL LLVM https://reviews.llvm.org/D33102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.

2017-06-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D32942#776915, @malcolm.parsons wrote: > Does this check consider `else if` chains to be nesting? > > void nesting() { // 1 > if (true) { // 2 >int j; > } else if (true) { // 2 or 3? >int j; > } else if (true)

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 102119. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. After further mail exchange, i will proceed to commit this as-is. No code changes, rebase before commit. Repository: rL

[PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri planned changes to this revision. lebedev.ri added a comment. In https://reviews.llvm.org/D33102#773296, @dblaikie wrote: > But sure. Could you also (manually, I guess) confirm that this matches GCC's > cast-qual behavior (insofar as the warning fires in the same situations). If >

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D34202#780394, @malcolm.parsons wrote: > What do you expect for this? > > if (true) > if (true) > if (true) { > int j; > } > that it is equivalent to if (true && true && true) { // 1 int j;

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/FunctionSizeCheck.cpp:38 +} else { + assert(!isa(Node)); + if (TrackedParent.back()) malcolm.parsons wrote: > I don't think this assert adds anything. Yes, however the original

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D34202#780424, @malcolm.parsons wrote: > My coding standards require the `{}`, so while I may not agree with your > definition of nesting, it doesn't matter. Well, seeing `readability-deep-nesting.cpp` in the issue, i suppose the

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 102542. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Fix spelling. Repository: rL LLVM https://reviews.llvm.org/D34202 Files: clang-tidy/readability/FunctionSizeCheck.cpp test/clang-tidy/readability-function-size.cpp

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 102548. lebedev.ri added a comment. Simplify it even further by moving the logic into it's proper place - `TraverseCompoundStmt()` Repository: rL LLVM https://reviews.llvm.org/D34202 Files: clang-tidy/readability/FunctionSizeCheck.cpp

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri planned changes to this revision. lebedev.ri added a comment. Hmm, this is moving in the right direction, but now it invalidated (decreases the nesting level) too eagerly Repository: rL LLVM https://reviews.llvm.org/D34202 ___

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 102533. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. Ok, i really messed up the initial https://reviews.llvm.org/D32942 :) Malcolm, thank you for bothering to report :) This should be so much better. Repository: rL LLVM

[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D32942#780053, @malcolm.parsons wrote: > In https://reviews.llvm.org/D32942#779143, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D32942#778729, @malcolm.parsons wrote: > > > > > In https://reviews.llvm.org/D32942#777001, @lebedev.ri

[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.

2017-06-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D32942#778729, @malcolm.parsons wrote: > In https://reviews.llvm.org/D32942#777001, @lebedev.ri wrote: > > > Which makes sense, since in AST, they are nested: > > > They're not nested in the formatting, so I don't think it makes sense. As

[PATCH] D32942: [clang-tidy] readability-function-size: add NestingThreshold param.

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Hm, oh, maybe this is as simple as a partially-unintentional `switch` fallthrough Repository: rL LLVM https://reviews.llvm.org/D32942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34202: [clang-tidy] readability-function-size: fix nesting level calculation

2017-06-14 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang-tools-extra. A followup for https://reviews.llvm.org/D32942. Malcolm Parsons has provided a valid testcase that the initial version of the check complained about nested `if`'s. As it turns out, the culprit is the **partially**

[PATCH] D16717: [clang-tidy] Add non-constant references in function parameters check.

2017-06-09 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. // Don't warn on dependent types in templates. Hmm, i wanted to try to fix my Bug 32683 about templated references being ignored, and i found out that it is actually intentional... Reading

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

2017-05-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. This warning complains about macros from system headers, e.g. `PTHREAD_MUTEX_INITIALIZER`: $ ninja -j1 -v [1/110] /usr/bin/cmake -E __run_iwyu --tidy=/usr/local/bin/clang-tidy --source=../src/librawspeed/common/DngOpcodes.cpp -- /usr/local/bin/clang++ -DDEBUG

[PATCH] D32914: Introduce Wzero-as-null-pointer-constant.

2017-05-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. And a lot of warnings from code using googletest, highlight: ../src/librawspeed/metadata/ColorFilterArrayTest.cpp:56:1: error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant] TEST(ColorFilterArrayTestBasic, Constructor) { ^

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-09-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. What about GNU extension `case 1 ... 3:` ? https://reviews.llvm.org/D37808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-09-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D37808#869879, @JonasToth wrote: > In https://reviews.llvm.org/D37808#869823, @lebedev.ri wrote: > > > What about GNU extension `case 1 ... 3:` ? > > > Strictly speaking, the coding standard (which should be enforced by the > patch)

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-09-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D36836#873032, @JonasToth wrote: > I built the patch locally, here is my output: > > The Unittest failed for me: `Neither CHECK-FIXES nor CHECK-MESSAGES found in > the input` > > Script call: > `/usr/bin/python2.7 >

[Diffusion] rL302247: Introduce Wzero-as-null-pointer-constant.

2017-09-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added subscribers: thakis, cfe-commits, dblaikie, malcolm.parsons, hans, lebedev.ri. lebedev.ri raised a concern with this commit. lebedev.ri added a comment. Hi @thakis! Are you planning on addressing the kind-of false-positives this diagnostic produces? It should not warn if `0`

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

2017-09-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D37808#869810, @Eugene.Zelenko wrote: > In https://reviews.llvm.org/D37808#869612, @JonasToth wrote: > > > In https://reviews.llvm.org/D37808#869602, @Eugene.Zelenko wrote: > > > > > I think will be good idea to extend -Wswitch diagnostics.

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-09-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 115547. lebedev.ri marked 13 inline comments as done. lebedev.ri added a comment. Address even more @JonasToth's review notes. - Fixed docs - Validated the implementation (the testcase) against the "reference" implementation - Fixed discrepancies in the

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-09-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102-114 +const std::array CognitiveComplexity::Msgs = {{ +// B1 + B2 + B3 +"+%0, including nesting penalty of %1, nesting level increased to %2", + +// B1 + B2 +

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 115890. lebedev.ri added a comment. Simplify `CheckTautologicalComparisonWithZero()` as per review notes. Less LOC. Repository: rL LLVM https://reviews.llvm.org/D37629 Files: include/clang/Basic/DiagnosticGroups.td

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Reverted due to buildbot failures. Need to investigate. Repository: rL LLVM https://reviews.llvm.org/D37629 ___ cfe-commits mailing

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-09-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang. Currently, clang only diagnoses completely out-of-range comparisons (e.g. `char` and constant `300`), and comparisons of unsigned and `0`. But gcc also does diagnose the comparisons with the `std::numeric_limits<>::max()` /

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-09-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D36836#877636, @alexfh wrote: > > The metric is implemented as per COGNITIVE COMPLEXITY by SonarSource > > specification version 1.2 (19 April 2017), > > Err, "I did ask them, and received an answer that it is it can be implemented > in

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-09-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Tried stage2 build, so far only one warning. But found out that `ubsan_value.h`'s `typedef s128 SIntMax;` crashes this code because class LLVM_NODISCARD APSInt : public APInt { ... /// \brief Get the correctly-extended \c int64_t value. int64_t

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-09-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:102-114 +const std::array CognitiveComplexity::Msgs = {{ +// B1 + B2 + B3 +"+%0, including nesting penalty of %1, nesting level increased to %2", + +// B1 + B2 +

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-09-17 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D36836#873246, @JonasToth wrote: > For my part the current state is ok. @JonasToth thank you for the review! > but @alexfh and @aaron.ballman should do their review before committing. +1 :) Now what one full review is done, it may be

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-09-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 115482. lebedev.ri marked 13 inline comments as done. lebedev.ri added a comment. Address most of the @JonasToth's review notes. Repository: rL LLVM https://reviews.llvm.org/D36836 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-09-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:62 +std::pair Process() const { + assert(C != Criteria::None && "invalid criteria"); + JonasToth wrote: > You acces `Critera` always scoped. I

[PATCH] D38171: Implement clang-tidy check aliases.

2017-09-22 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. On a somewhat related note, since this is already talking about aliases I feel like the current handling of the clang-tidy check aliases needs adjusting. If one enables all the checks (`Checks: '*'`), and then disables some of them on check-by-check basis, if the

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Ping, would be really nice to get this going :) Would unblock me for working on the second half of the fix for https://bugs.llvm.org/show_bug.cgi?id=34147 Repository: rL LLVM https://reviews.llvm.org/D37629 ___

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 115877. lebedev.ri added a comment. 1. Correctly added `TautologicalUnsignedEnumZeroCompare` to `TautologicalCompare` group (sigh) 2. Hopefully fixed spelling of the comments and added a bit better comments 3. Fixed variable names to comply with the

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-06 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8719 + // Type limit values are handled later by CheckTautologicalComparison(). + if (IsTypeLimit(S, Other, Constant, ConstValue) && (!OtherIsBooleanType)) return; lebedev.ri wrote: >

[PATCH] D38718: Patch to Bugzilla 20951

2017-10-10 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: test/Sema/conditional-expr.c:84 + //char x; + return (((x != ((void *) 0)) ? (*x = ((char) 1)) : (void) ((void *) 0)), (unsigned long) ((void *) 0)); // expected-warning {{C99 forbids conditional expressions with only one void

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:76 + // So either static out-of-line or non-static in-line. + const std::array Msgs = {{ + // FIXME: these messages somehow trigger an assertion:

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 119091. lebedev.ri marked 9 inline comments as done and an inline comment as not done. lebedev.ri added a comment. @aaron.ballman, thank you for the review! Rebased, addressed most of review notes. Repository: rL LLVM https://reviews.llvm.org/D36836

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 117802. lebedev.ri marked 2 inline comments as done. lebedev.ri added a subscriber: rtrieu. lebedev.ri added a comment. - Address review notes - ~~Avoid re-evaluating ICE when called from `DiagnoseOutOfRangeComparison()` which already knows the evaluated

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8719 + // Type limit values are handled later by CheckTautologicalComparison(). + if (IsTypeLimit(S, Other, Constant, ConstValue) && (!OtherIsBooleanType)) return; rsmith wrote: > Is

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-05 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 117824. lebedev.ri added a comment. Ping. - Rebased - Added the legal status info into `LICENSE.txt`, referencing `clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt`, as suggested by @aaron.ballman Question: Is it expected that

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros.

2017-10-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang. The warning was initially introduced in https://reviews.llvm.org/D32914 by @thakis, and the concerns were raised there, and later in https://reviews.llvm.org/rL302247 and PR33771. I do believe that it makes sense to relax the

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros.

2017-10-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D38954#898586, @thakis wrote: > As said on the bug, this matches gcc's behavior, https://bugs.llvm.org/show_bug.cgi?id=33771#c3 > and with this you won't see this warning for NULL. Finally, an argument that can actually be addressed. >

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros.

2017-10-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 119159. lebedev.ri added a comment. Address @thakis review notes: do make sure that we still warn on `NULL`. Any other special macros/cases? Repository: rL LLVM https://reviews.llvm.org/D38954 Files: lib/Sema/Sema.cpp

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 118793. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. @rsmith, thank you for the review! Rebased, address review notes, simplified all the things even further. Testing: - `$ ninja check-clang-sema check-clang-semacxx` - Stage

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8667-8679 + bool Result; // The result of the comparison + if ((Op == BO_GT && ValueType == LimitType::Max && ConstOnRight) || + (Op == BO_GT && ValueType == LimitType::Min && !ConstOnRight) || +

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-10-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D36836#889375, @aaron.ballman wrote: > Adding @dberlin for licensing discussion questions. Ping. Yes, i agree that what i have added is not a directory, and not a proper license. That is more of a template to hopefully stat moving

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 118829. lebedev.ri marked an inline comment as done. Repository: rL LLVM https://reviews.llvm.org/D38101 Files: docs/ReleaseNotes.rst include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8930 +// We only care about expressions where just one side is literal +if (IsRHSIntegralLiteral ^ IsLHSIntegralLiteral) { + // Is the constant on the RHS or LHS? rsmith wrote:

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 118833. lebedev.ri added a comment. Getting really weird problems when trying to use arc patch, maybe re-updating the differential helps Repository: rL LLVM https://reviews.llvm.org/D38101 Files: docs/ReleaseNotes.rst

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri reopened this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Reverted due to http://bb9.pgr.jp/#/builders/20/builds/59 that i don't currently know how to deal with. It is really sad that i failed to encounter it during testing. Repository:

[Diffusion] rL302247: Introduce Wzero-as-null-pointer-constant.

2017-10-16 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added an edge: D32914: Introduce Wzero-as-null-pointer-constant.. Users: lebedev.ri (Auditor) https://reviews.llvm.org/rL302247 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments. Comment at: cfe/trunk/test/Sema/tautological-constant-compare.c:23 + + if (c > macro(255)) + return; I'm having second thoughts about macro handling. Right now we completely ignore the comparisons when the constant is

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

2017-10-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Forgot to add, i really noticed/though about it just now, in https://reviews.llvm.org/D38871, because i did not encountered any warnings in that code in stage-2 builds. Repository: rL LLVM https://reviews.llvm.org/D38101

[PATCH] D37620: [Sema] Put tautological comparison of unsigned and zero into it's own flag

2017-09-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang. As requested by Sam McCall. $ /build/llvm-build-Clang-release/./bin/clang -c /build/clang/test/Sema/outof-range-constant-compare.c /build/clang/test/Sema/outof-range-constant-compare.c:40:11: warning: comparison of

[PATCH] D37620: [Sema] Put tautological comparison of unsigned and zero into it's own flag

2017-09-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 114351. lebedev.ri edited the summary of this revision. lebedev.ri added a comment. Added test. Repository: rL LLVM https://reviews.llvm.org/D37620 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D37565: [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

2017-09-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang. This is a first half(?) of a fix for the following bug: https://bugs.llvm.org/show_bug.cgi?id=34147 (gcc -Wtype-limits) GCC's -Wtype-limits does warn on comparison of unsigned value with signed zero (as in, with 0), but clang

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision. lebedev.ri added a project: clang. As requested by Sam McCall: > Enums (not new I guess). Typical case: if (enum < 0 || enum > MAX) > The warning strongly suggests that the enum < 0 check has no effect > (for enums with nonnegative ranges). > Clang doesn't

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 114387. lebedev.ri added a subscriber: jroelofs. lebedev.ri added a comment. Rework as per @jroelofs's suggestion to have just one `switch`/`if` cascade that operates on `BinaryOperatorKind` Repository: rL LLVM https://reviews.llvm.org/D37629 Files:

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 114396. lebedev.ri added a comment. And finish reducing the code by for-range-loop`ing over array + use `std::array`. Repository: rL LLVM https://reviews.llvm.org/D37629 Files: include/clang/Basic/DiagnosticGroups.td

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

2017-09-08 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D37629#865068, @lebedev.ri wrote: > And finish reducing the code by for-range-loop`ing over array + use > `std::array`. I will need to fix handling of the second edge-case (comparison with max unsigned value or with min/max for signed

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2017-08-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 113226. lebedev.ri added a comment. Rebased. Repository: rL LLVM https://reviews.llvm.org/D36892 Files: test/clang-tidy/check_clang_tidy.py Index: test/clang-tidy/check_clang_tidy.py

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

2017-08-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 113228. lebedev.ri added a comment. Rebased. Repository: rL LLVM https://reviews.llvm.org/D36836 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

2017-08-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Thank you for working on this! I just tried, and the original false-positive i was hitting is now gone. So as far i'm concerned, this is good to go. Comment at:

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

2017-09-11 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In https://reviews.llvm.org/D33826#866161, @JonasToth wrote: > There is an exception to the general rule (EXP36-C-EX2), stating that the > result of `malloc` and friends is allowed to be casted to stricter > alignments, since the pointer is known to be of correct

  1   2   3   4   5   6   7   8   9   10   >