Introduce HasField::project_inner
#2885
Conversation
Summary of ChangesHello @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 enhances the Highlights
🧠 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 AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces HasField::project_inner with a default implementation, which provides a safe wrapper around project_raw. The implementation logic is sound, but the safety comments contain minor inaccuracies regarding the argument name, which could cause confusion. I've suggested a fix to improve clarity.
src/lib.rs
Outdated
| // SAFETY: `self`'s referent lives at a `NonNull` address, and is either | ||
| // zero-sized or lives in an allocation. In either case, it does not | ||
| // wrap around the address space [1], and so none of the addresses | ||
| // contained in it or one-past-the-end of it are null. | ||
| // | ||
| // By invariant on `project_raw`, `project_raw` is a | ||
| // provenance-preserving projection which preserves or shrinks the set | ||
| // of referent bytes, so `projected_raw` references a subset of `self`'s | ||
| // referent, and so it cannot be null. | ||
| // | ||
| // [1] https://doc.rust-lang.org/1.92.0/std/ptr/index.html#allocation | ||
| let projected_non_null = unsafe { NonNull::new_unchecked(projected_raw) }; | ||
| // SAFETY: As described in the preceding safety comment, | ||
| // `projected_raw`, and thus `projected_non_null`, addresses a subset of | ||
| // `self`'s referent. Thus, `projected_non_null` either: | ||
| // - Addresses zero bytes or, | ||
| // - Addresses a subset of the referent of `self`. In this case, `self` | ||
| // has provenance for its referent, which lives in an allocation. | ||
| // Since `projected_non_null` was constructed using a sequence of | ||
| // provenance-preserving operations, it also has provenance for its | ||
| // referent and that referent lives in an allocation. By invariant on | ||
| // `self`, that allocation lives for `'a`. | ||
| unsafe { PtrInner::new(projected_non_null) } |
There was a problem hiding this comment.
The safety comments in this function refer to self, but the argument is named slf. This should be corrected to slf for clarity and to accurately reflect the code. This is important for ensuring the safety argument is easy to understand and verify.
// SAFETY: `slf`'s referent lives at a `NonNull` address, and is either
// zero-sized or lives in an allocation. In either case, it does not
// wrap around the address space [1], and so none of the addresses
// contained in it or one-past-the-end of it are null.
//
// By invariant on `project_raw`, `project_raw` is a
// provenance-preserving projection which preserves or shrinks the set
// of referent bytes, so `projected_raw` references a subset of `slf`'s
// referent, and so it cannot be null.
//
// [1] https://doc.rust-lang.org/1.92.0/std/ptr/index.html#allocation
let projected_non_null = unsafe { NonNull::new_unchecked(projected_raw) };
// SAFETY: As described in the preceding safety comment,
// `projected_raw`, and thus `projected_non_null`, addresses a subset of
// `slf`'s referent. Thus, `projected_non_null` either:
// - Addresses zero bytes or,
// - Addresses a subset of the referent of `slf`. In this case, `slf`
// has provenance for its referent, which lives in an allocation.
// Since `projected_non_null` was constructed using a sequence of
// provenance-preserving operations, it also has provenance for its
// referent and that referent lives in an allocation. By invariant on
// `slf`, that allocation lives for `'a`.
unsafe { PtrInner::new(projected_non_null) }
gherrit-pr-id: G90165d85418ed6203ef3caaa77662dd3b313e030
jswrenn
force-pushed
the
G90165d85418ed6203ef3caaa77662dd3b313e030
branch
from
bb6f84e to
c6f7382
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## Gb943ae3571a06b4a5c727268ae1829579e6350dc #2885 +/- ##
============================================================================
Coverage ? 91.82%
============================================================================
Files ? 20
Lines ? 5883
Branches ? 0
============================================================================
Hits ? 5402
Misses ? 481
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
ProjectFieldtrait #2886HasField::project_inner#2885HasField::projecttoHasField::project_raw#2884Latest Update: v2 — Compare vs v1
📚 Full Patch History
Links show the diff between the row version and the column version.