Skip to content

feat: Added typed method registration for py_class#567

Merged
junrushao merged 4 commits intoapache:mainfrom
Kathryn-cat:fix-typed-method
Apr 24, 2026
Merged

feat: Added typed method registration for py_class#567
junrushao merged 4 commits intoapache:mainfrom
Kathryn-cat:fix-typed-method

Conversation

@Kathryn-cat
Copy link
Copy Markdown
Contributor

Add type method registration on py_class

feat(dataclasses): add @method decorator for FFI TypeMethod registration

Closes the gap where @py_class-decorated Python classes couldn't
expose user-defined methods to the FFI reflection table. Trait
references like $method:NAME in __ffi_ir_traits__ now resolve
cleanly against Python-defined methods, on parity with C++-defined
refl::ObjectDef<T>().def("name", ...).

Architecture

  • tvm_ffi.method decorator stamps fn.__ffi_method__ = True on the
    decorated function (unwraps staticmethod to __func__ so both
    @method @staticmethod and @staticmethod @method work).
  • _collect_py_methods in py_class.py widens beyond the existing
    _FFI_RECOGNIZED_METHODS allowlist: it now also picks up any
    callable in cls.__dict__ carrying the __ffi_method__ marker
    via _is_method_marked. The allowlist still routes TypeAttrColumn
    dunders (__ffi_repr__, __ffi_ir_traits__, etc.) through
    TVMFFITypeRegisterAttr unchanged.
  • Marked methods register through the existing TypeMethod path
    (TVMFFITypeRegisterMethod) — same machinery the C++ side uses.
    The C++ printer's FindMethod (pyast_trait_print.cc:130-148) walks
    info->methods[] and discovers these entries identically to
    C++-defined methods, including ancestor-walking for inheritance.
  • _validate_method_name rejects @method decoration on:
    • __ffi_* reserved prefix (would double-register as TypeAttrColumn),
    • Python protocol dunders (__len__, __iter__, etc. — reserved
      for Python semantics),
    • @classmethod (the cls first arg breaks the packed-call
      convention; raises at decoration time).
  • Identity-typed: @method returns the original function unchanged
    (no wrapping); IDE / type-checker sees the method's signature
    unmodified.

Public Interfaces

  • tvm_ffi.method (re-exported from tvm_ffi.dataclasses)

Behavioral Changes

None for existing classes. Only NEW @method-decorated callables land
in TVMFFITypeInfo.methods[].

Tests

  • tests/python/test_typed_method.py (new, 13 tests): registration
    shape, FFI Function callability, validation errors.
  • tests/python/test_ir_traits.py (5 new tests): end-to-end via
    pyast.to_python() against $method: refs in AssignTraits.text_printer_post,
    `

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the @method decorator for explicit FFI TypeMethod registration, enabling C++ and Rust to resolve Python methods by name. It includes validation for reserved names and method types. Feedback suggests improving the decorator's robustness for callables with restricted attribute access and adding a redundant check for classmethods during registration to ensure FFI compatibility.

raise TypeError(
f"@tvm_ffi.method: expected a callable, got {type(fn).__name__}.",
)
fn.__ffi_method__ = True
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Setting attributes on arbitrary callables might fail for certain types (e.g., built-in functions or objects with __slots__ that don't include __dict__). While @method is primarily intended for user-defined functions in a class body, adding a check or a try-except block would make the decorator more robust against unexpected input types.

Comment on lines +334 to +335
is_static = isinstance(value, staticmethod)
func = value.__func__ if is_static else value
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for determining is_static and func assumes that any callable not wrapped in staticmethod is an instance method. However, if a user manually marks a classmethod (bypassing the decorator's check), is_static will be False and func will be the classmethod object itself. This might lead to unexpected behavior in the FFI packed-call convention which expects either a bound method or a plain function. Consider explicitly checking for and rejecting classmethod objects here as well, or ensuring the FFI layer handles them correctly.

Suggested change
is_static = isinstance(value, staticmethod)
func = value.__func__ if is_static else value
is_static = isinstance(value, staticmethod)
if not is_static and isinstance(value, classmethod):
raise TypeError(f"@py_class({cls.__name__}): classmethod {name!r} is not supported for FFI registration.")
func = value.__func__ if is_static else value

Copy link
Copy Markdown
Contributor Author

@Kathryn-cat Kathryn-cat Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in latest commit

@junrushao
Copy link
Copy Markdown
Member

Please fix the CI, and update docstring to be decoupled with not-yet-available features

@Kathryn-cat Kathryn-cat changed the title [Bug fix] Added typed method registration for py_class feat: Added typed method registration for py_class Apr 24, 2026
Copy link
Copy Markdown
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@junrushao junrushao merged commit 02a344d into apache:main Apr 24, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants