Skip to content

fix(mypyc): preserve inherited class attribute defaults under separate=True#10

Merged
georgesittas merged 2 commits into
release-2.1from
fix/incremental-defaults-setup-chain
Jun 4, 2026
Merged

fix(mypyc): preserve inherited class attribute defaults under separate=True#10
georgesittas merged 2 commits into
release-2.1from
fix/incremental-defaults-setup-chain

Conversation

@georgesittas

@georgesittas georgesittas commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Under separate=True, find_attr_initializers walked the full MRO and pulled default-value AssignmentStmt nodes out of info.defn.defs.body. After mypy loads a base class from its incremental cache, that body is always empty (ClassDef.serialize deliberately doesn't serialize defs). When the subclass got recompiled but the parent stayed cache-loaded, the subclass's emitted __mypyc_defaults_setup silently dropped every inherited initialization. Any access to an inherited attribute through compiled code then raised:

AttributeError: attribute '<name>' of '<base>' undefined

That's the bug that has been intermittently breaking sqlglot's CI partial rebuilds and producing wheels that crash at runtime on simple attribute reads (e.g. if self.ENSURE_BOOLS: inside Generator.preprocess on a freshly-rebuilt Snowflake instance).

Change

find_attr_initializers + generate_attr_defaults_init now switch on builder.options.separate:

  • separate=True: each class's __mypyc_defaults_setup sets only that class's own defaults, then calls the nearest ancestor's __mypyc_defaults_setup to fold in inherited ones (same shape as Python's __init__ chain). The chain is robust to cache-loaded bases because it doesn't need their AST — only that the ancestor has a method_decl entry for __mypyc_defaults_setup, which the cached ClassIR retains.
  • separate=False: original inline-everything behavior preserved as an optimization (the AST is always in memory, MRO walk is safe, chain call would be needless runtime overhead).

Aligns with @p-sawicki's suggestion in python/mypy#21542.

Trade-off

A new instance pays one method call per inheritance level in __mypyc_defaults_setup. On a synthetic 7-level chain with 80+ inherited attributes per leaf, instance creation went from ~104 ns → ~119 ns (14% slower). The same scenario's partial-rebuild path went from rebuilding 106 modules in ~64s → 0 modules in ~1s (no more cache invalidation cascade) and the resulting .so files are slightly smaller (less duplicated default-init code per subclass).

Regression test

testIncrementalCrossModuleInheritedAttrDefaultsWithOverride in run-multimodule.test, exercised under both TestRunSeparate and TestRunMultiFile. Verified that it fails on release-2.1 without the fix and passes with it.

@georgesittas
georgesittas force-pushed the fix/incremental-defaults-setup-chain branch 4 times, most recently from cf1897f to 7c5a1c4 Compare May 27, 2026 12:34
@georgesittas
georgesittas requested review from VaggelisD and Copilot May 27, 2026 12:35

This comment was marked as outdated.

@georgesittas
georgesittas force-pushed the fix/incremental-defaults-setup-chain branch from 8c8b1b1 to 49e4169 Compare May 27, 2026 16:10
…ss groups (python#21524)

The fix for this was included in python#21369, but no dedicated test was
added.

This adds `testIncrementalBuiltinBaseClassConstruction` to
`run-multimodule.test`: three modules compiled with `separate=True`,
where step 2 changes a helper module's signature to force the caller to
be recompiled while the exception module is only loaded from cache.
…thon#21547)

Fixes python#21542

Under `separate=True`, when a subclass is recompiled while its parent is
loaded from mypy's incremental cache, parent default-attribute
assignments are silently dropped from the subclass's
`__mypyc_defaults_setup`. The first read of an inherited default-attr
then raises:

```
    AttributeError: attribute '<name>' of '<Parent>' undefined
```

`find_attr_initializers` walks `cdef.info.mro` and reads
`info.defn.defs.body` for `AssignmentStmt`s. `ClassDef.serialize`
(mypy/nodes.py) does not serialize `defs`, so a cache-loaded parent has
`defs = Block([])`; the MRO walk collects no parent assignments and the
subclass's emitted setup leaves inherited slots in the
undefined-sentinel state.

This PR implements the fix discussed in the linked issue.
@georgesittas
georgesittas force-pushed the fix/incremental-defaults-setup-chain branch from 49e4169 to 7500bf1 Compare June 4, 2026 13:23
@georgesittas
georgesittas merged commit 1c79015 into release-2.1 Jun 4, 2026
16 checks passed
@georgesittas
georgesittas deleted the fix/incremental-defaults-setup-chain branch June 4, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants