4

I'm wondering is the variable declaration from the question topic is legitimate. Imagine the following code:

__weak typeof(self) weakSelf = self;
[self doSomethingThatMayCauseRetainCycleWithBlock:^{
    typeof(self) self = weakSelf; // <---- !!!!
    if (self == nil) return;

    NSAssert(self.someProperty != nil, @"This doesn't lead to retain cycle!");

    [self doSomething];
    self.someProperty = someValue;

    // even
    self->someIvar = anotherValue;
}

This code works perfectly in Xcode 4.5.2, only giving a warning that Declaration shadows a local variable.

What's the point of this quirk:

  1. Having redeclared self as a strong reference to a weak variable, you can safely copy/move code inside/outside the block without a risk to occasionally create a retain cycle (except for ivars, but they are evil).
  2. NSAssert in a block doesn't cause retain cycle anymore.

Update I discovered that this technique is used in libextobjc for @weakify/@strongify macros.

iHunter
  • 6,205
  • 3
  • 38
  • 56
  • Good source to learn about weakify & strongify : http://holko.pl/2015/05/31/weakify-strongify/ – lal Apr 25 '18 at 23:28

2 Answers2

5

There's two parts to this answer:

  1. The underlying cause of your Declaration shadows a local variable warning, GCC_WARN_SHADOW, and why it's probably a bad warning to have turned on.
  2. Building a replacement for NSAssert that doesn't need this trickery

GCC_WARN_SHADOW

The warning you're getting is from GCC_WARN_SHADOW, one of the more useless warnings (in my opinion, anyway). The compiler is doing the right thing here, but thanks to GCC_WARN_SHADOW it's drawing your attention to you possibly doing something other than you intended.

This is the sort of things the more paranoid compiler warnings are good at. The downside is that it's difficult (not impossible) to drop a particular warning to indicate that you know what you're doing.

With GCC_WARN_SHADOW on, this code will generate a warning:

int value = MAX(1,MAX(2,3));

Despite that, it will function perfectly.

The reason for this is that it compiles down to this, which is ugly but (to the compiler) perfectly clear:

({
   int __a = (1);
   int __b = (({
       int __a = (2);
       int __b = (3);
       __a < __b ? __b : __a;
   }));
   __a < __b ? __b : __a;
})

So what you're proposing will work well. NSAssert is implemented using a macro which uses the self variable. If you define self in your scope, it'll pick up that self instead.

Replacing NSAssert

But that said, if you think GCC_WARN_SHADOW has a use, there's another solution. It might be better anyway: Provide your own macro.

#define BlockAssert(condition, desc, ...) \
    do { \
        __PRAGMA_PUSH_NO_EXTRA_ARG_WARNINGS \
        if (!(condition)) { \
            [[NSAssertionHandler currentHandler] handleFailureInMethod:_cmd \
            object:strongSelf file:[NSString stringWithUTF8String:__FILE__] \
                lineNumber:__LINE__ description:(desc), ##__VA_ARGS__]; \
        } \
        __PRAGMA_POP_NO_EXTRA_ARG_WARNINGS \
    } while(0)

This is essentially a copy-paste of NSAssert, but it uses strongSelf instead of self. In places you use the strongSelf pattern, it'll work; in places where strongSelf is not defined, you'll get a compiler error: Use of undeclared identifier: 'strongSelf'. I think that's a pretty good hint.

Community
  • 1
  • 1
Steven Fisher
  • 44,462
  • 20
  • 138
  • 192
  • 1
    For those reading this excellent answer who aren't aware of the "strongSelf" pattern referred to, see this post and its comments: http://stackoverflow.com/a/17105368/796419 – smileyborg Dec 09 '13 at 21:59
  • Good link. I've added it to the answer. :) – Steven Fisher Dec 11 '13 at 17:51
  • Actually, the GCC_WARN_SHADOW warning can find some bugs that are otherwise notoriously hard to find. It's a warning that I wouldn't want to miss, and that I wouldn't give up over some slight inconvenience. – gnasher729 May 19 '14 at 18:11
  • Really? This gave a compiler warning due to GCC_WARN_SHADOW up until recently (and I don't think it's been fixed, though maybe it has): `MIN(3,MAX(1,4))`. Not to mention forcing you to name an error variable differently in blocks, which will result in bugs if you try to move code in and out of the blocks. It's bad warning. – Steven Fisher May 19 '14 at 23:45
3

Strictly speaking, there is nothing wrong with your code — the variable declaration is legitimate. However, you will probably get a compiler warning about the local variable shadowing the instance one.

GCD blocks are actually C functions, not Objective-C methods. When compiled, every Objective-C instance method has an extra parameter added to it, which is the self pointer. self isn't stored in the object struct like other variables.

For this reason, I would hesitate to use this code in a library I was going to share. The code may break with newer versions of then compiler because you're actually hacking the runtime a little more than is immediately apparent. Additionally, it's quirky code, as you point out :) I'm not sure that anyone else reading it would immediately understand what's going on.

Ash Furrow
  • 12,391
  • 3
  • 57
  • 92
  • Why would this be risky? Essentially all you're doing is relying on variable shadowing; if you have GCC_WARN_SHADOW turned on, you'll get a warning message. But variable shadowing is a legitimate technique, and it's well-understood and tested. – Steven Fisher Feb 21 '13 at 21:44
  • I never said risky - I said I would hesitate to do so because it may break in the future and other developers may misunderstand it. Maybe my first point isn't valid. – Ash Furrow Feb 22 '13 at 18:57
  • I think the only it really depends on the definition of NSAssert accessing self directly. That said, since posting this I thought of a better way to do this: define your own assert macro. No need for something that other developers might misunderstand. :) – Steven Fisher Feb 22 '13 at 19:07
  • Blocks are neither C functions nor objc methods - they are implemented on top of C functions, but so are objc methods. They are a class all to themselves, because they can also be invoked with an objective-c message (`-invoke`) – Richard J. Ross III Feb 22 '13 at 19:14
  • That's all true. NSAssert captures self rather than having self represent the block, though. – Steven Fisher Feb 22 '13 at 19:47
  • Variable shadowing _is_ risky, that's why you get the warning. The compiler (and maybe you) can never know which variable you meant when you write "self" - did you mean the one in your block or the one outside? – gnasher729 Mar 27 '14 at 13:19