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

ztamas created this revision. Herald added subscribers: cfe-commits, xazax.hun, mgorny. The new checker searches for those for loops which has a loop variable with a "too small" type which means this type can't represent all values which are part of the iteration range. For example: int main() {

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

ztamas updated this revision to Diff 172113. ztamas added a comment. Add a description of the checker with examples to the documentation Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt

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

ztamas added a comment. Just a note. I run the new checker on LibreOffice project. I found ~25 false positives, which seems small enough to me. This false positives can be supressed easily. Since the LibreOffice project has a similar check as a compiler plugin the code was already cleaned up, h

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

ztamas added a comment. In https://reviews.llvm.org/D53974#1283833, @sberg wrote: > > I run the new checker on LibreOffice project. I found ~25 false positives, > > which seems small enough to me. This false positives can be supressed > > easily. > > Do you have a link to such a false positive

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

ztamas added a comment. In https://reviews.llvm.org/D53974#1283759, @ZaMaZaN4iK wrote: > Hmm, i thought Clang has some warning for this, but I was wrong... Did you > think to implement this check as Clang warning? It did not come in my mind to do that. It might be good idea, I guess. This ver

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

ztamas added a comment. In https://reviews.llvm.org/D53974#1283833, @sberg wrote: > > I run the new checker on LibreOffice project. I found ~25 false positives, > > which seems small enough to me. This false positives can be supressed > > easily. > > Do you have a link to such a false positive

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

ztamas added a comment. In https://reviews.llvm.org/D53974#1284000, @JonasToth wrote: > For general understanding: Couldn't this check be generalized to comparing > integers of different sizes? We tried a 'dont-mix-int-types' check for > arithmetic already, its complicated :) > But this as a s

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

ztamas updated this revision to Diff 172226. ztamas added a comment. Run clang-formats on files, remove unneeded empty lines, fix up documentation. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMa

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

ztamas marked 30 inline comments as done. ztamas added a comment. I fixed up formatting and the docs based on comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

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

ztamas added a comment. Just to make it clear. I think this check is in a good shape now. I mean it catches usefull issues and also does not produce too many false positives, as I see. So my intention here is to get it in as it is now (of course after I fixed things reviewers find) and later it

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

ztamas updated this revision to Diff 172422. ztamas added a comment. Update code based on reviewer comments: - Remove const qualifier - Fix some comments - Use isInstantiationDependent() method - Do not ignore macros - Mark code-constructs in docs - Handle the use case when both operands of the b

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

ztamas marked 16 inline comments as done. ztamas added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:20 + +static const char LoopName[] = "forLoopName"; +static const char loopVarName[] = "loopVar"; JonasToth wrote: > Please move

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

ztamas updated this revision to Diff 172436. ztamas added a comment. - Use StringRef instead of char[] - Add test cases about big constant / literal / enum values Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-ti

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

