[PATCH] D91543: [clang-tidy] Improving bugprone-sizeof-expr check.
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 17 08:32:10 PST 2020
aaron.ballman added a comment.
In D91543#2400051 <https://reviews.llvm.org/D91543#2400051>, @balazske wrote:
> This checker has multiple weaknesses. There are more cases when the warnings should not appear (probably if the argument of `sizeof` is a template parameter), or more than one warning is generated for a code construct. The test code could be further improved too (but there are many cases to handle). This fix addresses only a single problematic case.
Sorry about being unclear -- I was intending to ensure we had test coverage for the changes in the patch. Specifically, the `ZeroLiteral` and `ArrayOfSamePointersExpr` matchers did not appear to have negative tests showing that we aren't diagnosing that code. I don't expect this patch to actually handle those cases (or for you to be on the hook for the fixes), it's more about documenting expectations.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:317
+ sum = sizeof(PtrArray) / sizeof(PtrArray1[0]);
+ // There is no warning for 'sizeof(T*)/sizeof(Q)' case.
+ sum += sizeof(PtrArray) / sizeof(A[0]);
----------------
Thanks for the new test case, can you also add one for `array[1]` (with the incorrect index) and add a FIXME to the comment so it's clear that we know we don't handle these cases currently but might like to someday?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91543/new/
https://reviews.llvm.org/D91543
More information about the cfe-commits
mailing list