2

I am seeing the condition described in this question but with an important caveat. Consider this code:

@implementation UIAlertView (Factory)

+ (instancetype)alertViewWithTitle:(NSString *)title
                           message:(NSString *)message
                          delegate:(id)delegate
                 cancelButtonTitle:(NSString *)cancelButtonTitle
                 otherButtonTitles:(NSArray *)otherButtonTitles
{
    UIAlertView *alertView = [[UIAlertView alloc] initWithTitle:title message:message delegate:delegate cancelButtonTitle:cancelButtonTitle otherButtonTitles:nil];
    for (NSString *otherButtonTitle in otherButtonTitles)
    {
        [alertView addButtonWithTitle:otherButtonTitle];
    }
    return alertView;
}

@end

If I do this:

@property (nonatomic, weak) UIAlertView *alertView;

...

self.alertView = [UIAlertView alertViewWithTitle:@"Hi" message:nil delegate:self cancelButtonTitle:@"OK" otherButtonTitles:nil];
[self.alertView show];

When I build in debug mode it works -- i.e. the factory method returns an autoreleasing pointer, so self.alertView has a value. Once it's shown, it's retained in the view hierarchy. This is not something I want to do, though -- this was a mistake and it reared its head as an actual problem when I built for release: the compiler (logically) appears to optimize away the assignment, leaving self.alertView equal to nil.

Even if I switch the analyzer to run on my release build it is not flagging this use of a weak pointer.

Is there a way to get the compiler / pre-compiler to flag this?

Community
  • 1
  • 1
Ben Flynn
  • 18,524
  • 20
  • 97
  • 142
  • 2
    You should be seeing this warning `Assigning retained object to weak property; object will be released after assignment`. – CrimsonChris May 27 '14 at 17:47
  • Just managed to reproduce your issue, very strange. I would expect it to be cleaned up. Something about it being a factory method is changing that. – CrimsonChris May 27 '14 at 17:56
  • possible duplicate of [ARC does not dealloc when pointer is set to nil (using factory methods)](http://stackoverflow.com/questions/14673523/arc-does-not-dealloc-when-pointer-is-set-to-nil-using-factory-methods) – CrimsonChris May 27 '14 at 17:58
  • @CrimsonChris I agree that is a related question, but does not address how I might get a warning when I write code that does this. – Ben Flynn May 27 '14 at 18:29
  • If the compiler is smart enough to optimize my code away, I'd think there would be a way for it to generate a warning, though perhaps this just doesn't exist yet... – Ben Flynn May 27 '14 at 18:32
  • It addresses why you are NOT getting the warning. – CrimsonChris May 27 '14 at 18:32
  • The compiler is only smart enough to optimize your code away because the method you are using starts with `init`. – CrimsonChris May 27 '14 at 18:55
  • But clearly the compiler is smart enough to optimize my code away, because that's what it does when I compile for release. It seems like the static analyzer could do perform a similar inspection. – Ben Flynn May 27 '14 at 19:27
  • It's not optimizing your code away in release, it's probably doing something like what I have in my answer. – CrimsonChris May 27 '14 at 19:31
  • 2
    The compiler has not optimized by making a deliberate choice to release the object earlier. Instead, the optimizations in a release build enable the `objc_autoreleaseReturnValue()` magic that avoids ever putting the object in the autorelease pool. So, the release that ARC was always putting into your code deallocates the object. There's no longer a reference outstanding in the autorelease pool. The compiler can warn when a +1 reference is assigned to a weak property, but yours is a +0 reference. If it were to warn about that, it would have to warn about *all* assignment to weak properties. – Ken Thomases May 27 '14 at 23:17
  • Here is a good article on objc_autoreleaseReturnValue() http://www.galloway.me.uk/2012/02/how-does-objc_retainautoreleasedreturnvalue-work/ So my assignment of *alertView to the factory is optimizing out the autorelease, so I get a simple retain count 1 on my object which then goes out of scope immediately after I assign it and disappears (the weak pointer nils it out). – Ben Flynn May 28 '14 at 00:52
  • That said, I can imagine a world in which the static analyzer could walk the code and see what was going to happen -- presumably any case where the next operation on the stack after assignment would reduce the retain count to zero. If I can read my code and see that problem brewing, I'm guessing an analyzer could. That said, it's at the least non-trivial and does not appear to exist. – Ben Flynn May 28 '14 at 00:56
  • Yes, it's conceivable but it would have to be whole-program analysis. I'm guessing that the category on `UIAlertView` is defined in a separate translation unit from the code that's assigning to the weak property. – Ken Thomases May 30 '14 at 13:35

1 Answers1

0

Add this to get the behavior you are expecting...

@autoreleasepool {
    self.alertView = [UIAlertView alertViewWithTitle:@"my alert view"];
}
[self.alertView show];

With ARC you are not guaranteed that your object will cleaned up immediately. The compiler doesn't know if your factory method is adding strong references. It could be.

This answer goes into more detail.

A static method is not always the same thing as an alloc init. The compiler can't tell the difference between +(id)makeMyObject and +(id)getMyObjectThatAlreadyExistsAndIsManagedBySomebodyElse.

Community
  • 1
  • 1
CrimsonChris
  • 4,651
  • 2
  • 19
  • 30
  • I don't see a warning, even when I do this. I only get a warning if I assign it to "[[UIAlertView alloc] init]". – Ben Flynn May 27 '14 at 18:31
  • You won't get a warning because `alertViewWithTitle` could be creating strong references behind the scenes. The compiler isn't going to analyze my custom factory method. – CrimsonChris May 27 '14 at 18:33
  • So your answer says "Add this to get the behavior you are expecting", but the behavior I am expecting is a warning, which this does not do. Moreover, it's not a reasonable solution to wrap every assignment in an autorelease pool. – Ben Flynn May 27 '14 at 18:38
  • I agree that you shouldn't use an autorelease pool here, this answer is attempting to explain why you CAN'T get the warning you are expecting. – CrimsonChris May 27 '14 at 18:39