Skip to content

Conversation

@jswrenn
Copy link
Collaborator

@jswrenn jswrenn commented Jan 14, 2026

This trait powers invariant-respecting projection over Ptrs.


Latest Update: v21 — Compare vs v20

📚 Full Patch History

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

Version v20 v19 v18 v17 v16 v15 v14 v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v21 v20 v19 v18 v17 v16 v15 v14 v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v20 v19 v18 v17 v16 v15 v14 v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v19 v18 v17 v16 v15 v14 v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v18 v17 v16 v15 v14 v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v17 v16 v15 v14 v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v16 v15 v14 v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v15 v14 v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v14 v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v13 v12 v11 v10 v9 v8 v7 v6 v5 v4 v3 v2 v1 Base
v12 v11 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

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @jswrenn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces the ProjectField trait, a foundational component for enabling safe and invariant-respecting field projection within the system. This trait provides a structured way to access fields of Ptr types while upholding data integrity and safety guarantees, likely serving as an internal mechanism for future derive macros or similar functionalities.

Highlights

  • New ProjectField Trait: Introduced a new unsafe trait ProjectField designed for invariant-respecting projection over Ptrs. This trait is marked doc(hidden) and includes a method only_derive_is_allowed_to_implement_this_trait, indicating its intended use primarily by derive macros for internal mechanisms.
  • Trait Definition: The ProjectField trait defines associated types Invariants (for the projected field's pointer's invariants) and Error (for fallible projection), along with a project function that safely projects a field from a Ptr.
  • Safety Invariants: The trait explicitly states safety requirements for its implementations and for the project function, ensuring that the returned pointer refers to a non-strict subset of the original Ptr's bytes and maintains provenance.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new unsafe trait ProjectField to facilitate invariant-respecting projections over Ptrs. The new trait is well-defined and documented. My review includes a suggestion to correct a minor inconsistency in the documentation for the project method, where the parameter name used in the comments differs from the one in the function signature. Aligning these will improve clarity, which is crucial for an unsafe trait.

@jswrenn jswrenn force-pushed the Gb6fa34cec5080caffa0980ed1d3e1ae273550f44 branch from 7261c75 to 30859f4 Compare January 14, 2026 19:14
@jswrenn jswrenn force-pushed the G90165d85418ed6203ef3caaa77662dd3b313e030 branch from bb6f84e to c6f7382 Compare January 14, 2026 19:14
@codecov-commenter
Copy link

codecov-commenter commented Jan 14, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.71%. Comparing base (5c67d2c) to head (7f5262d).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/impls.rs 0.00% 12 Missing ⚠️
src/pointer/ptr.rs 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2886      +/-   ##
==========================================
- Coverage   91.90%   91.71%   -0.19%     
==========================================
  Files          20       20              
  Lines        5877     5903      +26     
==========================================
+ Hits         5401     5414      +13     
- Misses        476      489      +13     

☔ View full report in Codecov by Sentry.
📢 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.

