tripleCC added a comment. In D152269#4403404 <https://reviews.llvm.org/D152269#4403404>, @steakhal wrote:
> In D152269#4403282 <https://reviews.llvm.org/D152269#4403282>, @tripleCC > wrote: > >> 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`(enable >> Automatic Reference Counting) 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 think we shouldn't add the `-fobj-arc` as these two new issues are > considered TPs, and meaningful for the test file they are part of. It's just > that they are not that meaningful in the scope of this patch, but that > shouldn't hold us back from improving what the test covers and demonstrates, > so I'm fine if those two appear as part of this patch. > >> I will continue to contribute more ObjC-related fixes in the future, and >> currently, my work is also related to this. > > Sounds good. Thanks for clarifying. > If that's the case, after a few more quality patches I think it would make > sense to request you commit access. Let's keep this in mind now. > >> Thank you very much for your review. Would you mind if I merge your >> recommendations? > > I don't mind. You don't need to give attribution. Keep parts of the whole as > you wish. Thanks. Additionally , there are already other unit tests focus on leak scenarios, such as the retain-release*.m test cases. Should we focus on testing container issues here? Currently, Objective-C code almost always uses ARC (Automatic Reference Counting). 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
