- User Since
- Oct 5 2018, 9:46 AM (112 w, 6 d)
May 6 2020
May 5 2020
CertSTR34C -> DiagnoseSignedUnsignedCharComparisons
May 4 2020
May 2 2020
Spelling / grammar fixes.
Apr 28 2020
Apr 27 2020
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.
Apr 4 2020
Add suggested test case and rebase.
Mar 29 2020
Remove false TODO comment.
Rebase, TODO comment, remove unrelated change.
Mar 28 2020
Run clang-format on test code.
Mar 14 2020
Mar 7 2020
Update code based on reviewer comments.
Use quotes in warning message.
One more unneeded allOf and clang-format
Changed code based on reviewer comments.
Fix pre-merge check error
Mar 6 2020
In the LibreOffice codebase, I found 8 catches:
I run the check on LLVM code and found only one catch of suspicious comparison:
Jan 14 2020
Jan 12 2020
Jan 6 2020
Jan 4 2020
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.
Jan 1 2020
Update warning message in test files and rebase patch.
Update docs / warning message, according to reviewer comments.
Dec 28 2019
Dec 27 2019
I added the above comments two weeks ago, I just forget to push the submit button.
Dec 12 2019
Add newlines to the end of the files.
Update code based on reviewer comments.
Dec 9 2019
Remove anonymus namespace
Fixes small things mentioned by reviewer commments.
Dec 8 2019
I run the new check on LLVM codebase with the option CharTypdefsToIgnore = "int8_t".
The check produced 12 findings.
I run the new check on LibreOffice codebase with the option CharTypdefsToIgnore = "sal_Int8".
The check produced 32 findings.
May 23 2019
May 22 2019
Link burprone module to cert module
May 21 2019
May 20 2019
May 12 2019
May 9 2019
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). The CERT alias is a bit more of a grey area -- I'd like to insist on it being added because it's trivial, it improves the behavior of this check by introducing an option some users will want, and checks conforming to coding standards are always more compelling than one-off checks. However, you seem very resistant to that and I don't want to add to your work load if you are opposed to doing it, so I don't insist on the change.
copy operator -> copy assignment operator
Changed "might not" to "does not" in the warning message.
Added the requested test case with the swapped template arguments.
May 5 2019
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), however, it does not conform with the corresponding cert rule. I expect that a cert alias with an option can be added in a follow-up patch (option to ignore ThisHasSuspiciousField pattern). The bugprone-* check would have the same default behavior as it has now in this patch. So adding the cert alias would not change this behavior, right? In this case, this code change can be added in a separate patch I guess.
Apr 28 2019
Better handling of template and non-template self-copy
Apr 25 2019
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.
Apr 24 2019
Remove outdated comment from docs.
Apr 23 2019
Mark some comments Done, which were handled some way.
I also reformatted the code with clang-format.
Make the check to work with templates too
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.
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
Apr 22 2019
Add false positive test cases.
Added a note about template related limitation to the docs.
Apr 16 2019
Update patch based on reviewer comments.