Fix @protocol methods so return values reach Objective-C

[akshayaurora]

Summary

• @protocol / forwardInvocation: previously discarded Python return values; they are now written onto the NSInvocation (@, integers, BOOL/char, float/double; void unchanged). Structs are still not forwarded.
• Register respondsToSelector: as B@:: (was wrongly v@::).
• Correct fallback protocols.py encodings: NSDraggingSource mask is Q (not @); 64-bit CGFloat methods use double (d), matching AppKit runtime.
• Refresh cached NSMethodSignatures when the resolved encoding changes (e.g. after a framework loads), so a stale float fallback cannot stick.
• Document supported return kinds and that AppKit/UIKit should be loaded before creating delegates.

Test plan

• python -m pytest tests/test_delegate_returns.py tests/test_delegate.py -v

[Copilot]

Pull request overview

This PR fixes Pyobjus’s dynamic @protocol delegate forwarding so that Python return values are propagated back to Objective-C callers via forwardInvocation: (instead of being discarded), and adds coverage/documentation for the supported return encodings.

Changes:

• Write Python return values back onto NSInvocation in protocol_forwardInvocation for object, integer, and BOOL/char return kinds.
• Correct runtime type encoding for respondsToSelector: and fix the fallback protocol encoding for NSDraggingSource.
• Add tests validating return forwarding behavior and update docs to describe supported return types.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_delegate_returns.py	Adds regression tests validating return values (object/bool/int/void) and respondsToSelector: behavior.
pyobjus/pyobjus.pyx	Implements return-value forwarding in forwardInvocation: and adjusts respondsToSelector: method registration encoding.
pyobjus/protocols.py	Fixes the fallback type encoding for NSDraggingSource mask return type.
pyobjus/common.pxi	Declares CoreFoundation retain/autorelease functions used for returned Objective-C objects.
docs/source/core_tutorials.rst	Documents which return encodings are currently forwarded back to Objective-C.

Suppressed comments (1)

pyobjus/pyobjus.pyx:1070

• The Objective-C type encoding used when registering respondsToSelector: should match the C return type of protocol_respondsToSelector (which is BOOL, typedef’d as signed char in common.pxi). Using "B@::" advertises _Bool/C++ bool, which is inconsistent with the actual implementation type; "c@::" matches BOOL and the runtime encoding used for many Cocoa APIs returning BOOL/signed char.

    class_addMethod(
        objc_cls, sel_registerName(b"methodSignatureForSelector:"),
        <IMP>&protocol_methodSignatureForSelector, "@@::")
    dprint('   register forwardInvocation:')

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

tests/test_delegate_returns.py:42

• ObjcBOOL is an Objective-C type marker (encoding c), not a value wrapper, so ObjcBOOL(True) does not reliably represent a boolean value (and ObjcBOOL(False) would still be truthy). For testing BOOL returns, return a real Python bool (or 0/1) from the delegate method so the forwarding logic is exercised correctly.

    @protocol('PyobjusReturnTest')
    def flagForKey_(self, key):
        self.log.append('flag')
        return ObjcBOOL(True)

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pyobjus/pyobjus.pyx:910

• protocol_forwardInvocation derives the return "kind" from signature.methodReturnType by taking the first byte. If the runtime encoding includes Objective-C type qualifiers (e.g. 'r', 'n', 'N', 'o', 'O', 'R', 'V'), the first byte will be a qualifier and the return value won't be forwarded (falls into the unhandled branch). Strip qualifiers before selecting the kind, consistent with clean_type_specifier usage elsewhere in the call machinery.

    ret_tp = signature.methodReturnType
    if ret_tp is not None:
        kind = ret_tp[:1]
        if isinstance(kind, unicode):
            kind = kind.encode('utf8')

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

pyobjus/pyobjus.pyx:947

• For unsupported return encodings, forwardInvocation currently only logs and leaves the NSInvocation return buffer untouched, so Objective-C callers can receive uninitialized garbage. At minimum, pointer-like return types should be explicitly zeroed (NULL) when not supported.

        ret_double = <double>(float(py_ret) if py_ret is not None else 0.0)
        inv.setReturnValue_(<unsigned long long>&ret_double)
    else:
        dprint('pfi: unhandled return type {!r}'.format(kind))

tests/test_delegate_returns.py:13

• Hardcoding '/usr/lib/libobjc.A.dylib' makes the test brittle across macOS variants/SDK layouts. Use ctypes.util.find_library('objc') to locate libobjc and load it from the resolved path.

OBJC = ctypes.cdll.LoadLibrary('/usr/lib/libobjc.A.dylib')

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

pyobjus/pyobjus.pyx:939

