gh-145497: Check _Py_NUM_MANAGED_STATIC_EXTRA_TYPES at runtime

[vstinner]

In debug mode, _Py_NUM_MANAGED_PREINITIALIZED_TYPES, _Py_NUM_STATIC_EXCEPTIONS and _Py_NUM_MANAGED_STATIC_EXTRA_TYPES are now checked with assertions at Python startup to detect if their values is outdated.

Add an array to count static extra types in init_static_type().

• Issue: Adjust _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES value automatically #145497

[vstinner]

@corona10: Would you mind to review my new PR? This is a new approach which actually counts all static types at Python startup (in debug mode) to make sure that we accounted all static types correctly.

3 macros are now checked at runtime: _Py_NUM_MANAGED_PREINITIALIZED_TYPES, _Py_NUM_STATIC_EXCEPTIONS and _Py_NUM_MANAGED_STATIC_EXTRA_TYPES.

What's left is _Py_MAX_MANAGED_STATIC_EXT_TYPES macro. I didn't understand the purpose of this macro. I don't know if it's related to the number of static types? If we also need to check that this macro is up to date, maybe a separated PR can be written.

[ZeroIntensity]

I think this needs to be wrapped in #ifdef Py_DEBUG as well.

Also, why is this in the header file in the first place? Why not just define it in exceptions.c?

[vstinner]

I think this needs to be wrapped in #ifdef Py_DEBUG as well.

This type is also used in release mode by Objects/exceptions.c.

Also, why is this in the header file in the first place? Why not just define it in exceptions.c?

The PR adds count_static_types() in Objects/typeobject.c which does access to extern struct static_exception *_Py_static_exceptions;: it has to know the structure members. Extract:

        for (size_t i=0; i < _Py_num_static_exceptions; i++) {
            if (type == _Py_static_exceptions[i].exc) {
                found = 1;
                break;
            }
        }

[ZeroIntensity]

Oh, I missed that it was an existing type. Can we rename it to _Py_static_exception or something like that? It's generally uncommon for our header files to expose names that aren't prefixed with _Py.

[vstinner]

Oh, my change just detected a regression in the main branch! The commit 0a6c1ed removed _PyAnextAwaitable_Type without updating _Py_NUM_MANAGED_PREINITIALIZED_TYPES. So the check failed when building Python, as expected:

_freeze_module: ../../Objects/object.c:2684: _PyTypes_InitTypes: Assertion `_Py_NUM_MANAGED_PREINITIALIZED_TYPES == Py_ARRAY_LENGTH(static_types)' failed.

In short, my change just works 😉

[corona10]

Sorry, I may be a bit biased, but when I started digging into this issue, I was hoping to find a solution based on compile-time computation. (If our codebase was based on C++ the solution might be easier, but we are using C..)
This approach relies on runtime computation, which slows down debug builds.

I think it comes down to which trade-offs we are willing to accept when comparing this approach with #151004. (compile time + verifialbe for whole builds)

[vstinner]

Sorry, I may be a bit biased, but when I started digging into this issue, I was hoping to find a solution based on compile-time computation.

The practical problem is that arrays in defined in two files, constants are defined in an internal header file, and constants are used in another file. If everything would be in the same place, it would be easier.

This approach relies on runtime computation, which slows down debug builds.

What do you mean by slowing down debug build? Arrays are quite small (around 100 items), and the check is only done once at startup. The impact on performance should not be significant.

[corona10]

So why the number of _Py_NUM_MANAGED_PREINITIALIZED_TYPES is decreased to 121?, it still needs to pay attention each number :(

[corona10]

Better way would be we don't need to designate the specific number but just register types into some place.

[vstinner]

So why the number of _Py_NUM_MANAGED_PREINITIALIZED_TYPES is decreased to 121?

A built-in type has been removed recently. When the type was removed, the constant was not updated. This PR detected that the constant is outdated, so I updated it.

Better way would be we don't need to designate the specific number but just register types into some place.

The number of static types and the number of static exceptions can be exported, my PR does exactly that in debug mode. But I don't see how to get the number of "static extra type" at build time. It would require a script to look for all built-in types and then exclude static types and static exceptions. It sounds complicated to do. Counting types at runtime in _PyStaticType_InitForExtension() and _PyStaticType_InitBuiltin() as does my PR looks more reliable to me, since all static types call these functions.

Adding or removing a type or exception is an uncommon operation, so I don't think that we need a too elaborated check for these. IMO this PR is good enough to solve the problem: detect when a constant is outdated.