[PATCH] D39711: [analyzer] ObjCGenerics: Don't warn on cast conversions involving explicit cast

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:587
   if (TrackedType &&
+      !isa<ExplicitCastExpr>(CE) &&
       !ASTCtxt.canAssignObjCInterfaces(DestObjectPtrType, *TrackedType) &&
----------------
dcoughlin wrote:
> xazax.hun wrote:
> > george.karpenkov wrote:
> > > Should it check that we are actually casting to the right type? Also it's a bit strange that isa<> check on line 569 did not catch this case, maybe that if- branch should be generalized instead?
> > I agree, line 569 supposed to handle this case and also update the state accordingly.
> For suppressive casts, I am worried about updating the state to reflect destination type.
> 
> Programmers use a suppressive cast when they know a better invariant about the contents of a collection -- at a particular point in time -- than the analysis infers. It is not a promise that the invariant will hold at all later program points. I'm particularly worried that adding one suppressive cast would require the programmer to add other, different suppressive casts later in the program. For this reason I don't think it make sense to update the specialized type args map with the destination type of the cast.
> 
> Gabor: what do you think about the alternative of always removing the inferred specialized type args information for an unspecialized symbol on an explicit cast? After an explicit cast we would essentially treat the value as unspecialized and so not warn about later uses until the analyzer infers more information. Essentially this would be an acknowledgement that an explicit cast means the programmer had an invariant in mind that couldn't be represented in the type system and so the analyzer should back off.
Sounds reasonable, though actual examples would be even more helpful. 


https://reviews.llvm.org/D39711