On Aug 6, 2012, at 5:53 PM, Jordan Rose wrote:
>
> On Aug 6, 2012, at 16:25 , Anna Zaks <[email protected]> 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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
