[cfe-commits] r161358 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp test/Analysis/inlining/ObjCDynTypePopagation.m

[Anna Zaks]

On Aug 6, 2012, at 5:53 PM, Jordan Rose wrote:

> 
> On Aug 6, 2012, at 16:25 , Anna Zaks <ganna at apple.com> wrote:
> 
>> +// Return a better dynamic type if one can be derived from the cast.
>> +// Compare the current dynamic type of the region and the new type to which we
>> +// are casting. If the new type is lower in the inheritance hierarchy, pick it.
>> +const ObjCObjectPointerType *
>> +DynamicTypePropagation::getBetterObjCType(const Expr *CastE,
>> +                                          CheckerContext &C) const {
>> +  const MemRegion *ToR = C.getSVal(CastE).getAsRegion();
>> +  assert(ToR);
>> +
>> +  // Get the old and new types.
>> +  const ObjCObjectPointerType *NewTy =
>> +      CastE->getType()->getAs<ObjCObjectPointerType>();
>> +  if (!NewTy)
>> +    return 0;
>> +  QualType OldDTy = C.getState()->getDynamicTypeInfo(ToR).getType();
>> +  if (OldDTy.isNull()) {
>> +    return NewTy;
>> +  }
>> +  const ObjCObjectPointerType *OldTy =
>> +    OldDTy->getAs<ObjCObjectPointerType>();
>> +  if (!OldTy)
>> +    return 0;
>> +
>> +  // Id the old type is 'id', the new one is more precise.
>> +  if (OldTy->isObjCIdType() && !NewTy->isObjCIdType())
>> +    return NewTy;
>> +
>> +  // Return new if it's a subclass of old.
>> +  const ObjCInterfaceDecl *ToI = NewTy->getInterfaceDecl();
>> +  const ObjCInterfaceDecl *FromI = OldTy->getInterfaceDecl();
>> +  if (ToI && FromI && FromI->isSuperClassOf(ToI))
>> +    return NewTy;
>> +
>> +  return 0;
>> +}
> 
> Why is this here and not in addDynamicTypeInfo? (If there's a good reason that's fine; it just seemed odd.)
I think it's much better to keep all the dynamic propagation details in one place. Also, if we wanted to add a check for dynamic type mismatch, it would go right into this method. It just makes sense to have it here.

I'll rename addDynamicTypeInfo into setDynamicTypeInfo.

> 
> 
>> +// Casting to parent should not pessimize the dynamic type. 
>> ++ (void) testCastToParent {
>> + id a = [[self alloc] init];
>> +  MyParent *p = a;  
>> +  clang_analyzer_eval([p getZeroOverridden] == 0); // expected-warning{{TRUE}}
>> +}
>> +
>> +// The type of parameter gets used.
>> ++ (void)testTypeFromParam:(MyParent*) p {
>> +  clang_analyzer_eval([p getZero] == 0); // expected-warning{{TRUE}}
>> +}
>> +
>> +// Test implisit cast.
>> ++ (void) testCastFromId:(id) a {
>> +  MyParent *p = a;  
>> +  clang_analyzer_eval([p getZero] == 0); // expected-warning{{TRUE}}
>> }
> 
> Typo: "implisit".
> 
> This is also eventually the case where we should have a branch, since even if 'a' is a MyParent instance, the implementation of -getZero should have been overridden. (With all the caveats about MyParent being declared in the main source file, but also having subclasses.)
Added the comment in r161391.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120806/18773517/attachment.html>