[PATCH] D12889: [Static Analyzer] Generics Checker: When an ObjC method returns a specialized object, track it properly.

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:672
@@ +671,3 @@
+
+  if (IsDeclaredAsInstanceType)
+    return SelfType;
----------------
If you swap this 'if' with the 'if' above, you do not need to check for IsDeclaredAsInstanceType twice.

================
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:675
@@ +674,3 @@
+
+  QualType ResultType = Method->getReturnType().substObjCTypeArgs(
+      C, TypeArgs, ObjCSubstitutionContext::Result);
----------------
Could you use StaticResultType here?

================
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:795
@@ -817,1 +794,3 @@
+  // Checking that the return type is used correctly is done in pre-call to get
+  // cleaner diagnostic paths.
   checkReturnType(MessageExpr, *TrackedType, Sym, Method, *TypeArgs,
----------------
The path is longer if the call is inlined, correct? (Not much difference otherwise..)

================
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:803
@@ +802,3 @@
+/// initialized with by invoking the 'class' method on a class.
+/// This method is also used to infer the type information for the return
+/// types.
----------------
It looks like it's only inferring the type information of the specializations and not for non-generic types..

================
Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:865
@@ +864,3 @@
+  // When the result is a specialized type, track it for the result symbol.
+  State = State->set<TypeParamMap>(RetSym, ResultPtrType);
+  C.addTransition(State);
----------------
Please, rename the map to make it more explicit that it tracks the most specialized type. (I think I've mentioned it before in another review..)


http://reviews.llvm.org/D12889