[cfe-commits] r164868 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp test/Analysis/objc_invalidation.m

[Jordan Rose]

On Oct 1, 2012, at 11:03 , Anna Zaks <ganna at apple.com> wrote:
>> 
>>> +
>>> +bool IvarInvalidationChecker::MethodCrawler::isZero(const Expr *E) const {
>>> +  E = peel(E);
>>> +  if (const IntegerLiteral *IL = dyn_cast<IntegerLiteral>(E))
>>> +    return IL->getValue() == 0;
>>> +
>>> +  if (const CastExpr *ICE = dyn_cast<CastExpr>(E))
>>> +    return ICE->getCastKind() == CK_NullToPointer;
>>> +
>>> +  return false;
>>> +}
>> 
>> The more robust form of this is Expr::isNullPointerConstant.
>> 
> 
> isNullPointerConstant seems to do type checking, determining which type of null pointer we have. Why is it more robust to use here (all the typechecking has been done already)?

It catches a few other types of null pointer constants, like C++11 'nullptr'. It does do a small bit of extra work but reusing it ensures that we won't ever have to manually update this code.


>>> +
>>> +  // Check if we call the 'invalidation' routine on the ivar.
>>> +  if (Receiver) {
>>> +    InvalidationMethod = MD;
>>> +    check(Receiver->IgnoreParenCasts());
>>> +    InvalidationMethod = 0;
>> 
>> Passing state around like this does not make me happy.
> 
> It does not make me happy either.:) It does not make me more happy to pass this as a parameter through a bunch of calls.
> 
>> In particular, [[a foo] bar] will not work because the check() method calls back to VisitObjCMessageExpr.
> 
> It does not.

Oops, I can't read. In my defense the method names are a little confusing. Okay.