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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