ztamas marked 3 inline comments as done. ztamas added inline comments. Comment at: test/clang-tidy/bugprone-too-small-loop-variable.cpp:6 +void voidBadForLoop() { + for (int i = 0; i < size(); ++i) { +// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has a narrower

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

ztamas updated this revision to Diff 172486. ztamas marked an inline comment as done. ztamas added a comment. - Analyze template dependant for loops too (add some test cases) - Use double backticks for marking code in docs Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Fi

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

ztamas marked 2 inline comments as done. ztamas added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:109 + +if (RHSE->isInstantiationDependent() || LHSE->isInstantiationDependent()) + return 0; Szelethus wrote: > You seem

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

ztamas updated this revision to Diff 172487. ztamas marked an inline comment as done. ztamas added a comment. Update docs based on reviewer comment Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMa

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

ztamas added a comment. In https://reviews.llvm.org/D53974#1285930, @Szelethus wrote: > In https://reviews.llvm.org/D53974#1283759, @ZaMaZaN4iK wrote: > > > Hmm, i thought Clang has some warning for this, but I was wrong... Did you > > think to implement this check as Clang warning? > > > That i

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

ztamas marked 10 inline comments as done. ztamas added a comment. Mark all handled inline comments as Done. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

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

ztamas added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:142 + if (LoopVar->getType() != LoopIncrement->getType()) +return; // We matched the loop variable incorrectly + JonasToth wrote: > Does this try to ensure a precondi

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

ztamas added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:45 + + // We need to catch only those comparisons which contain any integer cast + StatementMatcher LoopVarConversionMatcher = JonasToth wrote: > missing full stop. Sorr

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

ztamas updated this revision to Diff 172635. ztamas added a comment. - Add a range-based loop test case - Restructure test cases a bit - Fix-up comments, position, punctuation Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Files: clang-tidy/bugprone/BugproneTidyModule.c

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

ztamas added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:142 + if (LoopVar->getType() != LoopIncrement->getType()) +return; // We matched the loop variable incorrectly + ztamas wrote: > JonasToth wrote: > > Does this try to

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

ztamas updated this revision to Diff 172938. ztamas added a comment. - no `else` after `return` - `static constexpr llvm::StringLiteral` - CamelCase variable names - Remove unneeded isIntegerType() check - Better terminology: not terminating condition, but iteration's upper bound. Repository:

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

ztamas updated this revision to Diff 173372. ztamas added a comment. - Make local functions `static` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/TooSmallLoopV

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

ztamas added a comment. In https://reviews.llvm.org/D53974#1293268, @JonasToth wrote: > LGTM. > Did you run this check in its final form against a bigger project? These > results would be interesting. I'll run it on LibreOffice code again and we'll see. > Do you have commit access? Commit a

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

ztamas added a comment. I have the result after running the current version if the check. Found around 50 new issues were hidden by macros: https://cgit.freedesktop.org/libreoffice/core/commit/?id=afbfe42e63cdba1a18c292d7eb4875009b0f19c0 Also found some new false positives related to macros. The

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

ztamas added a comment. I also tested on LLVm code. The output is here: https://gist.github.com/tzolnai/fe4f23031d3f9fdbdbf1ee38abda00a4 I found 362 warnings. Around 95% of these warnings are similar to the next example: /home/zolnai/lohome/llvm/lib/Support/Chrono.cpp:64:24: warning: loop var

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

ztamas updated this revision to Diff 173559. ztamas added a comment. - Add new test cases based on findings in LibreOffice code base Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt c

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

ztamas added a comment. >> Also found some new false positives related to macros. The number of all >> false positives is around 38, which is still seems good to me. > > I would be very interested why these false positives are created. Is there a > way to avoid them and maybe it makes sense to

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

ztamas updated this revision to Diff 173560. ztamas added a comment. - Fix comment Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp

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

ztamas added a comment. In https://reviews.llvm.org/D53974#1294659, @JonasToth wrote: > >> I would be very interested why these false positives are created. Is there > >> a way to avoid them and maybe it makes sense to already add `FIXME` at the > >> possible places the check needs improvements

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

ztamas updated this revision to Diff 173573. ztamas added a comment. - Add requested test case Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/TooSmallLoopVariabl

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

ztamas added a comment. > Agree. I think this check is good to go. > > I would commit this check tomorrow if that is ok with you. Of course, It would be great if this check can get in, Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974

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

ztamas added a comment. > Thank you very much for the patch! Thank you for reviewing! Repository: rL LLVM https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas created this revision. Herald added subscribers: cfe-commits, jdoerfert, xazax.hun. Herald added a project: clang. The bugprone-too-small-loop-variable check often catches loop variables which can represent "big enough" values, so we don't actually need to worry about that this variable w

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas added a comment. I run the check with and without the option on llvm source code. Without the option I found 370 catches, while setting MagnitudeBitsUpperLimit to `30` I found only two catches: /home/zolnai/libreoffice/clang/llvm-project/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp:390

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas added a comment. I found a project where the bugprone-too-small-loop-variable was used: https://github.com/openMSX/openMSX/commit/55006063daffa90941d4c3f9b0034e494d9cf45a As the commit message says for 32-bit integers the overflow never happens in practice. Repository: rCTE Clang Tool

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas marked an inline comment as done. ztamas added a comment. In D59870#1444645 , @JonasToth wrote: > I think in general this is a good direction. What do you think about other > heuristics? > E.g. one could say that a vector will not contain more the

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas updated this revision to Diff 192582. ztamas added a comment. Rebased patch on https://github.com/llvm/llvm-project.git repo. Updated the docs based on review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59870/new/ https://reviews.llvm.org/D59870 Files: clang-tools-

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas added a comment. In D59870#1446537 , @JonasToth wrote: > > I think it's the easiest way to specify the bits of the ineteger type to > > limit the catches. In real life, I met with this overflow / infinite loop > > problem with 16-bit short type, s

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas updated this revision to Diff 193017. ztamas added a comment. Make 16 the default value of MagnitudeBitsUpperLimit option. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59870/new/ https://reviews.llvm.org/D59870 Files: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariable

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas updated this revision to Diff 193277. ztamas added a comment. Fixed typo. Mentioned the default value in release notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59870/new/ https://reviews.llvm.org/D59870 Files: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableChe

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas marked 3 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:33 + + Upper limit for the magnitue bits of the loop variable. If it's set the check + filters out those catches in

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas updated this revision to Diff 194261. ztamas added a comment. Fixed a typo in release notes. Rebased the patch withh git pull -r. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59870/new/ https://reviews.llvm.org/D59870 Files: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopV

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas added a comment. Ok, I just fixed a remaining typo and rebased the patch to have it work with a more recent version of the code base, so I guess this patch is ready for commit. Jonas, can you commit this change for me? I still don't have commit access, I just applied for it some days ago.

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas created this revision. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. This check searches for copy assignment operators which might not handle self-assignment properly. There are three patterns of handling a self assignment situation: self check, c

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas added a comment. On llvm source code the check finds three suspicious methods: /home/zolnai/libreoffice/clang/llvm-project/clang/lib/AST/NestedNameSpecifier.cpp:534:1: warning: operator=() might not handle self-assignment properly [bugprone-unhandled-self-assignment] operator=(const

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas added a comment. On LibreOffice source code I found 36 catches with this check. 16 of them was worth to fix because in those cases the object state was changed in some way after a self-assignment: https://cgit.freedesktop.org/libreoffice/core/commit/?id=3a5d78365dd172881c16c03e67f2d170ffc6

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas added a comment. I'll update the patch based on the comments. Just a note about the false positive ratio. In LibreOffice we run other static analyzers too, that's why there were less positive catches there than false positives. For example, PVS studio also catches unprotected copy assignm

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 194921. ztamas added a comment. Updated the code based on reviewer comments: - Alphabetical order in `BugproneTidyModule.cpp` - Handling of `auto_ptr` - Test cases for template classes (I made the check ignore these case otherwise this can lead to false posit

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas marked 13 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10 + +This check corresponds to the CERT C++ Coding Standard rule +`OOP54-CPP. Gracefully handle self-copy assignme

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 194925. ztamas marked an inline comment as done. ztamas added a comment. Documentation fixes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

ztamas added a comment. In the meantime, I've got the commit access, so I'll give it a try to push this myself. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59870/new/ https://reviews.llvm.org/D59870 ___ cfe-commits mailing list cfe-commi

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

This revision was automatically updated to reflect the committed changes. Closed by commit rL358356: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop… (authored by ztamas, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 195061. ztamas added a comment. Missed to syncronize ReleasNotes with the corresponding doc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507 Files: clang-tools-extra/c

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas marked an inline comment as done. ztamas added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:51 + // Matcher for standard smart pointers. + const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType( + r

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 195382. ztamas added a comment. Update patch based on reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507 Files: clang-tools-extra/clang-tidy/bugprone/B

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas marked 4 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:6 + +`cert-oop54-cpp` redirects here as an alias for this check. + aaron.ballman wrote: > You shoul

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas marked 2 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp:351 + int *p; +}; aaron.ballman wrote: > ztamas wrote: > > JonasToth wrote: > > > Please add tests with t

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 196066. ztamas added a comment. Add false positive test cases. Added a note about template related limitation to the docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas marked 4 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:34-36 + const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant( + binaryOperator(anyOf(hasOperatorName("=="), h

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas marked an inline comment as done. ztamas added inline comments. Comment at: clang-tools-extra/test/clang-tidy/bugprone-unhandled-self-assignment.cpp:351 + int *p; +}; aaron.ballman wrote: > ztamas wrote: > > aaron.ballman wrote: > > > ztamas wrote: > > >

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 196279. ztamas added a comment. Added missing fullstop Added a new test cases making sure that HasNoSelfCheck works correctly Added template instantiation to tests Remove the alias from cert module and add the CERT link as a see also Repository: rG LLVM Git

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas marked 5 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:34-36 + const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant( + binaryOperator(anyOf(hasOperatorName("=="), h

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas added a comment. Ok, so I removed the alias from cert module and added CERT rule link as a "see also". So I think we solved the problem that things do not conform with the CERT requirements and can focus on the actual problem. Summary, templates are still ignored. If there is any idea ho

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas marked 3 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:62-64 + const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl( + has(fieldDecl(anyOf(hasType(pointerT

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 196295. ztamas added a comment. Make the check to work with templates too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507 Files: clang-tools-extra/clang-tidy/bugprone/

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas added a comment. I also reformatted the code with clang-format. So now the templates are handled, however, it's still not fit with the cert rule because we not catch all classes, but only those who have suspicious fields. I think this one can be added in a follow-up patch and then this c

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas marked 8 inline comments as done. ztamas added a comment. Mark some comments Done, which were handled some way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507 _

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 196410. ztamas added a comment. Remove outdated comment from docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507 Files: clang-tools-extra/clang-tidy/bugprone/Bugpron

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas added a comment. Ok, is there anything else should be included in this patch? I saw more ideas about how to extend the functionality (custom pointers, fix-it, etc). I interpreted these ideas as nice-to-have things for the future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 197014. ztamas added a comment. Better handling of template and non-template self-copy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507 Files: clang-tools-extra/clang-t

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas added a comment. Ping. Is it good to go or is there anything else I need to include in this patch? Among the lots of idea, I'm not sure which is just brainstorming and which is a change request. The check seems to work (has useful catches and does not produce too many false positives), ho

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 198787. ztamas added a comment. Changed "might not" to "does not" in the warning message. Added the requested test case with the swapped template arguments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 198789. ztamas added a comment. copy operator -> copy assignment operator Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60507/new/ https://reviews.llvm.org/D60507 Files: clang-tools-extra/clang-tidy/bugprone/

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas added a comment. > I definitely want to see the diagnostic text changed away from "might be" -- > that's too noncommittal for my tastes. I'd also like to see the additional > test case (I suspect it will just work out of the box, but if it doesn't, it > would be good to know about it). T

[PATCH] D52936: Add some automatic tests for DivideZero checker

ztamas created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D52936 Files: test/Analysis/div-zero.cpp Index: test/Analysis/div-zero.cpp === --- test/Analysis/div-zero.cpp +

[PATCH] D52936: Add some automatic tests for DivideZero checker

ztamas added a reviewer: dcoughlin. ztamas added a comment. Based on CODE_OWNERS.txt. Repository: rC Clang https://reviews.llvm.org/D52936 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

This revision was automatically updated to reflect the committed changes. Closed by commit rL360540: [clang-tidy] new check: bugprone-unhandled-self-assignment (authored by ztamas, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: ht

[PATCH] D62192: [clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment

ztamas created this revision. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Added WarnOnlyIfThisHasSuspiciousField option to allow to catch any copy assignment operator independently from the container class's fields. Added the cert alias using this option. Rep

[PATCH] D62192: [clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 200714. ztamas added a comment. Fixed docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62192/new/ https://reviews.llvm.org/D62192 Files: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.c

[PATCH] D62192: [clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment

ztamas updated this revision to Diff 200775. ztamas added a comment. Herald added a subscriber: mgorny. Link burprone module to cert module Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62192/new/ https://reviews.llvm.org/D62192 Files: clang-too

[PATCH] D62192: [clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE361550: [clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment (authored by ztamas, committed by ). Changed prior to commit: https://reviews.llvm.org/D62192?vs=200775&id=201051#

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

ztamas updated this revision to Diff 255038. ztamas added a comment. Add suggested test case and rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76990/new/ https://reviews.llvm.org/D76990 Files: clang-tools-extra/clang-tidy/bugprone/Unhand

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

This revision was automatically updated to reflect the committed changes. Closed by commit rG0f9e1e3ae750: [clang-tidy]: fix false positive of cert-oop54-cpp check. (authored by ztamas). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76990/new/ https

[PATCH] D78904: [clang-tidy] extend bugprone-signed-char-misuse check with array subscript case.

ztamas added a comment. I run the check both on LibreOffice and LLVM codebase but did not find any catch. I expect that it's not a frequent use case. I added this extension only to fully cover the CERT rule. If this patch is accepted I can add a cert alias for this check. Repository: rG LLVM

[PATCH] D78904: [clang-tidy] extend bugprone-signed-char-misuse check with array subscript case.

ztamas created this revision. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. ztamas added a reviewer: aaron.ballman. ztamas added a comment. ztamas updated this revision to Diff 260243. I run the check both on LibreOffice and LLVM codebase but did not find any ca

[PATCH] D78904: [clang-tidy] extend bugprone-signed-char-misuse check with array subscript case.

ztamas updated this revision to Diff 260243. ztamas added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78904/new/ https://reviews.llvm.org/D78904 Files: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp clang

[PATCH] D78904: [clang-tidy] extend bugprone-signed-char-misuse check with array subscript case.

This revision was automatically updated to reflect the committed changes. Closed by commit rG030ff901f432: [clang-tidy] extend bugprone-signed-char-misuse check with array subscript case. (authored by ztamas). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D78904: [clang-tidy] extend bugprone-signed-char-misuse check with array subscript case.

ztamas updated this revision to Diff 261634. ztamas added a comment. Spelling / grammar fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78904/new/ https://reviews.llvm.org/D78904 Files: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuse

[PATCH] D79334: [clang-tidy]: Add cert-str34-c alias for bugprone-signed-char-misuse.

ztamas created this revision. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. ztamas edited the summary of this revision. ztamas edited the summary of this revision. ztamas edited the summary of this revision. ztamas added a reviewer: aaron.ballman. Added CertSTR34

[PATCH] D79334: [clang-tidy]: Add cert-str34-c alias for bugprone-signed-char-misuse.

ztamas updated this revision to Diff 262046. ztamas added a comment. CertSTR34C -> DiagnoseSignedUnsignedCharComparisons Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79334/new/ https://reviews.llvm.org/D79334 Files: clang-tools-extra/clang-tidy

[PATCH] D79334: [clang-tidy]: Add cert-str34-c alias for bugprone-signed-char-misuse.

ztamas marked 3 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h:41 const std::string CharTypdefsToIgnoreList; + const bool CertSTR34C; }; aaron.ballman wrote: > I'd prefer a mor

[PATCH] D79334: [clang-tidy]: Add cert-str34-c alias for bugprone-signed-char-misuse.

ztamas updated this revision to Diff 262316. ztamas marked an inline comment as done. ztamas added a comment. Documentation fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79334/new/ https://reviews.llvm.org/D79334 Files: clang-tools-extra/c

[PATCH] D79334: [clang-tidy]: Add cert-str34-c alias for bugprone-signed-char-misuse.

This revision was automatically updated to reflect the committed changes. Closed by commit rGfedd52682ec7: [clang-tidy]: Add cert-str34-c alias for bugprone-signed-char-misuse. (authored by ztamas). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79334

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

ztamas marked an inline comment as done. ztamas added inline comments. Herald added a subscriber: whisperity. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:43-44 + + const auto SignedCharType = expr(hasType(qualType( + allOf(isAnyCharacter(),

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

ztamas added a comment. I added the above comments two weeks ago, I just forget to push the submit button. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 ___

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

ztamas marked 2 inline comments as done. ztamas added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:66 bugprone-posix-return + bugprone-signed-char-misuse bugprone-sizeof-container sylvestre.ledru wrote: > list.rst has c

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

ztamas updated this revision to Diff 235470. ztamas added a comment. Rebase patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp cla

  1   2   >