Skip to content

Fix assembly privacy checks and build provenance inputs#3091

Merged
huitseeker merged 4 commits intonextfrom
followups-2945-2712-2897
May 6, 2026
Merged

Fix assembly privacy checks and build provenance inputs#3091
huitseeker merged 4 commits intonextfrom
followups-2945-2712-2897

Conversation

@huitseeker
Copy link
Copy Markdown
Collaborator

@huitseeker huitseeker commented May 4, 2026

This closes #2945, closes #2712, and closes #2897.

Some assembly imports were resolved without checking whether the target item was public, and signature-only type uses were skipped by the import visitor. Build reuse also depended on manifest data that should not affect compiled output.

This PR:

  • rejects private cross-module imports, including absolute-path aliases
  • counts imports used only in procedure signatures
  • narrows build provenance to fields that affect output
  • marks required core STARK helper procedures public
  • updates generated core library docs for those public procedures

Design decisions that could have gone another way:

  1. Build provenance means target-specific build identity: does not mean the full effective manifest hash. Only fields that can affect the selected target’s artifact should trigger rebuild/reuse changes.
  2. The project model owns provenance classification: Assembly shouldn't hash rendered TOML or infer meaning from serialization.
  3. Unrelated manifest data should not affect artifact reuse: Adding or changing another target should not rebuild a library artifact unless it can affect that library’s package output.
  4. Module visibility applies to all item kinds: Private procedures, constants, and types are all private across module boundaries. Constants are not exempt just because they are compile-time values. This is why this PR includes some masm changes.
  5. Signature-only type references count as import usage: A type used only in a proc argument or result is still semantically used, so its import must be retained / validated.

@huitseeker huitseeker requested review from Al-Kindi-0 and bitwalker May 4, 2026 22:26
@huitseeker huitseeker force-pushed the followups-2945-2712-2897 branch from 54d7665 to 95f1fbe Compare May 4, 2026 22:33
@huitseeker huitseeker marked this pull request as ready for review May 4, 2026 22:34
@bitwalker
Copy link
Copy Markdown
Collaborator

Design decisions that could have gone another way:

  1. Build provenance means target-specific build identity: does not mean the full effective manifest hash. Only fields that can affect the selected target’s artifact should trigger rebuild/reuse changes.
  2. The project model owns provenance classification: Assembly shouldn't hash rendered TOML or infer meaning from serialization.
  3. Unrelated manifest data should not affect artifact reuse: Adding or changing another target should not rebuild a library artifact unless it can affect that library’s package output.
  4. Module visibility applies to all item kinds: Private procedures, constants, and types are all private across module boundaries. Constants are not exempt just because they are compile-time values. This is why this PR includes some masm changes.
  5. Signature-only type references count as import usage: A type used only in a proc argument or result is still semantically used, so its import must be retained / validated.

All those seem fine to me.

I think the main thing missing here is that we must reject the use of private type declarations in the signature of exported procedures, since it would cause dangling references in the final package in some cases (namely enum types). The type system is structural, not nominal, so most type aliases are transparent (i.e. a private type alias could appear in an exported signature and it wouldn't have an impact on the type signature encoded in the final package) - but for the purpose of binding generation in Rust/other languages, we will probably move towards preserving at least some names in the final artifact, so rejecting that as an edge case will make that doable without causing breakages at some point.

@huitseeker huitseeker force-pushed the followups-2945-2712-2897 branch from 815c53f to 431fd2a Compare May 4, 2026 23:29
@huitseeker
Copy link
Copy Markdown
Collaborator Author

@bitwalker Right on, I opened #3092.

Copy link
Copy Markdown
Contributor

@Al-Kindi-0 Al-Kindi-0 left a comment

Choose a reason for hiding this comment

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

LGTM, modulo a couple of questions

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Q: What is the intention of this test ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The test checks that bundle --kernel can still produce a package when the bundled library has no exports. After the new privacy rule, the old fixture was invalid because its kernel called a private proc from lib_noexports. The new kernel_noexports.masm keeps the same test intent without relying on a private cross-module call.

Comment thread CHANGELOG.md Outdated
@huitseeker huitseeker force-pushed the followups-2945-2712-2897 branch 3 times, most recently from ba39621 to 2d48398 Compare May 6, 2026 00:39
@huitseeker huitseeker force-pushed the followups-2945-2712-2897 branch from 2d48398 to 6865e9f Compare May 6, 2026 12:25
@huitseeker huitseeker merged commit 0ca740b into next May 6, 2026
23 checks passed
@huitseeker huitseeker deleted the followups-2945-2712-2897 branch May 6, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants