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

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:587
   if (TrackedType &&
+      !isa<ExplicitCastExpr>(CE) &&
       !ASTCtxt.canAssignObjCInterfaces(DestObjectPtrType, *TrackedType) &&
----------------
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.


https://reviews.llvm.org/D39711