• The PR description says BOOL/char returns are supported, but c/C are currently treated as boolean truthiness (0/1). That loses non-boolean char values (and makes C behave like B). Split BOOL (B) from char (c/C) so char values round-trip correctly.

    elif kind in (b'B', b'c', b'C'):
        ret_bool = <unsigned char>(1 if py_ret else 0)
        inv.setReturnValue_(<unsigned long long>&ret_bool)

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pyobjus/pyobjus.pyx:977

• For unhandled return encodings (notably structs/unions/arrays), forwardInvocation currently leaves the NSInvocation return buffer untouched. That can surface as undefined/garbage return values to Objective-C callers. Consider zero-initializing a buffer of size signature.methodReturnLength and passing it to setReturnValue_ so unsupported returns deterministically come back as 0/NULL.

    else:
        dprint('pfi: unhandled return type {!r}'.format(kind))
        # Avoid leaving an uninitialized return buffer for pointer-sized
        # encodings (Class, SEL, C string, pointer, unknown).
        if kind in (b'#', b':', b'*', b'^', b'?'):
            ret_id = <id>NULL
            inv.setReturnValue_(<unsigned long long>&ret_id)

pyobjus/protocols.py:1401

• The 32-bit fallback encoding for NSAnimationDelegate -animation:valueForProgress: still treats the progress parameter as an object (@12). This parameter is a float, so the 32-bit encoding should use f12 to match the actual signature and avoid mis-decoding arguments on 32-bit runtimes.

        "animationDidEnd:": ("v12@0:4@8", "v24@0:8@16"),
        "animation:valueForProgress:": ("f16@0:4@8@12", "f28@0:8@16f24"),
        "animation:didReachProgressMark:": ("v16@0:4@8@12", "v32@0:8@16@24"),

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pyobjus/pyobjus.pyx:945

• The object-return path always does CFRetain + CFAutorelease on the return value. When py_ret is a Python scalar (e.g. str, int, float, bool), convert_py_to_nsobject creates a retained object via alloc/init..., so adding retain+autorelease leaves an extra retain (+1 leak). Consider only doing retain+autorelease when py_ret is already an ObjcClassInstance; for newly-created objects, just CFAutorelease to hand ownership to the autorelease pool (typical for delegate returns).

        obj = convert_py_to_nsobject(py_ret) if py_ret is not None else None
        ret_id = (<ObjcClassInstance>obj).o_instance if obj is not None else <id>NULL
        if ret_id != NULL:
            # Keep the returned object alive across the NSInvocation handoff.
            # CoreFoundation is linked on macOS by default; on iOS setup.py

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pyobjus/pyobjus.pyx:1154

• The Objective-C type encoding for respondsToSelector: should match BOOL as defined in objc/runtime.h (this codebase typedefs BOOL as signed char in pyobjus/common.pxi), which corresponds to type code 'c'. Using 'B' encodes C99/C++ bool instead and can produce an incorrect NSMethodSignature/introspection result for this method.

    # Encoding must be BOOL (B), matching -[NSObject respondsToSelector:].
    class_addMethod(
        objc_cls, sel_registerName(b"respondsToSelector:"),
        <IMP>&protocol_respondsToSelector, "B@::")

[akshayaurora]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)
pyobjus/pyobjus.pyx:1154

• The Objective-C type encoding for respondsToSelector: should match BOOL as defined in objc/runtime.h (this codebase typedefs BOOL as signed char in pyobjus/common.pxi), which corresponds to type code 'c'. Using 'B' encodes C99/C++ bool instead and can produce an incorrect NSMethodSignature/introspection result for this method.

    # Encoding must be BOOL (B), matching -[NSObject respondsToSelector:].
    class_addMethod(
        objc_cls, sel_registerName(b"respondsToSelector:"),
        <IMP>&protocol_respondsToSelector, "B@::")

The runtime encodes NSObject's respondsToSelector: as B24@0:8:16, confirming the type code is B.

Apple's NSObject returns BOOL (signed char), so the B encoding is correct. The protocol_respondsToSelector function returns signed char, matching the runtime.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

pyobjus/pyobjus.pyx:948

• This comment claims CoreFoundation is "linked on macOS by default", but setup.py only explicitly links CoreFoundation on iOS. To avoid misleading future maintainers (and accidental reliance on the Python executable’s link set), update the comment to reflect the intended linkage strategy.

            # CoreFoundation is linked on macOS by default; on iOS setup.py
            # adds -framework CoreFoundation.
            if already_objc:

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pyobjus/pyobjus.pyx:947

• The comment about CoreFoundation linking is now outdated: setup.py links CoreFoundation on both macOS (darwin) and iOS, not only iOS. Keeping this accurate avoids confusion when debugging link errors.

            # CoreFoundation is linked on macOS by default; on iOS setup.py
            # adds -framework CoreFoundation.

docs/source/core_tutorials.rst:714

