[PATCH] D86870: [analyzer] Add more tests for ArrayBoundCheckerV2

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/test/Analysis/out-of-bounds-false-positive.c:34
+
+void symbolic_uint_and_int0(unsigned len) {
+  (void)a[len + 1]; // no-warning
----------------
steakhal wrote:
> martong wrote:
> > Hmm, this seems to be quite redundant with the `size_t` tests. Why is it not enough to have test for one unsigned type?
> > Are you trying to check for overflow errors? Then I'd expect to have indexes around UINT_MAX and so on. 
> > 
> > Same comment applies to the tests with the signed types.
> In the current implementation - and in any implementation of the checker logic will have to deal with //integral-promotion// during the //simplification// of the //array indexer expression// and the given //extent//.
> All of these can have different signess and bitwidth which makes the implementation quite tricky.
> 
> In fact, this resulted in the bug, which this patch-stack aims to fix.
> I'm gonna highlight the related parts in the refactoring patch if you think it helps.
Okay, makes sense. 

It's just very painful to have code repetitions, even in the test code. In gtest unittests we can have tests with type parameters to avoid this kind of repetition. But I guess, there is no way to get rid of this repetition in lit tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86870/new/

https://reviews.llvm.org/D86870