Skip to content

Commit aed4543

Browse files
committed
OSX/MACOS/COCOATOUCH: Fix build break from RARCH_AUTORELEASE misuse
0062608 (Make [CocoaView get] honour the +0 "get" convention) introduced two mistakes in the use of the RARCH_* retain/release macros in ui/drivers/cocoa/cocoa_common.m. Both are visible under any ARC build (i.e. every shipping Xcode project); the first is a hard error, the second is a -Wunused-value warning. RARCH_RETAIN / RARCH_RELEASE / RARCH_AUTORELEASE are defined in libretro-common/include/defines/cocoa_defines.h as follows: #if __has_feature(objc_arc) #define RARCH_RETAIN(x) (x) #define RARCH_RELEASE(x) ((void)0) #define RARCH_AUTORELEASE(x) ((void)0) ... #else #define RARCH_RETAIN(x) [(x) retain] #define RARCH_RELEASE(x) [(x) release] #define RARCH_AUTORELEASE(x) [(x) autorelease] #endif RARCH_RETAIN is expression-form: it evaluates to x (under ARC) or [x retain] (under MRR), and is meant to appear on the RHS of an assignment, e.g. _renderView = RARCH_RETAIN([CocoaView get]). RARCH_RELEASE and RARCH_AUTORELEASE are statement-only: their ARC expansion is ((void)0), which is not an lvalue or pointer rvalue and cannot participate in an expression. They must appear on their own line. ui_cocoa.m already follows this convention consistently (see e.g. RARCH_AUTORELEASE(metal_layer); after the CAMetalLayer alloc in -setViewType:). Site 1 - hard compile error: view = RARCH_AUTORELEASE([CocoaView new]); expands under ARC to `view = ((void)0);` which fails with "assigning to 'CocoaView *__strong' from incompatible type 'void'". Split into the two-line statement form so the assignment and the autorelease are independent: view = [CocoaView new]; RARCH_AUTORELEASE(view); Semantics are unchanged under both ARC and MRR: view receives +1 from +new, autorelease balances it after nsview_set_ptr has taken its own retain for g_instance. Site 2 - -Wunused-value warning: RARCH_RETAIN(p); expands under ARC to `(p);` which triggers -Wunused-value on any build that hardens with -Werror (or just clutters the log otherwise). Under MRR it expands to `[p retain];` which is a method call with a discarded return value and does not warn. Cast the RARCH_RETAIN call to (void) so both expansions discard the result explicitly: (void)RARCH_RETAIN(p); Retain semantics are unchanged under both ARC and MRR. Audited the rest of the tree; cocoa_common.m was the only offender. Every other RARCH_AUTORELEASE / RARCH_RELEASE call site already uses the statement form, and every RARCH_RETAIN call site either uses expression form (y = RARCH_RETAIN(x)) or was this one standalone use.
1 parent 08a6562 commit aed4543

1 file changed

Lines changed: 19 additions & 7 deletions

File tree

ui/drivers/cocoa/cocoa_common.m

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,18 @@ + (CocoaView*)get
213213
* returned pointer obeys the Cocoa +0 "get" convention on
214214
* both the first call (where we allocate) and every
215215
* subsequent call (where we just read g_instance) - so
216-
* callers do not have to guess the retain count. Under ARC
217-
* RARCH_AUTORELEASE is a no-op; the strong local's end-of-
218-
* scope release balances +new's +1 after g_instance's
219-
* storeStrong has taken its own retain. */
220-
view = RARCH_AUTORELEASE([CocoaView new]);
216+
* callers do not have to guess the retain count.
217+
*
218+
* RARCH_AUTORELEASE is a statement-only macro (expands to
219+
* ((void)0) under ARC and [x autorelease] under MRR), so it
220+
* must be called on its own line after the assignment rather
221+
* than wrapping the rvalue. Under ARC the ((void)0) is a
222+
* no-op and the strong local's end-of-scope release balances
223+
* +new's +1 after g_instance's storeStrong has taken its
224+
* own retain; under MRR the explicit autorelease does the
225+
* same balancing once the pool drains. */
226+
view = [CocoaView new];
227+
RARCH_AUTORELEASE(view);
221228
nsview_set_ptr(view);
222229
#if defined(IOS)
223230
view.displayLink = [CADisplayLink displayLinkWithTarget:view selector:@selector(step:)];
@@ -958,10 +965,15 @@ void nsview_set_ptr(CocoaView *p)
958965
* accessor, and that only holds if g_instance is the one keeping
959966
* the view alive. Under ARC RARCH_RETAIN and RARCH_RELEASE are
960967
* no-ops; the static __strong pointer does retain/release via
961-
* objc_storeStrong when assigned. */
968+
* objc_storeStrong when assigned.
969+
*
970+
* The (void) cast on RARCH_RETAIN silences -Wunused-value under
971+
* ARC, where the macro expands to the bare expression (p); under
972+
* MRR it expands to [p retain], where the discarded return value
973+
* is conventional and warning-free. */
962974
if (g_instance != p)
963975
{
964-
RARCH_RETAIN(p);
976+
(void)RARCH_RETAIN(p);
965977
RARCH_RELEASE(g_instance);
966978
g_instance = p;
967979
}

0 commit comments

Comments
 (0)