MAINT: refactor `scalartypes.c.src` to pure C++ #30361

scratchmex · 2025-12-03T04:05:33Z

remove begin repeat and use C macros. work towards #29528

for now I'm just refactoring to use macros and whenever the c++ templates are needed I will change to it. moving to C macros still has it's advantages cause I'm aiming for grepability and thus LSP advantages: Py@NAME@ArrType_Type vs PyNumberArrType_Type

the objective is to have a scalartypes.c or scalartypes.cpp

edit: the objective now is scalartypes.cpp

- DEF_PYARRTYPE - DEF_BINOP

…artypes

- DEF_BIT_COUNT - DEF_UN_OP

seberg · 2025-12-03T09:57:06Z

I think the helpful first step may to be convert the file to C++ to continue in steps from there. These custom macros seem a step in the wrong direction, since if we are to replace custom templating with C++ templating, we should use that.

scratchmex · 2025-12-03T19:38:26Z

I'm going to do that in another PR because apparently is not as trivial as I thought. converting from scalartypes.c.src to scalartypes.cpp.src

edit: I pushed the changes to review, and because this will have a dependency on it. Just confirm me if you want this in separate PR for easy review and to avoid noise on this

edit 2: well apparently for the migration to C++ in windows it requires C++ 20 :/. do you have a suggestion? I think it is not suitable to do it without designated initializers { .tp_name = val }

error C7555: use of designated initializers requires at least '/std:c++20'

edit 3: learning stuff: struct factories at top level and compile time. introduce make_pyarr_type

edit 4: removed all designated initializers just trying to make it compile in MSVC so that I can mark as "completed" the migration to C++.

> warning: ISO C++11 does not allow conversion from string literal to 'char *' in python >=3.13 they changed PyArg_ParseTupleAndKeywords to have char *const* in C++, instead of char** ref: https://docs.python.org/3/c-api/arg.html#c.PyArg_ParseTupleAndKeywords TODO: remove these (char **) castings when we drop support for 3.12

