Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb updated this revision to Diff 53905. etienneb added a comment. rebased http://reviews.llvm.org/D19014 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/SizeofExpressionCheck.cpp clang-tidy/misc/SizeofExpressionCheck.h

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb updated this revision to Diff 53609. etienneb marked 2 inline comments as done. etienneb added a comment. alexfh@ comment: top-down approach. http://reviews.llvm.org/D19014 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb added inline comments. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:28 @@ +27,3 @@ + return Node.getValue().getZExtValue() > N; +} + alexfh wrote: > There are no firm rules. It mostly depends on how generic/useful in other > tools the matcher

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

malcolm.parsons added a subscriber: malcolm.parsons. Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:16 @@ +15,3 @@ +A common mistake is to query the ``sizeof`` of an integer literal. This is +equivalent to query the size of it's type (probably ``int``). The intent

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

Eugene.Zelenko added a subscriber: Eugene.Zelenko. Comment at: docs/ReleaseNotes.rst:100 @@ -99,1 +99,3 @@ +- New `misc-sizeof-expression + `_ check Please put this check after

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with one more nit. I'm not sure what to do with `hasSizeOfAncestor`, we can leave it as is for now. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:47 @@

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh added inline comments. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:198 @@ +197,3 @@ +} + +void SizeofExpressionCheck::check(const MatchFinder::MatchResult ) { I'm probably wrong about "a more effective approach in general", but for `sizeof` case

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb updated this revision to Diff 53442. etienneb marked an inline comment as done. etienneb added a comment. alexfh comments. http://reviews.llvm.org/D19014 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/SizeofExpressionCheck.cpp

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh added inline comments. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:28 @@ +27,3 @@ + return Node.getValue().getZExtValue() > N; +} + There are no firm rules. It mostly depends on how generic/useful in other tools the matcher could be. This one

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb added inline comments. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:27 @@ +26,3 @@ + +AST_MATCHER(BinaryOperator, isRelationnalOperator) { + return Node.isRelationalOp(); alexfh wrote: > alexfh wrote: > > nit: extra 'n' in "relational" > I'd

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb updated this revision to Diff 53437. etienneb marked 5 inline comments as done. etienneb added a comment. alexfh comments. http://reviews.llvm.org/D19014 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/SizeofExpressionCheck.cpp

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh added inline comments. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:197 @@ +196,3 @@ + this); +} + What about this comment? Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:203 @@ +202,3 @@ + if (const auto *E =

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb updated this revision to Diff 53436. etienneb added a comment. fix typos and doc. http://reviews.llvm.org/D19014 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/SizeofExpressionCheck.cpp clang-tidy/misc/SizeofExpressionCheck.h

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb updated this revision to Diff 53431. etienneb added a comment. more unittests http://reviews.llvm.org/D19014 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/SizeofExpressionCheck.cpp clang-tidy/misc/SizeofExpressionCheck.h

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb updated this revision to Diff 53429. etienneb added a comment. release notes. http://reviews.llvm.org/D19014 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/SizeofExpressionCheck.cpp clang-tidy/misc/SizeofExpressionCheck.h

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb updated this revision to Diff 53428. etienneb marked 6 inline comments as done. etienneb added a comment. fix alexfh@ comments http://reviews.llvm.org/D19014 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/SizeofExpressionCheck.cpp

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

alexfh added a comment. The check is really awesome! Thank you for coming up with all the patterns that frequently happen to result from coding errors! Please update release notes. A few inline comments as well. Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:22 @@

[PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

etienneb created this revision. etienneb added a reviewer: alexfh. etienneb added a subscriber: cfe-commits. This check found suspicious cases of sizeof expression. Sizeof expression is retuning the size (in bytes) of a type or an expression. Programmers often abuse of misuse this expression.