[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D55869#1337706 <https://reviews.llvm.org/D55869#1337706>, @js wrote:

> Thanks for tagging me!
>
> The ObjFW runtime itself does not contain anything about release, retain or autorelease - these are all part of ObjFW (as in the framework). As ObjFW also supports the Apple runtime, as well as mixing with Foundation code, it so far only provides objc_retain and objc_release when they are missing, to make ARC work. They just call into the corresponding methods on the object and do nothing else.
>
> How will this change work from the Apple runtime? Will objc_retain/objc_release call the retain/release on the object if it implements its own? Should I drop my own retain/release implementation from my root object if I am compiling against the Apple runtime?


Nothing is fundamentally changing.  The Apple runtime has fast paths in `objc_retain` / `objc_release` for `NSObject`'s implementation of retain/release, and this patch allows us to take advantage of that in non-ARC files which explicitly call `-retain` and `-release`.  Of course, the runtime's own implementation of `objc_retain` and `objc_release` contains such calls, which would be miscompiled into recursive calls to `objc_retain` and `objc_release` without the command-line flag.

> I'm mostly concerned about mixing ObjFW and Foundation code while using the Apple runtime here. ObjFW with the ObjFW runtime and no Foundation is easy to change.

I assume you only define `objc_retain` and `objc_release` when compiling against your own runtime.  In that case, as Duncan reminded me, there's no risk of miscompiling as long as `shouldUseARCFunctionsForRetainRelease()` returns false for your runtime.  Since it sounds like your implementation doesn't try to avoid the message-send for common classes, there's no reason you would make that method return true; but if you ever want to, you'll need this compiler flag to avoid miscompiles.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55869/new/

https://reviews.llvm.org/D55869