…ake_pyarr_type`

this should be reverted when moving to c++20 in MSVC

scratchmex · 2025-12-04T02:00:50Z

nicee. finally C++ compiles jaja!

Just confirm me if you want the migration to C++ in a separate PR for easy review and to avoid noise on the removal of .src x2

edit 5: cannot make factories for array of methods because a) trying to use static variables inside a constexpr is not allowed and b) returning a stack address associated with a local variable is not something we want.

a) definition of a static variable in a constexpr function is a C++23 extension
b) address of stack memory associated with local variable 'methods' returned

because of that, I will just repeat the definition of those

edit 6: cannot use constexpr std::map until C++26 to avoid using a chain of if-else constexpr

edit 7: cannot use template <const char* mystr> to create function factories. nor I can make a function return another function (lambda). will need to make definition of each function that wraps the generic one

…ble)

seberg · 2025-12-05T09:15:37Z

I think it is not suitable to do it without designated initializers { .tp_name = val }

I think they are allowed to be written as long as they are in the exactly correct order? I am not sure how many holes they have, this is one annoyance with C++. If there are few we might also just fill them in explicitly, otherwise, I guess this tricky is fine.

Maybe it makes sense to keep going here for now and then see if we want to split out just the initial conversion for example. (A bit conflicting, I would prefer to split it out, but if we don't follow through with the full thing there is no point.)

For what it's worth, I think the changes you did beyond the main change are not far reaching enough to be nice, we need to go all-in on templates for things to make sense. (Almost) anything repeated should be a template instead, the bufferprocs are a good example, that should just be something like bufferprocs<typename T>.
(The gentype version is then the overload that kicks in if a more specialized version is not defined.)

EDIT: CC @athurdekoos for awareness (and maybe if you want to even review).

scratchmex · 2025-12-05T22:25:46Z

the bufferprocs are a good example, that should just be something like bufferprocs<typename T>

you want

static auto unicode_arrtype_as_buffer = generic_pybufferprocs<unicode_getbuffer>;
static auto bool_arrtype_as_buffer = generic_pybufferprocs<dunder_buffer_impl1<npy_bool, PyBoolScalarObject>>;

instead of

static auto unicode_arrtype_as_buffer = make_pybufferprocs(unicode_getbuffer);
static auto bool_arrtype_as_buffer = make_pybufferprocs(dunder_buffer_impl1<npy_bool, PyBoolScalarObject>);

?

seberg · 2025-12-06T08:15:50Z

No, unicode_arrtype_as_buffer need not exisst at all.

scratchmex · 2025-12-06T14:46:15Z

unicode_arrtype_as_buffer needs to live in the stack at the top level scope because later its pointer is passed to their type definition like this

// that's why I do
static auto unicode_arrtype_as_buffer = generic_pybufferprocs<unicode_getbuffer>;  // << top level
PyUnicodeArrType_Type.tp_as_buffer = &unicode_arrtype_as_buffer;
// instead of
PyUnicodeArrType_Type.tp_as_buffer = &make_pybufferprocs(unicode_getbuffer);

and doing something like

PyUnicodeArrType_Type.tp_as_buffer = &arrtype_as_buffer<unicode_getbuffer>

I wasn't able to do it yet. Can you give an example for that?

by adding dependent_false_v. see issue CWG2518 [1] [2] [3] [1]: https://cplusplus.github.io/CWG/issues/2518.html [2]: https://reviews.llvm.org/D144285 [3]: https://en.cppreference.com/w/cpp/language/if.html#Constexpr_If

I cherry picked the changes to be mostly if-else chains and function definitions

scratchmex · 2025-12-07T00:21:36Z

I think the PR can be taken a first look. Remarking the following points

The most difficult part of the migration was the __new__ impls block, I would like a review on that
I want to get rid of the macros but I don't find a better way to do it right now
seberg comment about assigning a pointer to a templated struct. I don't know how to do it

ccing the top 3 commiters of scalartypes.c.src @seberg @ngoldbaum @mtsokol

scratchmex · 2025-12-07T00:30:20Z

numpy/_core/src/multiarray/scalartypes.cpp

+    PyMem_Free(ip);
+    if (new_ == NULL) {
        return NULL;
    }


Suggested change

PyMem_Free(ip);

if (new_ == NULL) {

return NULL;

}

if (new_ == NULL) {

PyMem_Free(ip);

return NULL;

}

ngoldbaum · 2025-12-07T01:26:45Z

ping @athurdekoos as well

ngoldbaum

I skimmed over most of the changes in scalartypes.cpp and left a few comments. No careful review here.

ngoldbaum · 2025-12-09T15:49:41Z

numpy/_core/src/multiarray/scalartypes.cpp

+        npy_type scalar = PyArrayScalar_VAL(self, ScalarName);                     \
+        uint8_t count = npy_popcount_func(scalar);                                 \
+        return ConvertFrom(count);                                                 \
+    }


can't this be a template function?

ngoldbaum · 2025-12-09T15:51:35Z

numpy/_core/src/multiarray/scalartypes.cpp

+gentype_squeeze(PyObject *self, PyObject *args, PyObject *kwds)
+{
+    return gentype_generic_method(self, args, kwds, "squeeze");
+}


given that it's trivial it's probably better to make this a macro though, it'll be a lot more concise.

ngoldbaum · 2025-12-09T15:52:45Z

numpy/_core/src/multiarray/scalartypes.cpp

+        (PyCFunction)npy_int_bit_count,
+        METH_NOARGS, NULL},
+    {NULL, NULL, 0, NULL} /* sentinel */
+};


I like that these are all explicitly defined in the source now, it's OK that it's more verbose.

ngoldbaum · 2025-12-09T15:54:09Z

numpy/_core/src/multiarray/scalartypes.cpp

+    else if constexpr (std::is_same_v<PT, PyCDoubleScalarObject>) {
+        return "Zd";
+    }
+    else if constexpr (std::is_same_v<PT, PyCLongDoubleScalarObject>) {


general question about code like this from a non-expert in C++, code like this using constexpr is evaluated at compile-time, right?

seberg · 2025-12-09T16:27:53Z

seberg comment about assigning a pointer to a templated struct. I don't know how to do it

Maybe check out what jax/ml_dtypes is doing and see if you get inspired from a pattern. I don't really want to try around right now, but I think this has to happen in some form.
I would hope all explicit instantiation with a name is unnecessary with the exception a single global type probably.
(In part, focusing on this type of discussion is why it might be helpful to do just the conversion first.)

remove begin repeat and use macros

370daf3

- DEF_PYARRTYPE - DEF_BINOP

github-actions bot added the 03 - Maintenance label Dec 3, 2025

scratchmex added 2 commits December 2, 2025 22:43

Merge remote-tracking branch 'upstream/main' into numpygh-29528--scal…

df1167e

…artypes

use macros for bit count and unary defs

dba6e5a

- DEF_BIT_COUNT - DEF_UN_OP

scratchmex added 3 commits December 3, 2025 14:07

migrate scalartypes.c.src to C++ (for now scalartypes.cpp.src)

9e7ec22

learning struct factories at top level and compile time. introduce `m…

