<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 6, 2012, at 5:53 PM, Jordan Rose wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 6, 2012, at 16:25 , Anna Zaks <<a href="mailto:ganna@apple.com">ganna@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">+// Return a better dynamic type if one can be derived from the cast.<br>+// Compare the current dynamic type of the region and the new type to which we<br>+// are casting. If the new type is lower in the inheritance hierarchy, pick it.<br>+const ObjCObjectPointerType *<br>+DynamicTypePropagation::getBetterObjCType(const Expr *CastE,<br>+ CheckerContext &C) const {<br>+ const MemRegion *ToR = C.getSVal(CastE).getAsRegion();<br>+ assert(ToR);<br>+<br>+ // Get the old and new types.<br>+ const ObjCObjectPointerType *NewTy =<br>+ CastE->getType()->getAs<ObjCObjectPointerType>();<br>+ if (!NewTy)<br>+ return 0;<br>+ QualType OldDTy = C.getState()->getDynamicTypeInfo(ToR).getType();<br>+ if (OldDTy.isNull()) {<br>+ return NewTy;<br>+ }<br>+ const ObjCObjectPointerType *OldTy =<br>+ OldDTy->getAs<ObjCObjectPointerType>();<br>+ if (!OldTy)<br>+ return 0;<br>+<br>+ // Id the old type is 'id', the new one is more precise.<br>+ if (OldTy->isObjCIdType() && !NewTy->isObjCIdType())<br>+ return NewTy;<br>+<br>+ // Return new if it's a subclass of old.<br>+ const ObjCInterfaceDecl *ToI = NewTy->getInterfaceDecl();<br>+ const ObjCInterfaceDecl *FromI = OldTy->getInterfaceDecl();<br>+ if (ToI && FromI && FromI->isSuperClassOf(ToI))<br>+ return NewTy;<br>+<br>+ return 0;<br>+}<br></blockquote><div><br></div><div>Why is this here and not in addDynamicTypeInfo? (If there's a good reason that's fine; it just seemed odd.)</div></div></div></blockquote>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.</div><div><br></div><div>I'll rename addDynamicTypeInfo into setDynamicTypeInfo.</div><div><br></div><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><br></div><br><blockquote type="cite">+// Casting to parent should not pessimize the dynamic type. <br>++ (void) testCastToParent {<br>+ id a = [[self alloc] init];<br>+ MyParent *p = a; <br>+ clang_analyzer_eval([p getZeroOverridden] == 0); // expected-warning{{TRUE}}<br>+}<br>+<br>+// The type of parameter gets used.<br>++ (void)testTypeFromParam:(MyParent*) p {<br>+ clang_analyzer_eval([p getZero] == 0); // expected-warning{{TRUE}}<br>+}<br>+<br>+// Test implisit cast.<br>++ (void) testCastFromId:(id) a {<br>+ MyParent *p = a; <br>+ clang_analyzer_eval([p getZero] == 0); // expected-warning{{TRUE}}<br> }<br></blockquote><div><br></div>Typo: "implisit".</div><div><br></div></div></blockquote><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>This is also eventually the case where we should have a branch, since even if 'a' <i>is</i> 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.)</div></div></blockquote></div>Added the comment in r161391.</body></html>