gh-86542: New C-APIs to simplify module attribute declaration by tiran · Pull Request #23286 · python/cpython

tiran · 2020-11-14T15:45:08Z

Check for reference leaks
verify refcounts.dat
add CAPI tests

Signed-off-by: Christian Heimes christian@python.org

https://bugs.python.org/issue42376

Issue: Add helpers to populate modules in C #86542

vstinner · 2020-11-23T10:22:07Z

Include/moduleobject.h

I'm not sure how to make this member future-proof in term of stable ABI.

We should try to put a max_align_t inside, but this type requires C11. GCC defines it with:

typedef struct { long long __max_align_ll __attribute__((__aligned__(__alignof__(long long)))); long double __max_align_ld __attribute__((__aligned__(__alignof__(long double)))); /* _Float128 is defined as a basic type, so max_align_t must be sufficiently aligned for it. This code must work in C++, so we use __float128 here; that is only available on some architectures, but only on i386 is extra alignment needed for __float128. */ #ifdef __i386__ __float128 __max_align_f128 __attribute__((__aligned__(__alignof(__float128)))); #endif } max_align_t;

Maybe we can at least put long long and long double:

// Unused members added to make PyModuleConst_Def large enough // to get a stable ABI and support future additions. long long m_long_long; long double m_long_double;

This is the only issue I found here :)
The stable ABI doesn't have a good story around evolving structs, and I think we should design a general mechanism for that rather than try to future-proof individual structs.
To move this PR forward, could we exclude PyModule_AddConstants & co. from the limited API for the time being?

vstinner · 2020-11-23T10:22:39Z

Include/modsupport.h

Could you start by adding these two functions in a separated PR, to make this PR shorter?

I don't really think that's necessary; the PR is not that big.

shihai1991 · 2020-11-26T12:02:48Z

Doc/c-api/module.rst

nitpick: static PyObject * in here.

shihai1991 · 2020-11-26T12:03:40Z

Modules/posixmodule.c

How about using another sepeareted PR to do this converation operation?

Objects/moduleobject.c

github-actions · 2020-12-27T00:35:27Z

This PR is stale because it has been open for 30 days with no activity.

Signed-off-by: Christian Heimes <christian@python.org>

Add helpers to create new type/exception and add it to the module dict in one call. Signed-off-by: Christian Heimes <christian@python.org>

Signed-off-by: Christian Heimes <christian@python.org>

This reverts commit 042e0999ed5ecf88de63de1140d968ab14745e3f.

encukou

This just needs a bit of polish, and checking the refleaks/refcounts as the OP suggests.
@tiran, do you plan to work on this? Should I take over?

encukou · 2021-11-16T15:30:06Z

Doc/c-api/module.rst

+   The values for *type* and the definition of the *value* union are
+   internal implementation details. Use any of the ``PyModuleConst_`` macros
+   to define entries. The array must be terminated by an entry with name
+   set to ``NULL``.
+
+   .. c:member:: const char *name
+
+      Attribute name.
+
+   .. c:member:: int type
+
+      Attribute type.
+
+   .. c:member:: void *value
+
+      Value of the module constant definition, whose meaning depends on
+      *type*.


Suggested change

The values for *type* and the definition of the *value* union are

internal implementation details. Use any of the ``PyModuleConst_`` macros

to define entries. The array must be terminated by an entry with name

set to ``NULL``.

.. c:member:: const char *name

Attribute name.

.. c:member:: int type

Attribute type.

.. c:member:: void *value

Value of the module constant definition, whose meaning depends on

*type*.

Definition of a module constant.

The members of this struct are internal implementation details.

To define entries, use only the ``PyModuleConst_`` macros below,

and ``{NULL}`` to mark the end of the array.

encukou · 2021-11-16T15:46:13Z

Python/modsupport.c