• This new doc section mentions unsupported return encodings (structs, …), but the implementation also treats complex argument types (struct/union/array) as unsupported and passes them as None. Documenting this limitation will help users understand why some delegate methods receive None for struct args.

``unsigned char``, or ``float`` / ``double``). Supported return kinds today
are objects (``@``), common integer widths, ``BOOL``, ``char`` /
``unsigned char``, and ``f`` / ``d``; other encodings (structs, …) are not
forwarded yet.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tests/test_delegate_returns.py:16

• Import-time RuntimeError when libobjc is missing will fail the whole test suite on non-macOS environments. Other tests in this repo use conditional skips for darwin-only behavior; this module should skip instead of raising.

_objc_path = ctypes.util.find_library('objc')
if not _objc_path:
    raise RuntimeError('libobjc not found')
OBJC = ctypes.cdll.LoadLibrary(_objc_path)

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (4)

tests/test_delegate_returns.py:252

• This test also mutates the global protocols['NSTableViewDelegate'] mapping without restoring it, which can leak the poisoned encoding into other tests. Restore the original mapping via addCleanup() (or similar) to keep the suite order-independent.

        protocols['NSTableViewDelegate'] = dict(
            protocols.get('NSTableViewDelegate') or {})
        protocols['NSTableViewDelegate']['tableView:heightOfRow:'] = (
            'f16@0:4@8i12', 'f32@0:8@16i24')

tests/test_delegate_returns.py:186

• objc_msgSend should be called with a None (void) restype for void-returning methods. Declaring it as c_void_p can read garbage return registers and is undefined behavior in ctypes (and can crash on some ABIs).

    def test_msgsend_void_return(self):
        _msg_send(ctypes.c_void_p, self.target, 'voidForKey:', self.key)

tests/test_delegate_returns.py:236

• This test mutates the global protocols['NSTableViewDelegate'] mapping but never restores it, which can create order-dependent failures if other tests (or later code in this module) rely on the original encodings. Add a cleanup hook to restore the original value after the test.

This issue also appears on line 249 of the same file.

        # Poison the static fallback; runtime AppKit protocol must still win.
        protocols['NSTableViewDelegate'] = dict(
            protocols.get('NSTableViewDelegate') or {})
        protocols['NSTableViewDelegate']['tableView:heightOfRow:'] = (
            'f16@0:4@8i12', 'f32@0:8@16i24')

pyobjus/pyobjus.pyx:845

• The new signature-cache refresh logic (tracking _sigenc_... and rebuilding the NSMethodSignature when the encoding changes) isn’t currently exercised by tests. Consider adding a regression that (in a fresh subprocess or otherwise controlled environment) forces methodSignatureForSelector: to cache a fallback encoding before the framework is loaded, then loads the framework and verifies the cached signature is rebuilt with the runtime encoding.

    # Refresh the cached signature if the encoding changed (e.g. protocols.py
    # fallback used before the framework was loaded; runtime CGFloat is
    # double on 64-bit, while the static table may still say float).
    if hasattr(delegate, sig_name) and getattr(delegate, enc_name, None) == enc:
        sig = getattr(delegate, sig_name)
    else:
        NSMethodSignature = autoclass("NSMethodSignature")
        sig = NSMethodSignature.signatureWithObjCTypes_(enc)
        setattr(delegate, sig_name, sig)
        setattr(delegate, enc_name, enc)

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pyobjus/pyobjus.pyx:900

• Argument decoding in protocol_forwardInvocation uses tp[:1] for both libffi sizing and convert_cy_ret_to_py(). This drops pointee information for pointer types (e.g. '^i' becomes '^'), producing an ObjcReferenceToType with an empty of_type, and it can also lead to writing into a pointer-sized buffer even when the ffi type is unknown/oversized. Use the full encoding for pointer types and guard against NULL/oversized ffi types before calling getArgument_atIndex_.

        arg_type = type_encoding_to_ffitype(tp[:1])
        dprint('pfi: convert arg {} with type {}'.format(i, tp[:1]))
        c_arg = NULL
        inv.getArgument_atIndex_(<unsigned long long>&c_arg, i)
        py_arg = convert_cy_ret_to_py(&c_arg, tp[:1],

pyobjus/pyobjus.pyx:934

• Return-kind detection in protocol_forwardInvocation assumes clean_type_specifier() strips all ObjC qualifiers, but clean_type_specifier() only removes a single leading qualifier. Encodings with multiple qualifiers (e.g. 'rn@') would leave a qualifier as the kind and skip writing the Python return value back to the NSInvocation.

    kind = b'v'
    ret_tp = signature.methodReturnType
    if ret_tp is not None:
        if isinstance(ret_tp, unicode):
            ret_tp = ret_tp.encode('utf8')
        kind = clean_type_specifier(ret_tp)[:1]

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.