Skip to content

transpile: fix zero initializer imports#1864

Open
thedataking wants to merge 1 commit into
masterfrom
perl/zero-init-imports
Open

transpile: fix zero initializer imports#1864
thedataking wants to merge 1 commit into
masterfrom
perl/zero-init-imports

Conversation

@thedataking

@thedataking thedataking commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Fix zero-initializer import replay when a cached initializer is reused from a reorganized submodule.

The initializer type import now participates in the deferred import set that is cached with the initializer. This prevents typedef names such as STAT_T from being emitted in a header module without the matching use.

Surfaced when working on PR #1861.

@thedataking thedataking marked this pull request as ready for review June 18, 2026 10:56
@thedataking thedataking added the bug Something isn't working label Jun 18, 2026
@fw-immunant

fw-immunant commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

From 1:1 discussion: I don't understand why this isn't a semantics-preserving change, given that beforehand we call import_type upfront (even on calls that will be cache hits), and afterward we defer imports and then do so (so that calling import_type does not directly modify the imports when called but rather adds these imports to the pending cache entry). It seems like both before and after this change, the imports added by import_type will be added on every call to zero_initializer and the only thing changing is whether they are read from the cache or not.

I'd like to see an explanation of what isn't true in the above understanding, or an example trace of how control/data flow works out for a particular case that makes it clear why this is a fix.

Does the attached test really fail without the accompanying change to zero_initializer?

@thedataking

Copy link
Copy Markdown
Contributor Author

Does the attached test really fail without the accompanying change to zero_initializer?

It does not; investigating.

@thedataking thedataking force-pushed the perl/zero-init-imports branch from 7b82bd7 to 604ebcb Compare July 12, 2026 09:31
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@thedataking thedataking force-pushed the perl/zero-init-imports branch from 604ebcb to 771c8a8 Compare July 12, 2026 11:31
@thedataking

Copy link
Copy Markdown
Contributor Author

From 1:1 discussion: I don't understand why this isn't a semantics-preserving change, given that beforehand we call import_type upfront (even on calls that will be cache hits), and afterward we defer imports and then do so (so that calling import_type does not directly modify the imports when called but rather adds these imports to the pending cache entry). It seems like both before and after this change, the imports added by import_type will be added on every call to zero_initializer and the only thing changing is whether they are read from the cache or not.

I'd like to see an explanation of what isn't true in the above understanding, or an example trace of how control/data flow works out for a particular case that makes it clear why this is a fix.

Does the attached test really fail without the accompanying change to zero_initializer?

You're right that the test as written doesn't fail without the change. Your model is right except for one thing: type_id differs per call site while the cache is keyed on decl_id, and the two sides of the cache disagree about which one matters. On a miss, the cached expression's head name follows the miss site's type_id (via name_decl_id), but the import_type(type_id) import was applied before defer_imports(), so it never entered the cached import set. On a hit, we re-ran import_type with the hit site's type_id which only compensates if both sites spell the type identically.

Concrete trace from libgit2 (clar): clar.c has typedef struct stat STAT_T. sandbox.h's is_valid_tmp_path declares STAT_T st; — first implicit zero-init of struct stat, so the cache stores STAT_T { ... }, and the STAT_T import goes directly into sandbox_h's module only. Later fs.h's fs_copy declares struct stat source_st; — cache hit, fs_h emits the cached STAT_T { ... } but the compensating import_type imports only stat: error[E0422]: cannot find struct STAT_T. Moving import_type after defer_imports() puts the head-name import into the cached set, so hits replay exactly the imports the cached expression needs.

The snapshot test misses this because both sites use the same spelling and = {0} init-lists rebuild the expression rather than replaying the cached one. I'll update it to the failing shape: populate the cache through a typedef-spelled field (implicit zero-init), then hit it from a struct-tag-spelled field in a header module.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants