[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

[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:

[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

[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:

[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

[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:

[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

[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

[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

[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

[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/

[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:

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

ztamas updated this revision to Diff 253438. ztamas added a comment. Remove false TODO comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76990/new/ https://reviews.llvm.org/D76990 Files:

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

ztamas added a comment. In D76990#1948286 , @njames93 wrote: > I'm not entirely sure this is where the fix needs to be for this. The test > case code is whacky as hell, but from what I can see clang should always emit > a `BinaryOperator` for dependent

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

ztamas updated this revision to Diff 253412. ztamas added a comment. Rebase, TODO comment, remove unrelated change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76990/new/ https://reviews.llvm.org/D76990 Files:

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

ztamas updated this revision to Diff 253354. ztamas added a comment. Run clang-format on test code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76990/new/ https://reviews.llvm.org/D76990 Files:

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

ztamas created this revision. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. ztamas edited the summary of this revision. ztamas added a reviewer: aaron.ballman. It seems we need a different matcher for binary operator in a template context. Fixes this issue:

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

ztamas added a comment. In D75749#1912338 , @njames93 wrote: > LGTM > > However, how does this handle cases when the type is written as `char`. They > can be signed/unsigned based on what platform is being targeted. But on a > platform where `char` is

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

This revision was automatically updated to reflect the committed changes. Closed by commit rG04410c565aa0: [clang-tidy] extend bugprone-signed-char-misuse check. (authored by ztamas). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75749/new/

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

ztamas updated this revision to Diff 248917. ztamas added a comment. Update code based on reviewer comments. - Update docs: comparison -> equality/inequality comparison. - Use hasAnyOperatorName - Plus fix a typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

ztamas updated this revision to Diff 248912. ztamas added a comment. Use quotes in warning message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75749/new/ https://reviews.llvm.org/D75749 Files:

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

ztamas marked an inline comment as done. ztamas added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97 + const auto CompareOperator = + expr(binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")), +

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

ztamas updated this revision to Diff 248909. ztamas added a comment. One more unneeded allOf and clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75749/new/ https://reviews.llvm.org/D75749 Files:

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

ztamas updated this revision to Diff 248907. ztamas added a comment. Fix pre-merge check error Plus remove unneeded "no warning" comments and add a missing quote mark in docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75749/new/

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

ztamas updated this revision to Diff 248908. ztamas added a comment. Changed code based on reviewer comments. - Removed unneeded allOf() - Remove unneeded ';' - Added new line at the end of the file - Moved variable declarations inside if() Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

ztamas added a comment. In the LibreOffice codebase, I found 8 catches: https://gist.github.com/tzolnai/2b22492c0cf79f5dba577f6a878657e3 All catches seem valid to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75749/new/

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

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. I run the check on LLVM code and found only one catch of suspicious comparison: clang-tidy -p=/home/zolnai/clang/build

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

ztamas added a comment. I run the check on LLVM code and found only one catch of suspicious comparison: clang-tidy -p=/home/zolnai/clang/build /home/zolnai/clang/llvm-project/clang/lib/Lex/Lexer.cpp /home/zolnai/clang/llvm-project/clang/lib/Lex/Lexer.cpp:1293:39: warning: comparison

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

ztamas added a comment. In D71174#1818883 , @sylvestre.ledru wrote: > In D71174#1774249 , @ztamas wrote: > > > I run the new check on LibreOffice codebase with the option > > CharTypdefsToIgnore = "sal_Int8". > >

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

ztamas added a comment. In D71174#1815830 , @lebedev.ri wrote: > A little bit late to the party, but still. > I'm honestly wondering if this should be a proper clang static analyzer > data-flow aware check. > This is basically diagnosing every `signed

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG350da402ef6b: [clang-tidy] new check: bugprone-signed-char-misuse (authored by ztamas). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/

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

ztamas added a comment. Thanks for the review! I applied for the renewal of my commit access. I had commit access only for the SVN repo. After I get access to the GitHub repo I'll push this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

ztamas updated this revision to Diff 235790. ztamas added a comment. Update warning message in test files and rebase patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files:

[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/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97-98 + diag(CastExpression->getBeginLoc(), + "singed char -> integer (%0) conversion; " + "consider to cast to

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

ztamas updated this revision to Diff 235765. ztamas marked an inline comment as done. ztamas added a comment. Update docs / warning message, according to reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/

[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

[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

[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 updated this revision to Diff 233643. ztamas marked 2 inline comments as done. ztamas added a comment. Add newlines to the end of the files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files:

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

ztamas updated this revision to Diff 233638. ztamas added a comment. Update code based on reviewer comments. - Remove spurious semicolon. - Add tests about plain char. - Extend documentation describing how to control char signdness. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

ztamas updated this revision to Diff 232851. ztamas added a comment. Remove anonymus namespace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files:

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

ztamas updated this revision to Diff 232814. ztamas added a comment. Fixes small things mentioned by reviewer commments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71174/new/ https://reviews.llvm.org/D71174 Files:

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

ztamas added a comment. I run the new check on LLVM codebase with the option CharTypdefsToIgnore = "int8_t". The check produced 12 findings. All findings seem valid use cases, where a char -> integer conversion happens. I had a closer look at some of the catches. Somewhere I see only type

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

ztamas added a comment. I run the new check on LibreOffice codebase with the option CharTypdefsToIgnore = "sal_Int8". The check produced 32 findings. I see 3 false positives which are similar to the DereferenceWithTypdef test case where the code uses sal_Int8 typedef, but the check still

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

ztamas created this revision. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. This check searches for signed char -> integer conversions which might indicate programming error, because of the misinterpretation of char values. A signed char might store the

[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:

[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:

[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:

[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.

[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:

[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).

[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:

[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

[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),

[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:

[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 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:

[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 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

[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:

[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( +

[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

[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("=="),

[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

[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 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("=="),

[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/

[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

[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

[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:

[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( +

[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:

[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

[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

[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:

[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

[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

[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

[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:

[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]

[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,

[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

[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:

[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 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:

[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:

[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,

[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:

[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

[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

[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:

[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

[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

[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 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

[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

[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. >> 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 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

  1   2   >