[PATCH] D12123: [analyzer] Skip Pre/Post handlers for ObjC calls when receiver is nil.

[Ted Kremenek via cfe-commits]

krememek added a subscriber: krememek.
krememek added a comment.

I think this is a great refinement overall, with a few minor nits.  It also isn't clear what the test does.


================
Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:577
@@ -559,1 +576,3 @@
 
+  const std::vector<CheckObjCMessageFunc> &
+  getObjCMessageCheckers(ObjCCheckerKind Kind);
----------------
Can we break with tradition here and add a documentation comment?

================
Comment at: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:64
@@ -62,2 +63,3 @@
   void checkPreObjCMessage(const ObjCMethodCall &msg, CheckerContext &C) const;
+  void checkObjCMessageNil(const ObjCMethodCall &msg, CheckerContext &C) const;
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
----------------
Can we break with tradition here and actually add a small documentation comment?

================
Comment at: test/Analysis/NSContainers.m:297
@@ -294,1 +296,3 @@
 
+ at interface MyView : NSObject
+-(NSArray *)subviews;
----------------
Can we add a comment above this test that makes it clear what this test is doing?  It is not obvious at all from basic inspection.

There's also no matching 'no-warning' or 'expected-warning', so it is not clear at all what it is testing.  Having the comment clearly say what the test is testing will make it more resilient to somebody accidentally deleting it.


http://reviews.llvm.org/D12123