[PATCH] D91543: [clang-tidy] Improving bugprone-sizeof-expr check.

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