[cfe-commits] [PATCH] Add analyzer_assert for regression tests

Jordan Rose jediknil at belkadan.com
Tue May 15 19:37:18 PDT 2012


On May 15, 2012, at 18:25, Anna Zaks wrote:
>> For the case of TRUE or FALSE there would be no change, but UNKNOWN is a problem. (In the previous iteration, these were separated into analyzer_assert and analyzer_assert_unknown, but I don't think it's necessary to check that something is true generally /and/ in a specific inlined case.)
> This checker might be useful to debug in presence of IPA. I though it's just the matter of having extra diagnostics - one per frame in which the call is evaluated. But maybe I am missing something..

Here is the actual case that came up (from dynamic-cast.cpp):

void testDynCastMostLikelyWillFail(C *c) {
  B *b = 0;
  b = dynamic_cast<B*>(c);
  clang_analyzer_eval(b); // expected-warning{{UNKNOWN}}
}

class M : public B, public C {};
void callTestDynCastMostLikelyWillFail() {
  M m;
  testDynCastMostLikelyWillFail(&m);
}

This seems like a reasonable use of clang_analyzer_eval to me -- the main thing being tested is that we haven't mistakenly assumed that 'c' is or isn't a 'B*'. When the call below is inlined, however, we know the cast succeeds.

While I'm going to follow your suggestion and not change dynamic-cast right now, this doesn't actually allow us to make useful assertions within inlined functions. Anything that ought to be true or false in the general case will be in the specific case as well, and as shown above, UNKNOWNs can change into knowns. And any function that gets inlined is still analyzed separately, at least for now. (I remember you were working on that already.)

On the other hand, the /calling/ function is perfectly able to assert results about the callee, which can be used to test specific cases. I think we should leave it as is, at least for now.

I'll clean up everything else and check it in, then!

Jordy





More information about the cfe-commits mailing list