diff --git a/atom/atom.py b/atom/atom.py index 324c39c9..1ca95b74 100644 --- a/atom/atom.py +++ b/atom/atom.py @@ -38,21 +38,6 @@ class Atom(CAtom, metaclass=AtomMeta): """ - __atom_members__: ClassVar[Mapping[str, Member]] - - @classmethod - def members(cls) -> Mapping[str, Member]: - """Get the members dictionary for the type. - - Returns - ------- - result : Mapping[str, Member] - The dictionary of members defined on the class. User code - should not modify the contents of the dict. - - """ - return cls.__atom_members__ - @contextmanager def suppress_notifications(self) -> Iterator[None]: """Disable member notifications within in a context. diff --git a/atom/meta/atom_meta.py b/atom/meta/atom_meta.py index 24455a5e..25c1a406 100644 --- a/atom/meta/atom_meta.py +++ b/atom/meta/atom_meta.py @@ -28,6 +28,7 @@ from ..catom import ( CAtom, + CAtomMeta, DefaultValue, GetState, Member, @@ -63,7 +64,7 @@ def add_member(cls: "AtomMeta", name: str, member: Member) -> None: member.set_name(name) # The dict is mutable but we do not want to say it too loud - cls.__atom_members__[name] = member # type: ignore + cls.__add_member(name, member) # type: ignore cls.__atom_specific_members__ = frozenset( set(cls.__atom_specific_members__) | {name} ) @@ -501,7 +502,7 @@ def create_class(self, meta: type) -> type: return cls -class AtomMeta(type): +class AtomMeta(CAtomMeta): """The metaclass for classes derived from Atom. This metaclass computes the memory layout of the members in a given @@ -515,7 +516,6 @@ class so that the CAtom class can allocate exactly enough space for """ - __atom_members__: Mapping[str, Member] __atom_specific_members__: FrozenSet[str] def __new__( diff --git a/atom/src/catom.cpp b/atom/src/catom.cpp index f56542a3..0f1837d5 100644 --- a/atom/src/catom.cpp +++ b/atom/src/catom.cpp @@ -17,6 +17,7 @@ #include #include "atomref.h" #include "catom.h" +#include "catommeta.h" #include "globalstatic.h" #include "methodwrapper.h" #include "packagenaming.h" @@ -41,20 +42,17 @@ static PyObject* atom_flags; PyObject* CAtom_new( PyTypeObject* type, PyObject* args, PyObject* kwargs ) { - cppy::ptr membersptr( PyObject_GetAttr( pyobject_cast( type ), atom_members ) ); - if( !membersptr ) - return 0; - if( !PyDict_CheckExact( membersptr.get() ) ) - return cppy::system_error( "atom members" ); + if ( !CAtomMeta::TypeCheck( pyobject_cast(type) ) ) + return cppy::type_error("catommeta"); cppy::ptr selfptr( PyType_GenericNew( type, args, kwargs ) ); if( !selfptr ) return 0; CAtom* atom = catom_cast( selfptr.get() ); - uint32_t count = static_cast( PyDict_Size( membersptr.get() ) ); - if( count > 0 ) + + if( uint16_t count = catommeta_cast(type)->slot_count ) { - if( count > MAX_MEMBER_COUNT ) - return cppy::type_error( "too many members" ); + // As long as the types of CAtomMeta::slot_count and CAtom::slot_count + // are the same the `count < MAX_SLOT_COUNT` does not need checked here size_t size = sizeof( PyObject* ) * count; void* slots = PyObject_MALLOC( size ); if( !slots ) @@ -172,19 +170,29 @@ CAtom_set_notifications_enabled( CAtom* self, PyObject* arg ) PyObject* -CAtom_get_member( PyObject* self, PyObject* name ) +CAtom_get_member( PyObject* cls, PyObject* name ) { - if( !PyUnicode_Check( name ) ) - return cppy::type_error( name, "str" ); - cppy::ptr membersptr( PyObject_GetAttr( pyobject_cast( Py_TYPE(self) ), atom_members ) ); - if( !membersptr ) - return 0; - if( !PyDict_CheckExact( membersptr.get() ) ) - return cppy::system_error( "atom members" ); - cppy::ptr member( cppy::xincref( PyDict_GetItem( membersptr.get(), name ) ) ); - if( !member ) - Py_RETURN_NONE; - return member.release(); + if ( !CAtomMeta::TypeCheck(cls) ) + return cppy::type_error("get_members must be called on a CAtomMeta instance"); + return catommeta_cast( cls )->get_member(name); +} + + +PyObject* +CAtom_members( PyObject* cls ) +{ + if ( !CAtomMeta::TypeCheck(cls) ) + return cppy::type_error("members must be called on a CAtomMeta instance"); + return catommeta_cast( cls )->members(); +} + + +PyObject* +CAtom_init_subclass( PyObject* cls ) +{ + if ( !CAtomMeta::TypeCheck(cls) ) + return cppy::type_error("init_subclass must be called on a CAtomMeta instance"); + return catommeta_cast( cls )->init_subclass(); } @@ -412,13 +420,12 @@ CAtom_getstate( CAtom* self ) } } - cppy::ptr membersptr = selfptr.getattr(atom_members); - if ( !membersptr || !PyDict_CheckExact( membersptr.get() ) ) { - return cppy::system_error( "atom members" ); - } - + cppy::ptr membersptr( catommeta_cast( Py_TYPE(selfptr.get()) )->members() ); + if ( !membersptr ) + return 0; PyObject *name, *member; Py_ssize_t pos = 0; + utils::CriticalSection lock(membersptr.get()); while ( PyDict_Next(membersptr.get(), &pos, &name, &member) ) { cppy::ptr should_gs = member_cast( member )->should_getstate( self ); if ( !should_gs ) { @@ -490,8 +497,10 @@ CAtom_methods[] = { "Get whether notification is enabled for the atom." }, { "set_notifications_enabled", ( PyCFunction )CAtom_set_notifications_enabled, METH_O, "Enable or disable notifications for the atom." }, - { "get_member", ( PyCFunction )CAtom_get_member, METH_O, + { "get_member", ( PyCFunction )CAtom_get_member, METH_CLASS | METH_O, "Get the named member for the atom." }, + { "members", ( PyCFunction )CAtom_members, METH_CLASS | METH_NOARGS, + "Get members for the atom." }, { "observe", ( PyCFunction )CAtom_observe, METH_FASTCALL, "Register an observer callback to observe changes on the given topic(s)." }, { "unobserve", ( PyCFunction )CAtom_unobserve, METH_FASTCALL, @@ -510,6 +519,8 @@ CAtom_methods[] = { "The base implementation of the pickle getstate protocol." }, { "__setstate__", ( PyCFunction )CAtom_setstate, METH_O, "The base implementation of the pickle setstate protocol." }, + { "__init_subclass__", ( PyCFunction )CAtom_init_subclass, METH_CLASS | METH_NOARGS, + "Initialize the atom_members for the subclass." }, { 0 } // sentinel }; diff --git a/atom/src/catommeta.cpp b/atom/src/catommeta.cpp new file mode 100644 index 00000000..96ed26f3 --- /dev/null +++ b/atom/src/catommeta.cpp @@ -0,0 +1,257 @@ +/*----------------------------------------------------------------------------- + | Copyright (c) 2025, Nucleic Development Team. * + | + | Distributed under the terms of the Modified BSD License. + | + | The full license is in the file LICENSE, distributed with this software. + |----------------------------------------------------------------------------*/ +#include "catommeta.h" +#include "member.h" +#include "packagenaming.h" + +namespace atom +{ + +namespace +{ + +static PyObject* atom_members_str; + +static PyObject* invalid_members_error() +{ + return cppy::system_error("CAtomMeta members are not initialized. Subclasses that use __init_subclass__ must call super().__init_subclass__()"); +} + +void +CAtomMeta_clear( CAtomMeta* self ) +{ + Py_CLEAR( self->atom_members ); +} + +int +CAtomMeta_traverse( CAtomMeta* self, visitproc visit, void* arg ) +{ + Py_VISIT( self->atom_members ); + return 0; +} + +void +CAtomMeta_dealloc( CAtomMeta* self ) +{ + PyObject_GC_UnTrack( self ); + CAtomMeta_clear( self ); + Py_TYPE(self)->tp_free( pyobject_cast( self ) ); +} + +PyObject* +CAtomMeta_get_atom_members( CAtomMeta* self, void* context ) +{ + if ( !self->atom_members ) + return invalid_members_error(); + return cppy::incref( self->atom_members ); +} + +// Validates the members argument and returns the slot count required. +// If an error occurs it returns -1 and sets an error +int +CAtomMeta_validate_members( CAtomMeta* self, PyObject* members ) +{ + if ( !PyDict_CheckExact( members ) ) + { + cppy::type_error("CAtomMeta __atom_members__ must be a dict"); + return -1; + } + + PyObject *key, *value; + uint32_t count = 0; + Py_ssize_t pos = 0; + utils::CriticalSection lock( members ); + while ( PyDict_Next( members, &pos, &key, &value ) ) + { + if ( !PyUnicode_CheckExact( key ) ) + { + cppy::type_error("CAtomMeta __atom_members__ key must be a str"); + return -1; + } + if ( !Member::TypeCheck( value ) ) + { + cppy::type_error("CAtomMeta __atom_members__ value must be a Member"); + return -1; + } + + // Members that don't require storage can set the index over the limit + Member* member = reinterpret_cast( value ); + if ( member->index < MAX_MEMBER_COUNT ) + count += 1; + } + if (count > MAX_MEMBER_COUNT) + { + cppy::type_error("CAtomMeta __atom_members__ has too many members"); + return -1; + } + return count; +} + +int +CAtomMeta_set_atom_members( CAtomMeta* self, PyObject* members, void* context ) +{ + int count = CAtomMeta_validate_members( self, members ); + if ( count < 0 ) + return -1; + self->atom_members = cppy::incref( members ); + self->slot_count = count; + return 0; +} + +PyObject* +CAtomMeta_init_subclass( CAtomMeta* self ) +{ + // Since type_new does a copy of the attrs and does not use setattr + // we have to manually move __atom_members__ from the internal dict + // to the our managed self->atom_members pointer + if ( !self->atom_members ) + { + if ( PyObject** dict = _PyObject_GetDictPtr( pyobject_cast(self) ) ) + { + if ( PyObject* members = PyDict_GetItem( *dict, atom_members_str ) ) + { + if ( CAtomMeta_set_atom_members( self , members, 0 ) < 0 ) + return 0; + if ( PyDict_DelItem( *dict, atom_members_str ) < 0) + return 0; // LCOV_EXCL_LINE + } + } + + } + // If it was not set return an error + if ( !self->atom_members ) + return invalid_members_error(); + Py_RETURN_NONE; +} + +// Updates the slot +bool +CAtomMeta_update_slot_count( CAtomMeta* self ) +{ + if ( !self->atom_members ) + return invalid_members_error(); + int count = CAtomMeta_validate_members( self, self->atom_members ); + if ( count < 0 ) + return false; + self->slot_count = count; + return true; +} + +PyObject* +CAtomMeta_get_member( CAtomMeta* self, PyObject* name ) +{ + if ( !PyUnicode_Check(name) ) + return cppy::type_error("get_member name must be a str"); + if ( !self->atom_members ) + return invalid_members_error(); + PyObject* member = PyDict_GetItem( self->atom_members, name ); + if ( !member ) + Py_RETURN_NONE; + return cppy::incref(member); +} + +PyObject* +CAtomMeta_set_member( CAtomMeta* self, PyObject*const *args, Py_ssize_t n ) +{ + if ( n != 2 || !PyUnicode_Check(args[0]) || !Member::TypeCheck(args[1]) ) + return cppy::type_error("set_member requires 2 arguments name, member"); + if ( !self->atom_members ) + return invalid_members_error(); + if ( PyDict_SetItem( self->atom_members, args[0], args[1] ) < 0 ) + return 0; + if ( !CAtomMeta_update_slot_count( self ) ) + return 0; + Py_RETURN_NONE; +} + +PyObject* +CAtomMeta_del_member( CAtomMeta* self, PyObject* name ) +{ + if ( !PyUnicode_Check(name) ) + return cppy::type_error("del_member name must be a str"); + if ( !self->atom_members ) + return invalid_members_error(); + if ( PyDict_DelItem( self->atom_members, name ) < 0 ) + return 0; + if ( !CAtomMeta_update_slot_count( self ) ) + return 0; + Py_RETURN_NONE; +} + +static PyGetSetDef +CAtomMeta_getset[] = { + { "__atom_members__", ( getter )CAtomMeta_get_atom_members, ( setter )CAtomMeta_set_atom_members, + "Get and set the atom members." }, + { 0 } // sentinel +}; + +static PyMethodDef +CAtomMeta_methods[] = { + + { "get_member", ( PyCFunction )CAtomMeta_get_member, METH_O, + "Get the member with the given name." }, + { "__add_member", ( PyCFunction )CAtomMeta_set_member, METH_FASTCALL, + "Set the member with the given name." }, + { "__del_member", ( PyCFunction )CAtomMeta_del_member, METH_O, + "Delete the member with the given name." }, + { "members", ( PyCFunction )CAtomMeta_get_atom_members, METH_NOARGS, + "Get all the members." }, + { 0 } // sentinel +}; + +static PyType_Slot CAtomMeta_Type_slots[] = { + { Py_tp_dealloc, void_cast( CAtomMeta_dealloc ) }, + { Py_tp_traverse, void_cast( CAtomMeta_traverse ) }, + { Py_tp_clear, void_cast( CAtomMeta_clear ) }, + { Py_tp_methods, void_cast( CAtomMeta_methods ) }, + { Py_tp_getset, void_cast( CAtomMeta_getset ) }, + { Py_tp_free, void_cast( PyObject_GC_Del ) }, + { 0, 0 }, +}; + +} // namespace + +// Initialize static variables (otherwise the compiler eliminates them) +PyTypeObject* CAtomMeta::TypeObject = NULL; + +PyType_Spec CAtomMeta::TypeObject_Spec = { + .name = PACKAGE_TYPENAME( "CAtomMeta" ), + .basicsize = sizeof( CAtomMeta ), + .itemsize = 0, + .flags = Py_TPFLAGS_DEFAULT |Py_TPFLAGS_BASETYPE |Py_TPFLAGS_HAVE_GC, + .slots = CAtomMeta_Type_slots +}; + +bool CAtomMeta::Ready() +{ + atom_members_str = PyUnicode_InternFromString( "__atom_members__" ); + if ( !atom_members_str ) + return 0; + TypeObject = pytype_cast( PyType_FromSpecWithBases( &TypeObject_Spec, pyobject_cast( &PyType_Type ) ) ); + return TypeObject != 0; +} + +PyObject* +CAtomMeta::members( ) +{ + return CAtomMeta_get_atom_members( this, 0 ); +} + +PyObject* +CAtomMeta::get_member( PyObject* name ) +{ + return CAtomMeta_get_member( this, name ); +} + +PyObject* +CAtomMeta::init_subclass() +{ + return CAtomMeta_init_subclass( this ); +} + +} // namespace atom diff --git a/atom/src/catommeta.h b/atom/src/catommeta.h new file mode 100644 index 00000000..3d2b5612 --- /dev/null +++ b/atom/src/catommeta.h @@ -0,0 +1,42 @@ +/*----------------------------------------------------------------------------- + | Copyright (c) 2025, Nucleic Development Team. * + | + | Distributed under the terms of the Modified BSD License. + | + | The full license is in the file LICENSE, distributed with this software. + |----------------------------------------------------------------------------*/ +#pragma once + +#include +#include "platstdint.h" +#define catommeta_cast( o ) ( reinterpret_cast( o ) ) + + +namespace atom +{ + +// Base type for all CAtom classes +// see cpythons's testapi/heaptype.c' +struct CAtomMeta +{ + PyHeapTypeObject base; + PyObject* atom_members; + uint16_t slot_count; + + static PyType_Spec TypeObject_Spec; + static PyTypeObject* TypeObject; + static bool Ready(); + static int TypeCheck( PyObject* object ) + { + return PyObject_TypeCheck( object, TypeObject ); + } + + // All of these functions may return 0 and set an error + // if the class was not properly initialized + PyObject* init_subclass(); + PyObject* members(); // New reference. + PyObject* get_member( PyObject* name ); + +}; + +} diff --git a/atom/src/catommodule.cpp b/atom/src/catommodule.cpp index 8a4c56bf..a3c2fdf6 100644 --- a/atom/src/catommodule.cpp +++ b/atom/src/catommodule.cpp @@ -8,6 +8,7 @@ #include #include "behaviors.h" #include "catom.h" +#include "catommeta.h" #include "member.h" #include "memberchange.h" #include "eventbinder.h" @@ -55,6 +56,10 @@ bool ready_types() { return false; } + if( !CAtomMeta::Ready() ) + { + return false; + } if( !CAtom::Ready() ) { return false; @@ -130,6 +135,14 @@ bool add_objects( PyObject* mod ) } member.release(); + // CAtomMeta + cppy::ptr catommeta( pyobject_cast( CAtomMeta::TypeObject ) ); + if( PyModule_AddObject( mod, "CAtomMeta", catommeta.get() ) < 0 ) + { + return false; + } + catommeta.release(); + // CAtom cppy::ptr catom( pyobject_cast( CAtom::TypeObject ) ); if( PyModule_AddObject( mod, "CAtom", catom.get() ) < 0 ) diff --git a/atom/src/utils.h b/atom/src/utils.h index 057afdbc..b60d82ee 100644 --- a/atom/src/utils.h +++ b/atom/src/utils.h @@ -16,6 +16,21 @@ namespace atom namespace utils { +// Used to guard python functions that are not thread safe in the free-threaded build +struct CriticalSection +{ + CriticalSection(PyObject* obj) { +#if defined(Py_GIL_DISABLED) + Py_BEGIN_CRITICAL_SECTION(obj); +#endif + } + ~CriticalSection() { +#if defined(Py_GIL_DISABLED) + Py_END_CRITICAL_SECTION(); +#endif + } +}; + inline PyObject* py_bool( bool val ) { diff --git a/setup.py b/setup.py index 40d0893d..a42a37c1 100644 --- a/setup.py +++ b/setup.py @@ -25,6 +25,7 @@ "atom/src/atomset.cpp", "atom/src/atomref.cpp", "atom/src/catom.cpp", + "atom/src/catommeta.cpp", "atom/src/catommodule.cpp", "atom/src/defaultvaluebehavior.cpp", "atom/src/delattrbehavior.cpp", diff --git a/tests/test_atom.py b/tests/test_atom.py index 3a01f46c..840dc3af 100644 --- a/tests/test_atom.py +++ b/tests/test_atom.py @@ -350,9 +350,23 @@ class B(A): assert members == {"a": B.a} +def test_subclass_init_missing(): + with pytest.raises(SystemError): + + class A(Atom): + def __init_subclass__(cls) -> None: + # This is called when subclassing B + # Must call super or __atom_members__ is not there + cls.members() + + class B(A): + pass + + def test_clone_if_needed(): class A(Atom): def __init_subclass__(cls) -> None: + super().__init_subclass__() for m in cls.members().values(): clone_if_needed(cls, m)