@jswrenn jswrenn force-pushed the G90165d85418ed6203ef3caaa77662dd3b313e030 branch from c6f7382 to dc4cfc8 Compare January 15, 2026 16:02
@jswrenn jswrenn force-pushed the Gb6fa34cec5080caffa0980ed1d3e1ae273550f44 branch 2 times, most recently from c37d1c9 to 105108b Compare January 15, 2026 18:45
@jswrenn jswrenn force-pushed the G90165d85418ed6203ef3caaa77662dd3b313e030 branch from dc4cfc8 to dbdce80 Compare January 16, 2026 18:18
@jswrenn jswrenn force-pushed the Gb6fa34cec5080caffa0980ed1d3e1ae273550f44 branch 2 times, most recently from 4c71ea4 to 7850e9f Compare January 16, 2026 19:23
@jswrenn jswrenn force-pushed the G90165d85418ed6203ef3caaa77662dd3b313e030 branch from dbdce80 to 16c5cd9 Compare January 16, 2026 19:23
Base automatically changed from G90165d85418ed6203ef3caaa77662dd3b313e030 to main January 16, 2026 20:35
@jswrenn jswrenn force-pushed the Gb6fa34cec5080caffa0980ed1d3e1ae273550f44 branch 9 times, most recently from e9aba20 to 3389089 Compare January 21, 2026 17:41
@jswrenn jswrenn force-pushed the Gb6fa34cec5080caffa0980ed1d3e1ae273550f44 branch from 3389089 to 0ea7f5e Compare January 21, 2026 18:37
@jswrenn jswrenn force-pushed the Gb6fa34cec5080caffa0980ed1d3e1ae273550f44 branch 3 times, most recently from 9ee7b9e to 86ed3f5 Compare January 22, 2026 20:53
@jswrenn jswrenn force-pushed the Gb6fa34cec5080caffa0980ed1d3e1ae273550f44 branch from 86ed3f5 to 0208eba Compare January 23, 2026 12:39
@jswrenn jswrenn requested a review from joshlf January 23, 2026 12:41
Comment on lines 462 to 493
unsafe { #core::ptr::addr_of_mut!((*slf).#ident) }
unsafe { #zerocopy_crate::util::macro_util::core_reexport::ptr::addr_of_mut!((*slf).#ident) }
Copy link
Member

Choose a reason for hiding this comment

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

Revert this?

Comment on lines 458 to 460
let fields_preserve_alignment = StructUnionRepr::from_attrs(&ctx.ast.attrs)
.map(|repr| repr.get_packed().is_none())
.unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Move this to the else if matches!(&ctx.ast.data, Data::Struct(..)) branch below since it's only used there?

@joshlf
Copy link
Member

joshlf commented Jan 23, 2026

/gemini review

This trait powers invariant-respecting projection over `Ptr`s.

gherrit-pr-id: Gb6fa34cec5080caffa0980ed1d3e1ae273550f44
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request introduces the ProjectField trait, which is a significant and positive change for invariant-respecting projection over Ptrs. This refactoring allows for more granular control and safety guarantees when projecting fields, especially with fallible projections. The introduction of the into_inner! macro for infallible Result unwrapping is also a good addition.

Comment on lines +272 to +273
invariants: parse_quote!((Aliasing, Alignment, #zerocopy_crate::invariant::Initialized)),
},
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The ProjectField implementation for enum fields uses (Aliasing, Alignment, #zerocopy_crate::invariant::Initialized) for type Invariants. This means that projecting an enum field will always result in an Initialized pointer, even if the original Ptr had Valid invariants and the field itself is Valid. This could lead to a loss of the stronger Valid invariant, which might be an unnecessary restriction or potentially incorrect if Valid could be preserved.

Suggested change
invariants: parse_quote!((Aliasing, Alignment, #zerocopy_crate::invariant::Initialized)),
},
invariants: parse_quote!((Aliasing, Alignment, I::Validity)),

Comment on lines +569 to +571
// `Initialized`, so too is the validity of its fields.
type Invariants = (Aliasing, #alignment, #zerocopy_crate::invariant::Initialized);
Copy link
Contributor

Choose a reason for hiding this comment

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

high

Similar to the enum field projection, the ProjectField implementation for struct fields also uses (Aliasing, #alignment, #zerocopy_crate::invariant::Initialized) for type Invariants. This has the same potential issue of losing the Valid invariant if the original Ptr was Valid and the field is also Valid. The invariant propagation should ideally preserve the strongest possible validity.

Suggested change
// `Initialized`, so too is the validity of its fields.
type Invariants = (Aliasing, #alignment, #zerocopy_crate::invariant::Initialized);
type Invariants = (Aliasing, #alignment, I::Validity);

unsafe { self.cast_unchecked::<_, C>() }
}

// FIXME(#196): Support all validity invariants (not just those that are
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The FIXME(#196) comment regarding supporting all validity invariants is still present. With the introduction of the ProjectField trait, it might be a good time to re-evaluate if this new abstraction helps address or changes the scope of this long-standing issue.

Copy link
Member

Choose a reason for hiding this comment

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

Please fix.

@jswrenn jswrenn force-pushed the Gb6fa34cec5080caffa0980ed1d3e1ae273550f44 branch from 0208eba to 7f5262d Compare January 23, 2026 16:54
Comment on lines +1188 to +1191
/// Is the given field projectable from `ptr`?
///
/// This method must be overriden if the field's projectability depends on
/// the value of the bytes in `ptr`.
Copy link
Member

Choose a reason for hiding this comment

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

Update these docs (and maybe the name of the name of the method) given the new signature (which returns Ptr in the Ok variant)?

Copy link
Member

@joshlf joshlf left a comment

Choose a reason for hiding this comment

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

Please resolve remaining comments before merging.

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.

3 participants