Skip to content

Commit 85f872a

Browse files
Update AGENTS.md with pre-submission check instruction (#2792)
* [agents] Add pre-submission checks and instructions * [agents] Add more instructions, rewrap comments * [agents] Add more advice * Undo wrapping of compiler output --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Joshua Liebow-Feeser <[email protected]>
1 parent bba1c6c commit 85f872a

File tree

12 files changed

+133
-87
lines changed

12 files changed

+133
-87
lines changed

AGENTS.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,32 @@ Clippy should **always** be run on the `nightly` toolchain.
4141
# Run a specific test on stable
4242
./cargo.sh +stable test -- test_name
4343
```
44+
45+
## Workflow
46+
47+
### Pre-submission Checks
48+
49+
Before submitting code, run `./githooks/pre-push` to confirm that all pre-push hooks succeed.
50+
51+
### UI Tests
52+
53+
When updating UI test files (in `tests/ui*` or `zerocopy-derive/tests/ui*`), run `./tools/update-ui-test-files.sh` to update the corresponding stderr files.
54+
55+
### Pull Requests and Commit Messages
56+
57+
When a PR resolves an issue, the PR description and commit message should include a line like `Closes #123`.
58+
When a PR makes progress on, but does not close, an issue, the PR description and commit message should include a line like `Makes progress on #123`.
59+
60+
## Code Style
61+
62+
### Comments
63+
64+
All comments (including `//`, `///`, and `//!`) should be wrapped at 80 columns.
65+
66+
**Exceptions:**
67+
- Markdown tables
68+
- Inline ASCII diagrams
69+
- Long URLs
70+
- Comments inside of code blocks
71+
- Comments which trail non-comment code
72+
- Other cases where wrapping would significantly degrade readability (use your judgment).

src/error.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -897,8 +897,9 @@ impl<Src, Dst: ?Sized + TryFromBytes> From<CastError<Src, Dst>> for TryCastError
897897

898898
/// The error type of fallible read-conversions.
899899
///
900-
/// Fallible read-conversions, like [`TryFromBytes::try_read_from_bytes`] may emit
901-
/// [size](SizeError) and [validity](ValidityError) errors, but not alignment errors.
900+
/// Fallible read-conversions, like [`TryFromBytes::try_read_from_bytes`] may
901+
/// emit [size](SizeError) and [validity](ValidityError) errors, but not
902+
/// alignment errors.
902903
// Bounds on generic parameters are not enforced in type aliases, but they do
903904
// appear in rustdoc.
904905
#[allow(type_alias_bounds)]

src/macros.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,7 +1204,8 @@ mod tests {
12041204
// that we do on and after 1.61.0.
12051205
#[cfg(not(zerocopy_generic_bounds_in_const_fn_1_61_0))]
12061206
{
1207-
// Test that `transmute_ref!` supports non-`KnownLayout` `Sized` types.
1207+
// Test that `transmute_ref!` supports non-`KnownLayout` `Sized`
1208+
// types.
12081209
const ARRAY_OF_NKL_U8S: Nkl<[u8; 8]> = Nkl([0u8, 1, 2, 3, 4, 5, 6, 7]);
12091210
const ARRAY_OF_NKL_ARRAYS: Nkl<[[u8; 2]; 4]> = Nkl([[0, 1], [2, 3], [4, 5], [6, 7]]);
12101211
const X_NKL: &Nkl<[[u8; 2]; 4]> = transmute_ref!(&ARRAY_OF_NKL_U8S);
@@ -1223,7 +1224,8 @@ mod tests {
12231224
transmute_ref!(t)
12241225
}
12251226

1226-
// Test that `transmute_ref!` supports non-`KnownLayout` `Sized` types.
1227+
// Test that `transmute_ref!` supports non-`KnownLayout` `Sized`
1228+
// types.
12271229
const ARRAY_OF_NKL_U8S: Nkl<[u8; 8]> = Nkl([0u8, 1, 2, 3, 4, 5, 6, 7]);
12281230
const ARRAY_OF_NKL_ARRAYS: Nkl<[[u8; 2]; 4]> = Nkl([[0, 1], [2, 3], [4, 5], [6, 7]]);
12291231
const X_NKL: &Nkl<[[u8; 2]; 4]> = transmute_ref(&ARRAY_OF_NKL_U8S);
@@ -1571,9 +1573,10 @@ mod tests {
15711573

15721574
cryptocorrosion_derive_traits! {
15731575
#[repr(C)]
1574-
/// Generic wrapper for unparameterized storage of any of the possible impls.
1575-
/// Converting into and out of this type should be essentially free, although it may be more
1576-
/// aligned than a particular impl requires.
1576+
/// Generic wrapper for unparameterized storage of any of the
1577+
/// possible impls. Converting into and out of this type should
1578+
/// be essentially free, although it may be more aligned than a
1579+
/// particular impl requires.
15771580
#[allow(non_camel_case_types)]
15781581
#[derive(Copy, Clone)]
15791582
pub union vec128_storage {

src/pointer/inner.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ mod _def {
5656
/// the set of addresses of an allocated object:
5757
/// ...
5858
/// - It is guaranteed that, given `o = a - base` (i.e., the offset of
59-
/// `a` within the allocated object), `base + o` will not wrap around
60-
/// the address space (in other words, will not overflow `usize`)
59+
/// `a` within the allocated object), `base + o` will not wrap
60+
/// around the address space (in other words, will not overflow
61+
/// `usize`)
6162
ptr: NonNull<T>,
6263
// SAFETY: `&'a UnsafeCell<T>` is covariant in `'a` and invariant in `T`
6364
// [1]. We use this construction rather than the equivalent `&mut T`,

src/pointer/ptr.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -389,8 +389,8 @@ mod _conversions {
389389
// SAFETY:
390390
// - This cast is a no-op, and so trivially preserves address,
391391
// referent size, and provenance
392-
// - It is trivially sound to have multiple `&T` referencing the same
393-
// referent simultaneously
392+
// - It is trivially sound to have multiple `&T` referencing the
393+
// same referent simultaneously
394394
// - By `T: TransmuteFromPtr<T, I::Aliasing, I::Validity, V>`, it is
395395
// sound to perform this transmute.
396396
let ptr = unsafe { self.transmute_unchecked(SizeEq::cast_from_raw) };
@@ -807,9 +807,9 @@ mod _transitions {
807807
I::Aliasing: Reference,
808808
I: Invariants<Validity = Initialized>,
809809
{
810-
// This call may panic. If that happens, it doesn't cause any soundness
811-
// issues, as we have not generated any invalid state which we need to
812-
// fix before returning.
810+
// This call may panic. If that happens, it doesn't cause any
811+
// soundness issues, as we have not generated any invalid state
812+
// which we need to fix before returning.
813813
if T::is_bit_valid(self.reborrow().forget_aligned()) {
814814
// SAFETY: If `T::is_bit_valid`, code may assume that `self`
815815
// contains a bit-valid instance of `T`. By `T:

src/util/macro_util.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -774,18 +774,19 @@ impl<'a, Src, Dst> Wrap<&'a Src, &'a Dst> {
774774
let src: *const Src = self.0;
775775
let dst = src.cast::<Dst>();
776776
// SAFETY:
777-
// - We know that it is sound to view the target type of the input reference
778-
// (`Src`) as the target type of the output reference (`Dst`) because the
779-
// caller has guaranteed that `Src: IntoBytes`, `Dst: FromBytes`, and
780-
// `size_of::<Src>() == size_of::<Dst>()`.
777+
// - We know that it is sound to view the target type of the input
778+
// reference (`Src`) as the target type of the output reference
779+
// (`Dst`) because the caller has guaranteed that `Src: IntoBytes`,
780+
// `Dst: FromBytes`, and `size_of::<Src>() == size_of::<Dst>()`.
781781
// - We know that there are no `UnsafeCell`s, and thus we don't have to
782-
// worry about `UnsafeCell` overlap, because `Src: Immutable` and `Dst:
783-
// Immutable`.
782+
// worry about `UnsafeCell` overlap, because `Src: Immutable` and
783+
// `Dst: Immutable`.
784784
// - The caller has guaranteed that alignment is not increased.
785-
// - We know that the returned lifetime will not outlive the input lifetime
786-
// thanks to the lifetime bounds on this function.
785+
// - We know that the returned lifetime will not outlive the input
786+
// lifetime thanks to the lifetime bounds on this function.
787787
//
788-
// FIXME(#67): Once our MSRV is 1.58, replace this `transmute` with `&*dst`.
788+
// FIXME(#67): Once our MSRV is 1.58, replace this `transmute` with
789+
// `&*dst`.
789790
#[allow(clippy::transmute_ptr_to_ref)]
790791
unsafe {
791792
mem::transmute(dst)
@@ -794,8 +795,8 @@ impl<'a, Src, Dst> Wrap<&'a Src, &'a Dst> {
794795
}
795796

796797
impl<'a, Src, Dst> Wrap<&'a mut Src, &'a mut Dst> {
797-
/// Transmutes a mutable reference of one type to a mutable reference of another
798-
/// type.
798+
/// Transmutes a mutable reference of one type to a mutable reference of
799+
/// another type.
799800
///
800801
/// # PME
801802
///

src/util/macros.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,10 @@ macro_rules! opt_unsafe_fn {
342342
/// assumption that the impl emitted by the custom derive is sound).
343343
///
344344
/// The caller is still required to provide a safety comment (e.g. using the
345-
/// `const _: () = unsafe` macro) . The reason for this restriction is that, while
346-
/// `impl_or_verify!` can guarantee that the provided impl is sound when it is
347-
/// compiled with the appropriate cfgs, there is no way to guarantee that it is
345+
/// `const _: () = unsafe` macro). The reason for this restriction is that,
346+
/// while `impl_or_verify!` can guarantee that the provided impl is sound when
347+
/// it is compiled with the appropriate cfgs, there is no way to guarantee that
348+
/// it is
348349
/// ever compiled with those cfgs. In particular, it would be possible to
349350
/// accidentally place an `impl_or_verify!` call in a context that is only ever
350351
/// compiled when the `derive` feature is disabled. If that were to happen,

src/util/mod.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ pub(crate) const fn padding_needed_for(len: usize, align: NonZeroUsize) -> usize
146146
#[allow(clippy::arithmetic_side_effects)]
147147
let mask = align.get() - 1;
148148

149-
// To efficiently subtract this value from align, we can use the bitwise complement.
149+
// To efficiently subtract this value from align, we can use the bitwise
150+
// complement.
150151
// Note that ((!len) & (align-1)) gives us a number that with (len &
151152
// (align-1)) sums to align-1. So subtracting 1 from x before taking the
152153
// complement subtracts `len` from `align`. Some quick inspection of
@@ -177,7 +178,8 @@ pub(crate) const fn padding_needed_for(len: usize, align: NonZeroUsize) -> usize
177178
//
178179
// (declare-const len (_ BitVec 32))
179180
// (declare-const align (_ BitVec 32))
180-
// ; Search for a case where align is a power of two and padding2 disagrees with padding1
181+
// ; Search for a case where align is a power of two and padding2 disagrees
182+
// ; with padding1
181183
// (assert (and (is-power-of-two align)
182184
// (not (= (padding1 len align) (padding2 len align)))))
183185
// (simplify (padding1 (_ bv300 32) (_ bv32 32))) ; 20
@@ -602,8 +604,9 @@ pub(crate) use len_of::MetadataOf;
602604
/// exist (stably) on our MSRV. This module provides polyfills for those
603605
/// features so that we can write more "modern" code, and just remove the
604606
/// polyfill once our MSRV supports the corresponding feature. Without this,
605-
/// we'd have to write worse/more verbose code and leave FIXME comments sprinkled
606-
/// throughout the codebase to update to the new pattern once it's stabilized.
607+
/// we'd have to write worse/more verbose code and leave FIXME comments
608+
/// sprinkled throughout the codebase to update to the new pattern once it's
609+
/// stabilized.
607610
///
608611
/// Each trait is imported as `_` at the crate root; each polyfill should "just
609612
/// work" at usage sites.

zerocopy-derive/src/enum.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ fn generate_variants_union(generics: &Generics, data: &DataEnum) -> TokenStream
162162
return None;
163163
}
164164

165-
// Field names are prefixed with `__field_` to prevent name collision with
166-
// the `__nonempty` field.
165+
// Field names are prefixed with `__field_` to prevent name collision
166+
// with the `__nonempty` field.
167167
let field_name_str = crate::ext::to_ident_str(&variant.ident);
168168
let field_name = Ident::new(&format!("__field_{}", field_name_str), variant.ident.span());
169169
let variant_struct_ident = variant_struct_ident(&variant.ident);

0 commit comments

Comments
 (0)