cedarbdd / cedar

BDD-style testing using Objective-C
http://groups.google.com/group/cedar-discuss
1.19k stars 140 forks source link

Xcode 7.3 beta 5 errors on __weak in manual reference counting #376

Closed codeman9 closed 8 years ago

codeman9 commented 8 years ago

Specific error is in CDRSpyInfo.mm and says: "cannot create __weak reference in file using manual reference counting", this error can be turned off in the build settings, but then an error regarding "No matching function for call to objc_loadWeak" crops up (same error but also for objc_storeWeak)

Reference https://openradar.appspot.com/24791037

tjarratt commented 8 years ago

Thanks for opening this issue @codeman9 -- I hadn't been aware of this breakage in the betas.

I don't have an answer to this issue right now -- it looks like we'll need to either investigate moving this over to ARC (which may be very hard to get right), or find another way of creating a weak reference. It's a little frustrating that Apple seems to be implying that weak references never worked outside of ARC, because I'm fairly certain I've actually seen __weak resolve retain cycles when ARC wasn't being used. Maybe I'm wrong.

briancroom commented 8 years ago

My understanding of these statements is that the __weak attribute never actually did anything in MRC, which isn't surprising to me at all. In MRC all references are effectively weak anyway, it just means that the auto-zeroing behavior wasn't available unless you called down to objc_storeWeak and objc_loadWeak yourself, which is what Cedar is doing here.

I've pushed https://github.com/pivotal/cedar/commit/9ab7ca0a6ea1cfda8c0973f4679f0abbbbe85b8b which seems to resolve the issue by just removing the annotation. The variable names still contain the word weak so the intention is still reasonably documented. @codeman9 could you check and see if this works on your end?

akitchen commented 8 years ago

Under MRC references are assign, not weak -- an important difference if relying on the value of the pointer (null/not null). An assign reference can be non-null, but point to garbage, whereas weak is guaranteed to be nil or point to something valid.

I have a feeling there is reliance on this value distinction in the code these days, so we should be careful here.

On Sun, Mar 6, 2016 at 05:26 Brian Croom notifications@github.com wrote:

My understanding of these statements is that the __weak attribute never actually did anything in MRC, which isn't surprising to me at all. In MRC all references are effectively weak anyway, it just means that the auto-zeroing behavior wasn't available unless you called down to objc_storeWeak and objc_loadWeak yourself, which is what Cedar is doing here.

I've pushed 9ab7ca0 https://github.com/pivotal/cedar/commit/9ab7ca0a6ea1cfda8c0973f4679f0abbbbe85b8b which seems to resolve the issue by just removing the annotation. The variable names still contain the word weak so the intention is still reasonably documented. @codeman9 https://github.com/codeman9 could you check and see if this works on your end?

— Reply to this email directly or view it on GitHub https://github.com/pivotal/cedar/issues/376#issuecomment-192888315.

briancroom commented 8 years ago

Good call @akitchen. I think the salient point here though is that __weak under MRC has never actually provided zeroing weak references, they were still just standard assign references after all. That's why the code using these variables calls into objc_storeWeak and objc_loadWeak to manually invoke the runtime magic that provides the nil-after-dealloc behavior.

akitchen commented 8 years ago

Yup, I completely agree. I think the key word here is "magic"

On Sun, Mar 6, 2016 at 07:46 Brian Croom notifications@github.com wrote:

Good call @akitchen https://github.com/akitchen. I think the salient point here though is that __weak under MRC has never actually provided zeroing weak references, they were still just standard assign references after all. That's why the code using these variables calls into objc_storeWeak and objc_loadWeak to manually invoke the runtime magic that provides the nil-after-dealloc behavior.

— Reply to this email directly or view it on GitHub https://github.com/pivotal/cedar/issues/376#issuecomment-192918631.

codeman9 commented 8 years ago

@briancroom The changes you pushed removed the errors for me. Thanks!