tripleCC added a comment. In D152269#4402593 <https://reviews.llvm.org/D152269#4402593>, @steakhal wrote:
> I had some improvement opportunities in mind scattered, so I decided to do > them, and here is how the diff looks for me now: F27853795: > recommendation.patch <https://reviews.llvm.org/F27853795> > > Basically, I reworked the two branches a bit to express the intent more > strait forward. > I also enable all `osx.cocoa` checkers, as this file is all about objc stuff > anyway - this also meant to diagnose two non-fatal leak errors, which are not > tied to this patch otherwise. > > I'm also testing that the "assumption" after the objc call thingie (message > passing?) the pointer is assumed to be "nonnull". For this, I'm using the > `eagerly-assume=false` to ensure we don't split null and non-null states > eagerly when we encounter the `p == nil` inside `clang_analyzer_eval()` call > argument. > > I think all my changes are aligned with your intent, right? > > One more thing I was thinking of was to make this ObjC null checking checker > depend on the "nullability" checker package, but it turns out we can only > depend on individual checkers as of now. I tried some ideas there, e.g. > depending on the base checker of that package but it didn't work, so I > dropped the idea. (`clang/include/clang/StaticAnalyzer/Checkers/Checkers.td`) > > Do you plan to contribute more ObjC-related fixes in the future? > Please note that I have absolutely no experience with ObjC stuff. > > And I also wanted to thank you for the high-quality patches you submitted so > far! Yes, your changes are aligned with my intent. It seems like you have made excellent optimizations to this patch. To eliminate the following two warnings, I add the `-fobjc-arc` compilation option for NSContainers test . + MyView *view = b ? [[MyView alloc] init] : 0; // expected-warning {{Potential leak of an object stored into 'view'}} + NSMutableArray *subviews = [[view subviews] mutableCopy]; // expected-warning {{Potential leak of an object stored into 'subviews'}} I will continue to contribute more ObjC-related issue fixes in the future, and currently, my work is also related to this. Thank you very much for your review. Do you mind if I merge your recommendations? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152269/new/ https://reviews.llvm.org/D152269 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