+}
+
+PyObject *
+PyModule_AddNewException(PyObject *module, const char *name, const char *doc,


These two functions need tests.

Hm, I see these return borrowed references (held by the module object). That saves a DECREF if you don't need the new object, but it'll be problematic for HPy.

Objects/moduleobject.c

encukou · 2024-01-11T09:51:41Z

I'll close this as it's not likely to get updated any time soon.
If anyone wants to pick this up, see the issue.

the-knights-who-say-ni added the CLA signed label Nov 14, 2020

bedevere-bot added the awaiting core review label Nov 14, 2020

tiran marked this pull request as draft November 14, 2020 15:45

tiran force-pushed the module_constants branch 3 times, most recently from ee79bab to ef08e94 Compare November 16, 2020 14:36

tiran changed the title ~~[PoC] Define module constants in PyModuleDef~~ bpo-42376: New C-APIs to simplify module attribute declaration Nov 16, 2020

tiran force-pushed the module_constants branch 2 times, most recently from 51767ca to bcd0d45 Compare November 16, 2020 20:56

tiran marked this pull request as ready for review November 17, 2020 07:33

tiran mentioned this pull request Nov 18, 2020

bpo-1635741: Port symtable module to multiphase initialization #23361

Merged

tiran force-pushed the module_constants branch 2 times, most recently from ed6c1b3 to 6de854c Compare November 22, 2020 13:27

vstinner reviewed Nov 23, 2020

View reviewed changes

tiran mentioned this pull request Nov 23, 2020

bpo-42327: Add PyModule_Add() (smaller). #23443

Merged

serhiy-storchaka self-requested a review November 23, 2020 10:43

shihai1991 reviewed Nov 26, 2020

View reviewed changes

github-actions bot added the stale Stale PR or inactive for long period of time. label Dec 27, 2020

tiran force-pushed the module_constants branch from 84759e5 to 3029bb4 Compare March 27, 2021 14:16

tiran added 10 commits April 17, 2021 12:19

Define module constants in PyModuleDef

436c1b0

Signed-off-by: Christian Heimes <christian@python.org>

Add PyModule_AddTypeFromSpec

d93f2da

PyModule_AddNew* helpers

a1c8240

Add helpers to create new type/exception and add it to the module dict in one call. Signed-off-by: Christian Heimes <christian@python.org>

Rename new API to PyModuleConst_

a277939

Signed-off-by: Christian Heimes <christian@python.org>

Use Py_mod_constants instead of PyModuleDef.m_constants

22e4f7f

Revert "Use Py_mod_constants instead of PyModuleDef.m_constants"

31f8189

This reverts commit 042e0999ed5ecf88de63de1140d968ab14745e3f.

Remove PyModuleDef.m_constants again

366e810

bpo-42376: New C-APIs to simplify module attribute declaration

bb41d82

ULong, private types, add module to call

751c560

Add CAPI tests

c34a4df

tiran added 3 commits April 17, 2021 12:19

Typo

3e50f44

Use single base class feature from bpo-42423

00236fe

It's a strong ref

8458120

tiran force-pushed the module_constants branch from 3029bb4 to 8458120 Compare April 17, 2021 10:20

encukou reviewed Nov 16, 2021

View reviewed changes

ezio-melotti removed the CLA signed label Jul 13, 2022

github-actions bot removed the stale Stale PR or inactive for long period of time. label Jul 31, 2022

erlend-aasland changed the title ~~bpo-42376: New C-APIs to simplify module attribute declaration~~ gh-86542: New C-APIs to simplify module attribute declaration Jun 22, 2023

erlend-aasland mentioned this pull request Jan 4, 2024

C API for simplifying module attribute declaration capi-workgroup/decisions#6

Closed

erlend-aasland marked this pull request as draft January 4, 2024 11:22

bedevere-app bot removed the awaiting core review label Jan 4, 2024

tiran mentioned this pull request May 13, 2023

Add helpers to populate modules in C #86542

Closed

encukou closed this Jan 11, 2024

Uh oh!

Conversation

tiran commented Nov 14, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Dec 27, 2020

Uh oh!

encukou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

encukou commented Jan 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tiran commented Nov 14, 2020 •

edited by bedevere-bot

Loading