martong added inline comments.
================
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:
> > 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.
> You can imagine duplicating all of this several times, since the constant in
> the subscript expression could also have different types, such as:
> `unsigned`, `long`, `unsigned long`, etc. Potentially uncovering bugs.
> However, I was reluctant to add those as well :|
>
> I could introduce a macro, to expand these - but IMO macros in tests ehh.
> probably reduces readability.
Macros, ehh.
Could it work if we instantiated a test function template with the types as
parameters? AFAIK, we analyze all instantiations as top level. The problem
seems to be with the `-verify` and matching the `expected-warning` for each
instantiation...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86870/new/
https://reviews.llvm.org/D86870
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
