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 @@ +@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