69fa910

…ake_pyarr_type`

scratchmex force-pushed the gh-29528--scalartypes branch from 567fb25 to 69fa910 Compare December 3, 2025 23:01

remove all designated initializers to make MSVC happy

b09125a

this should be reverted when moving to c++20 in MSVC

scratchmex force-pushed the gh-29528--scalartypes branch from d4daaff to b09125a Compare December 4, 2025 01:17

missing extern "C" for MSVC

3ed09f5

scratchmex added 4 commits December 4, 2025 14:25

convert all pymethoddefs and its methods to c++ templates (when possi…

86096b3

…ble)

refactor buffer protocol to use c++ templates

3c1499b

remove constexpr to make MSVC happy (again)

d0eafef

Merge remote-tracking branch 'upstream/main'

99a6b4d

refactor str and repr impls to use c++ templates

5a3a5a7

scratchmex added 9 commits December 5, 2025 16:48

fix py funcs signature to have all make factories constexpr

78201cb

expand all templating from initialize_numeric_types to explicit defs

2f158c3

simplify (a lot) tp_as_number definitions

09264f0

expand some generic method definitions

86f7ba9

refactor dunder index and hash to use c++ templates

cf47013

expand scalar kinds table

7ea0cfb

refactor scalarobj definitions with struct factory and c++ templates

0e0d841

refactor dunder new impls to use c++ templates

3af9ac3

rename scalartypes.cpp.src to scalartypes.cpp (pure c++ YEAH)

029e65c

fix GCC conflicts

c233cb5

scratchmex added 3 commits December 6, 2025 09:06

run clang-format (not all changes)

140a199

I cherry picked the changes to be mostly if-else chains and function definitions

missing constexpr

b13e002

scratchmex commented Dec 7, 2025

View reviewed changes

scratchmex marked this pull request as ready for review December 7, 2025 00:30

scratchmex changed the title ~~MAINT: refactor scalartypes.c.src to avoid using custom metaprogramming~~ MAINT: refactor scalartypes.c.src to pure C++ Dec 7, 2025

ngoldbaum reviewed Dec 9, 2025

View reviewed changes

Uh oh!

MAINT: refactor scalartypes.c.src to pure C++ #30361

Are you sure you want to change the base?

MAINT: refactor scalartypes.c.src to pure C++ #30361

Conversation

scratchmex commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seberg commented Dec 3, 2025

Uh oh!

scratchmex commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scratchmex commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seberg commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scratchmex commented Dec 5, 2025

Uh oh!

seberg commented Dec 6, 2025

Uh oh!

scratchmex commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scratchmex commented Dec 7, 2025

Uh oh!

scratchmex Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

ngoldbaum commented Dec 7, 2025

Uh oh!

ngoldbaum left a comment

Choose a reason for hiding this comment

Uh oh!

ngoldbaum Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

ngoldbaum Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

ngoldbaum Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

ngoldbaum Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

seberg commented Dec 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MAINT: refactor `scalartypes.c.src` to pure C++ #30361

MAINT: refactor `scalartypes.c.src` to pure C++ #30361

scratchmex commented Dec 3, 2025 •

edited

Loading

scratchmex commented Dec 3, 2025 •

edited

Loading

scratchmex commented Dec 4, 2025 •

edited

Loading

seberg commented Dec 5, 2025 •

edited

Loading

scratchmex commented Dec 6, 2025 •

edited

Loading