Skip to content

gh-145497: Use same size of static_types array in all builds#149139

Open
encukou wants to merge 4 commits intopython:mainfrom
encukou:static_types-fixed-size
Open

gh-145497: Use same size of static_types array in all builds#149139
encukou wants to merge 4 commits intopython:mainfrom
encukou:static_types-fixed-size

Conversation

@encukou
Copy link
Copy Markdown
Member

@encukou encukou commented Apr 29, 2026

When someone adds a new type but doesn't increment _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES or
_Py_MAX_MANAGED_STATIC_EXT_TYPES, JIT tests fail, because JIT builds define an extra type.
But the JIT tests don't necessarily run for the commit that causes the failure,
so the issue only shows up in subsequent PR(s) that touch the JIT.

As a workaround, use the same size for the array for all builds, potentially with an empty spot.

When someone adds a new type but doesn't increment
`_Py_MAX_MANAGED_STATIC_BUILTIN_TYPES` or
`_Py_MAX_MANAGED_STATIC_EXT_TYPES`, JIT tests fail,
because JIT builds define an extra type.
But the JIT tests don't necessarily run for the commit
that causes the failure.

As a workaround, use the same size for the array for all
builds, potentially with an empty spot.
@read-the-docs-community
Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #32464303 | 📁 Comparing 541834a against main (d71e3bc)

  🔍 Preview build  

53 files changed · ± 52 modified · - 1 deleted

± Modified

- Deleted

Comment thread Objects/object.c Outdated
Comment thread Objects/object.c Outdated
@encukou
Copy link
Copy Markdown
Member Author

encukou commented Apr 29, 2026

Why not :)

Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well.

To be clear: If we forget to update _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES from now on, it won't be possible to merge the PR, right? (If not, that's fine -- I think we should do this regardless.)

@vstinner
Copy link
Copy Markdown
Member

To be clear: If we forget to update _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES from now on, it won't be possible to merge the PR, right?

Correct. If you add an entry to static_types without updating _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES, you get a compiler error and so the PR cannot be merged.

@@ -531,7 +531,9 @@ struct _py_func_state {

/* For now we hard-code this to a value for which we are confident
all the static builtin types will fit (for all builds). */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 83 still needs to be bumped if you add some categories of new types, right?

Would be useful to rewrite the comment to something like "If you add a new type, you may have to update some of these numbers".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants