Introduce `doc(hidden)` `derive(Project)` by jswrenn · Pull Request #3492 · google/zerocopy

jswrenn · 2026-07-21T18:53:54Z

This factors out the derive(TryFromBytes) into project.rs, and
exposes this machinery as a doc-hidden derive(Project). The
implementation of is_bit_valid on enums is now fully safe, and a
soundness issue of mutable enum tag projection is resolved by
permitting only immutable tag projection.

Latest Update: v11 — Compare vs v10

📚 Full Patch History

Links show the diff between the row version and the column version.

Version	v10	v9	v8	v7	v6	v5	v4	v3	v2	v1	Base
v11	v10	v9	v8	v7	v6	v5	v4	v3	v2	v1	Base
v10		v9	v8	v7	v6	v5	v4	v3	v2	v1	Base
v9			v8	v7	v6	v5	v4	v3	v2	v1	Base
v8				v7	v6	v5	v4	v3	v2	v1	Base
v7					v6	v5	v4	v3	v2	v1	Base
v6						v5	v4	v3	v2	v1	Base
v5							v4	v3	v2	v1	Base
v4								v3	v2	v1	Base
v3									v2	v1	Base
v2										v1	Base
v1											Base

⬇️ Download this PR

Branch

git fetch origin refs/heads/Gcc56a15ce65387a017464947e8c2f25dc9e8c737 && git checkout -b pr-Gcc56a15ce65387a017464947e8c2f25dc9e8c737 FETCH_HEAD

Checkout

git fetch origin refs/heads/Gcc56a15ce65387a017464947e8c2f25dc9e8c737 && git checkout FETCH_HEAD

Cherry Pick

git fetch origin refs/heads/Gcc56a15ce65387a017464947e8c2f25dc9e8c737 && git cherry-pick FETCH_HEAD

Pull

git pull origin refs/heads/Gcc56a15ce65387a017464947e8c2f25dc9e8c737

Stacked PRs enabled by GHerrit.

codecov-commenter · 2026-07-21T18:57:01Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.78%. Comparing base (a175cac) to head (19a6a1b).

Additional details and impacted files

@@                              Coverage Diff                              @@
##           Gd2bc8560585e8f472223acf71b96e0f71b2d00c2    #3492      +/-   ##
=============================================================================
- Coverage                                      91.87%   91.78%   -0.09%     
=============================================================================
  Files                                             20       20              
  Lines                                           6101     6121      +20     
=============================================================================
+ Hits                                            5605     5618      +13     
- Misses                                           496      503       +7

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

joshlf · 2026-08-19T15:48:43Z

+// SPDX-License-Identifier: BSD-2-Clause OR Apache-2.0 OR MIT
+//


Needs to be the standard copyright header.

https://github.com/google/zerocopy/compare/35b52b53b682f7e003a7083cc78bbd71fdc6904b..f2265d47b41401f302b3ff86e20b436d258480ad#diff-209cd77b86ae434fb44cbeda3923b22c06764dd31eddcc22404f22ee953067a0R1-R9

Still need to remove this:

Suggested change

// SPDX-License-Identifier: BSD-2-Clause OR Apache-2.0 OR MIT

//

joshlf · 2026-08-19T15:52:28Z

+            // This casts the enum variant to its discriminant, and then
+            // converts the discriminant to the target integral type via a
+            // numeric cast [1].
+            //
+            // Because these are the same size, this is defined to be a no-op
+            // and therefore is a lossless conversion [2].
+            //
+            // [1] Per https://doc.rust-lang.org/1.81.0/reference/expressions/operator-expr.html#enum-cast:
+            //
+            //   Casts an enum to its discriminant.
+            //
+            // [2] Per https://doc.rust-lang.org/1.81.0/reference/expressions/operator-expr.html#numeric-cast:
+            //
+            //   Casting between two integers of the same size (e.g. i32 -> u32)
+            //   is a no-op.


Technically this should also document that this as conversion is implicitly two-step (ie, enum to discriminant and then discriminant to integer type).

We were a bit fried when we reviewed this, but we do indeed document this as a two-step conversion!

This casts the enum variant to its discriminant, and then converts the discriminant to the target integral type via a numeric cast.

Sorry, I meant that we need to cite the Rust documentation that that is the case, not just state it without justification.

joshlf · 2026-08-19T16:08:51Z

+        // preserve `Uninit` and `Initialized`, while `Valid` is weakened to
+        // `Uninit`.


Make sure to update this if we decide not to support mapping Valid to Uninit for unions.

joshlf · 2026-08-19T17:39:49Z

+/// `ProjectionValidity` and `EnumFieldProjection` enforce the remaining
+/// conditions:
+///
+/// - An uninitialized projection is infallible and remains uninitialized
+///   because `HasField::project` returns a subset of the input bytes and
+///   `Uninit` permits every bit pattern.
+/// - An initialized projection is infallible and remains initialized because
+///   every byte in a subset of a fully initialized byte range is initialized.
+/// - A valid projection is emitted only with a tag check derived from the same
+///   variant as its `VARIANT_ID`. Thus, once `is_projectable` succeeds, the
+///   projected field belongs to the active variant and is valid. This impl
+///   additionally requires `Reference` aliasing; together with the generated
+///   tag type's `Immutable` impl, that prevents the tag from changing during
+///   projection.


joshlf · 2026-08-19T17:45:47Z

+    field_id: Box<Expr>,
+    variant_struct_field_index: Index,


It feels like these two belong in a single type together. In fact, the only place we call EnumFieldProjection::new (which is the only constructor of this type) is from an enumerated iterator over fields. Thus, we could even introduce an EnumFieldIter which implements Iterator<Item = EnumField>, where EnumField { field_id: Box<Expr>, variant_struct_field_index: Index } (names to be bikeshedded ofc). That would make it clearer that these two fields are coupled rather than orthogonal. It would also give us a natural place to document their relationship.

joshlf · 2026-08-19T17:46:35Z

+    data: &DataEnum,
+    client: Client,
+    projection: &EnumFieldProjection,
+    validity: ProjectionValidity,


Suggested change

validity: ProjectionValidity,

input_validity: ProjectionValidity,

joshlf · 2026-08-19T17:47:58Z

+    /// `ProjectField` impls for this enum field. In particular, deriving the
+    /// variant ID here ensures that the valid projection's tag check and the
+    /// projected field always refer to the same variant.
+    fn field_projection(&self, ctx: &Ctx) -> FieldProjection {


TODO: Bikeshed better names for either this or for the receiver type or all of the above, considering readability at the use site.

joshlf · 2026-08-19T17:48:28Z

+    let projection = projection.field_projection(ctx);
+    let FieldProjection { variant_id, field, field_id } = projection;


Collapse into one line.

jswrenn · 2026-08-20T17:19:01Z

+            // This casts the enum variant to its discriminant, and then
+            // converts the discriminant to the target integral type via a
+            // numeric cast [1].
+            //
+            // Because these are the same size, this is defined to be a no-op
+            // and therefore is a lossless conversion [2].
+            //
+            // [1] Per https://doc.rust-lang.org/1.81.0/reference/expressions/operator-expr.html#enum-cast:
+            //
+            //   Casts an enum to its discriminant.
+            //
+            // [2] Per https://doc.rust-lang.org/1.81.0/reference/expressions/operator-expr.html#numeric-cast:
+            //
+            //   Casting between two integers of the same size (e.g. i32 -> u32)
+            //   is a no-op.


We were a bit fried when we reviewed this, but we do indeed document this as a two-step conversion!

This casts the enum variant to its discriminant, and then converts the discriminant to the target integral type via a numeric cast.

jswrenn · 2026-08-26T19:24:58Z

+            (Data::Struct(_), validity)
+            | (Data::Union(_), validity @ ProjectionValidity::Uninit)
+            | (Data::Union(_), validity @ ProjectionValidity::Initialized) => validity,
+            (Data::Union(_), ProjectionValidity::Valid) => ProjectionValidity::Uninit,


This doesn't have the same soundness issue per se, but it is basically the same as the soundness issue we're grappling with in rust-lang/reference#2340.

If the goal of our projection system is to provide all the safe projections there are, we should provide this projection, but doing so will conflict with also providing derive(IntoBytes) on unions.

This makes me worried that rust-lang/reference#2340 is heavy-handed; this dilemma could alternatively be resolved by making IntoBytes and unsafe derive (if such a thing existed) on unions. We want unsafe derives for other reasons, and perhaps that's where we should be throwing our weight.

jswrenn · 2026-08-27T21:33:08Z

Want to highlight this: It's now the case that the only occurrences of unsafe in derive(TryFromBytes) on enums are unsafe impls, and calls to addr_of!

This factors out the `derive(TryFromBytes)` into `project.rs`, and exposes this machinery as a doc-hidden `derive(Project)`. The implementation of `is_bit_valid` on enums is now fully safe, and a soundness issue of mutable enum tag projection is resolved by permitting only immutable tag projection. gherrit-pr-id: Gcc56a15ce65387a017464947e8c2f25dc9e8c737

joshlf · 2026-08-28T17:04:40Z

+// SPDX-License-Identifier: BSD-2-Clause OR Apache-2.0 OR MIT
+//


Still need to remove this prefix:

Suggested change

// SPDX-License-Identifier: BSD-2-Clause OR Apache-2.0 OR MIT

//

Didn't we add those intentionally for the linux folks? We have this prefix everywhere: https://github.com/search?q=repo%3Agoogle%2Fzerocopy+SPDX-License-Identifier%3A+BSD-2-Clause+OR+Apache-2.0+OR+MIT&type=code

joshlf · 2026-08-28T17:05:30Z

+            // This casts the enum variant to its discriminant, and then
+            // converts the discriminant to the target integral type via a
+            // numeric cast [1].
+            //
+            // Because these are the same size, this is defined to be a no-op
+            // and therefore is a lossless conversion [2].
+            //
+            // [1] Per https://doc.rust-lang.org/1.81.0/reference/expressions/operator-expr.html#enum-cast:
+            //
+            //   Casts an enum to its discriminant.
+            //
+            // [2] Per https://doc.rust-lang.org/1.81.0/reference/expressions/operator-expr.html#numeric-cast:
+            //
+            //   Casting between two integers of the same size (e.g. i32 -> u32)
+            //   is a no-op.


Sorry, I meant that we need to cite the Rust documentation that that is the case, not just state it without justification.

joshlf · 2026-08-28T17:13:38Z

+            (Data::Struct(_), validity)
+            | (Data::Union(_), validity @ ProjectionValidity::Uninit)
+            | (Data::Union(_), validity @ ProjectionValidity::Initialized) => validity,
+            (Data::Union(_), ProjectionValidity::Valid) => ProjectionValidity::Uninit,


So to clarify: since unions have no bit validity requirements from a language standpoint, then both the source and destination validities permit uninit bytes, and so this neither expands nor shrinks the validity. If instead we care about library safety, then it's a question of the "default" safety invariant, which is an open question.

However, I also think there's a soundness hole here regardless: This currently admits Initialized -> Uninit which is invalid regardless of union validity.

Conversation continued here.

joshlf · 2026-08-28T17:19:31Z

+// SPDX-License-Identifier: BSD-2-Clause OR Apache-2.0 OR MIT
+//


Still need to remove this:

Suggested change

// SPDX-License-Identifier: BSD-2-Clause OR Apache-2.0 OR MIT

//

jswrenn · 2026-08-28T18:21:32Z

+            //      safe-transmute reasons, we can add a way to opt-in to that,
+            //      e.g. via an attribute on the union or on the field that
+            //      should define the niche of the union.
+            (Data::Union(_), Validity::Valid) => Validity::Uninit,


So to clarify: since unions have no bit validity requirements from a language standpoint, then both the source and destination validities permit uninit bytes, and so this neither expands nor shrinks the validity. If instead we care about library safety, then it's a question of the "default" safety invariant, which is an open question.

I don't think there's any open question here. As I outlined here, it's already the case that anyone in the ecosystem could provide an abstraction like the one here and it would be sound, because the language enforces using unsafe on union field accesses and a discharge of the validity proof.

If the user wishes to have a compiler-enforced safety invariant, they will be able to do so by marking their field as unsafe. As a rule, we won't provide safe projections for unsafe fields.

However, I also think there's a soundness hole here regardless: This currently admits Initialized -> Uninit which is invalid regardless of union validity.

How so? For unions, this match maps Uninit to Uninit, Initialized to Initialized, and Valid to Uninitialized, but not Valid to Initialized.

jswrenn mentioned this pull request Jul 21, 2026

Add Client parameter to projection traits #3480

Open

jswrenn force-pushed the Gcc56a15ce65387a017464947e8c2f25dc9e8c737 branch from 35b52b5 to ba8ac3a Compare August 18, 2026 20:27

joshlf requested changes Aug 19, 2026

View reviewed changes

jswrenn commented Aug 26, 2026

View reviewed changes

jswrenn mentioned this pull request Aug 27, 2026

Specify the values of safely-constructed unions rust-lang/reference#2340

Closed

jswrenn force-pushed the Gcc56a15ce65387a017464947e8c2f25dc9e8c737 branch 5 times, most recently from f14a0cc to f2265d4 Compare August 27, 2026 21:19

jswrenn commented Aug 27, 2026

View reviewed changes

jswrenn force-pushed the Gcc56a15ce65387a017464947e8c2f25dc9e8c737 branch 2 times, most recently from df8080c to d32515a Compare August 28, 2026 14:19

jswrenn changed the title ~~[wip] Introduce derive(Project)~~ Introduce doc(hidden) derive(Project) Aug 28, 2026

jswrenn force-pushed the Gcc56a15ce65387a017464947e8c2f25dc9e8c737 branch from d32515a to 3501462 Compare August 28, 2026 15:05

jswrenn force-pushed the Gcc56a15ce65387a017464947e8c2f25dc9e8c737 branch from 3501462 to 19a6a1b Compare August 28, 2026 15:31

jswrenn force-pushed the Gd2bc8560585e8f472223acf71b96e0f71b2d00c2 branch from 644b98b to a175cac Compare August 28, 2026 15:31

joshlf requested changes Aug 28, 2026

View reviewed changes

jswrenn commented Aug 28, 2026

View reviewed changes

		// SPDX-License-Identifier: BSD-2-Clause OR Apache-2.0 OR MIT
		//

		// preserve `Uninit` and `Initialized`, while `Valid` is weakened to
		// `Uninit`.

	validity: ProjectionValidity,
	input_validity: ProjectionValidity,

		let projection = projection.field_projection(ctx);
		let FieldProjection { variant_id, field, field_id } = projection;

Uh oh!

Conversation

jswrenn commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshlf Aug 28, 2026 • edited by jswrenn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jswrenn commented Jul 21, 2026 •

edited

Loading

codecov-commenter commented Jul 21, 2026 •

edited

Loading

joshlf Aug 28, 2026 •

edited by jswrenn

Loading