Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Declare constants as variables, instead of macros #37

Open
vstinner opened this issue Oct 27, 2023 · 8 comments
Open

Declare constants as variables, instead of macros #37

vstinner opened this issue Oct 27, 2023 · 8 comments
Labels
guideline To be included in guidelines PEP

Comments

@vstinner
Copy link
Contributor

vstinner commented Oct 27, 2023

Macros are not usable by some users: see issue #18. Should we define a guideline to declare new constants are variables instead of macros? Would such change have an impact on performance?

Example: python/cpython#111389 proposes adding 5 constants to the C API, they are just constant numbers.

@vstinner vstinner added the guideline To be included in guidelines PEP label Oct 27, 2023
@vstinner
Copy link
Contributor Author

PEP 670 – Convert macros to functions in the Python C API keeps existing constants as macros: https://peps.python.org/pep-0670/#convert-macros-to-static-inline-functions

@davidhewitt
Copy link

For Rust/PyO3, constants as macros are not too hard to map, because we have const items which behave similarly enough (however they are typed). A Rust source file auto-generated from a C header could easily have these.

So from a Rust perspective, for variables specifically, there's not a strong need to move away from macros. Other languages may differ of course.

@encukou
Copy link
Contributor

encukou commented Oct 30, 2023

If changing the value would break ABI, it can be compiled into the extension; getting it at runtime seems unnecessarily dynamic. It's OK to expose them “for build time” -- in headers for now, and eventually in an info file (see capi-workgroup/problems#7).

@vstinner
Copy link
Contributor Author

Is it possible to provide constants in two flavors, macro and regular C variable (symbol)? Like:

PyAPI_DATA(const unsigned long) Py_Version;
#define Py_Version PY_VERSION_HEX

The Py_Version was added to solve a problem: if a C extension is built with Python 3.12.0 but run with Python 3.12.1, it sticks to 3.12.0 if it uses PY_VERSION_HEX macro (copy/paste value), but get expected 3.12.1 if it uses Py_Version variable (value read at runtime).

So maybe we need to distinguish "constants" for which the value can change between the Python used to build a C extension and the Python used to run a C extension.

The problem is more visible when you consider the stable ABI. I don't expect constants to identical between Python 3.2 and Python 3.12.

Maybe we should only declare constants as variable in the stable ABI (limited C API), and add macro for the non-limited C API?

@encukou
Copy link
Contributor

encukou commented Oct 30, 2023

So maybe we need to distinguish "constants" for which the value can change between the Python used to build a C extension and the Python used to run a C extension.

Do you have any examples, other than the version?

@encukou
Copy link
Contributor

encukou commented Oct 30, 2023

Oh, I see _PyHASH_BITS &c.
IMO, those are not a good candidate for the stable ABI (yet) -- AFAICS, extensions that need these also reach rather deep into PyLong internals.

@vstinner
Copy link
Contributor Author

vstinner commented Sep 6, 2024

Let's keep the status quo for now. PyHash constants are implemented as macros. Examples:

#define PyHASH_MODULUS (((size_t)1 << _PyHASH_BITS) - 1)
#define PyHASH_INF 314159
#define PyHASH_IMAG PyHASH_MULTIPLIER

I close the issue.

@vstinner vstinner closed this as not planned Won't fix, can't repro, duplicate, stale Sep 6, 2024
@encukou
Copy link
Contributor

encukou commented Sep 9, 2024

There should be some guideline about this. I'd like to keep this open to track that.

@encukou encukou reopened this Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
guideline To be included in guidelines PEP
Projects
None yet
Development

No branches or pull requests

3 participants