toolshed: tolerate anonymous struct renames in check_cython_abi.py#2242
Open
mdboom wants to merge 1 commit into
Open
toolshed: tolerate anonymous struct renames in check_cython_abi.py#2242mdboom wants to merge 1 commit into
mdboom wants to merge 1 commit into
Conversation
The name of an anonymous struct is an implementation detail, so it is
acceptable for the name to change between builds (e.g. from the old
anon_structXX / anon_unionXX scheme to the newer MODULE__anon_podXX
scheme that Cython now emits). This change makes the checker verify
that anonymous structs have the same *structure*, while tolerating name
changes, so spurious "Missing" / "Added" errors are no longer reported
for pure renames.
Three improvements:
1. `_format_base_type_name`: unwrap `CConstOrVolatileTypeNode` instead
of falling through to its class name. Const/volatile qualifiers do
not affect ABI layout, and the old behaviour stored the Python class
name literally ("CConstOrVolatileTypeNode*") in generated .abi.json
files, which caused field-type mismatches when comparing builds where
the qualifier was expressed differently.
2. `_build_anon_rename_map`: new iterative bottom-up pass that builds a
mapping from old-style anon names (expected) to new-style anon_pod
names (found) by matching field content. Leaf structs (no anon-type
fields) are matched first; each round normalises the remaining
candidates using already-known renames, allowing parent structs that
embed renamed children to match in subsequent rounds.
3. `_normalize_type`: uses `re.sub` with `\b` word-boundary anchors
instead of plain `str.replace`, so a shorter name like "anon_struct1"
cannot corrupt a longer one like "anon_struct12" if iteration order
happens to process the shorter key first.
`check_structs` is updated to look up each expected struct through the
rename map, normalise expected field types before comparing, and skip
the "Added" report for new-style names that are confirmed renames.
Note: baselines generated with the old tool (containing
"CConstOrVolatileTypeNode*" type strings) must be regenerated with the
fixed tool before the rename-matching logic can work correctly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The name of an anonymous struct is an implementation detail, so it is acceptable for the name to change between builds (e.g. from the old
anon_structXX/anon_unionXXscheme to the newerMODULE__anon_podXXscheme that Cython now emits). This checker now verifies that anonymous structs have the same structure, but tolerates name changes, so pure renames are no longer reported as errors.Three changes in
toolshed/check_cython_abi.py:_format_base_type_name: unwrapCConstOrVolatileTypeNodeinstead of falling through to its Python class name. Const/volatile qualifiers don't affect ABI layout; the old behaviour stored"CConstOrVolatileTypeNode*"literally in.abi.jsonfiles, causing spurious field-type mismatches across builds that expressed the qualifier differently._build_anon_rename_map: new iterative bottom-up pass that builds a mapping from old-style anon names (in the baseline) to new-styleanon_podnames (in the current build) by matching field content. Leaf structs are matched first; each round normalises remaining candidates using already-known renames, allowing parent structs that embed renamed children to be resolved in subsequent rounds._normalize_type: switched fromstr.replacetore.subwith\bword-boundary anchors, so a shorter name likeanon_struct1cannot corrupt a longer one likeanon_struct12due to iteration order.check_structsis updated to look up each expected struct through the rename map, normalise expected field types before comparing, and suppress the "Added" report for new-style names that are confirmed renames.Test plan
anon_structXXnaming schemecheckagainst a build that uses the newMODULE__anon_podXXnaming scheme🤖 Generated with Claude Code