Skip to content

Commit

Permalink
[ci.yml] Test zerocopy-derive in CI, fix tests (#27)
Browse files Browse the repository at this point in the history
In some cases, zerocopy-derive performs its analysis by emitting code
which the Rust compiler will reject if implementing a particular trait
would not be sound. This means that, when the rustc's error messages
change, so do zerocopy-derive's. We have "UI" tests which attempt to
comile invalid code, and expect that it is rejected in the appropriate
way. These tests are dependent upon the exact error messages produced in
different cases.

Since rustc sometimes changes its error messages between versions, this
means that a single set of UI tests cannot be correct for all Rust
versions. In this commit, we split our UI tests - we keep one copy of
the source `.rs` files, but multiple copies of the expected error
messages, one per toolchain pinned in CI.
  • Loading branch information
joshlf authored Oct 6, 2022
1 parent 168e4bc commit 2c3bf40
Show file tree
Hide file tree
Showing 24 changed files with 808 additions and 11 deletions.
43 changes: 33 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,17 @@ jobs:

strategy:
matrix:
# Nightly is specificied at a particular date since not all nightlies
# have a working Miri. See here for details:
# https://rust-lang.github.io/rustup-components-history/
channel: [ "1.56.1", "stable", "beta", "nightly-2022-09-26" ]
# See `INTERNAL.md` for an explanation of these pinned toolchain
# versions.
channel: [ "1.56.1", "1.64.0", "nightly-2022-09-26" ]
target: [ "i686-unknown-linux-gnu", "x86_64-unknown-linux-gnu", "arm-unknown-linux-gnueabi", "aarch64-unknown-linux-gnu", "powerpc-unknown-linux-gnu", "powerpc64-unknown-linux-gnu", "wasm32-wasi" ]
features: [ "" , "alloc,simd", "alloc,simd,simd-nightly"]
features: [ "" , "alloc,simd", "alloc,simd,simd-nightly" ]
exclude:
# Exclude any combination which uses a non-nightly toolchain but
# enables nightly features.
- channel: "1.56.1"
features: "alloc,simd,simd-nightly"
- channel: "stable"
features: "alloc,simd,simd-nightly"
- channel: "beta"
- channel: "1.64.0"
features: "alloc,simd,simd-nightly"

name: Build & Test (${{ matrix.channel }} for ${{ matrix.target }}, features set to "${{ matrix.features }}")
Expand Down Expand Up @@ -54,21 +51,47 @@ jobs:
- name: Check
run: cargo +${{ matrix.channel }} check --target ${{ matrix.target }} --features "${{ matrix.features }}" --verbose

- name: Check zerocopy-derive
run: cargo +${{ matrix.channel }} check --manifest-path ./zerocopy-derive/Cargo.toml --target ${{ matrix.target }} --verbose
# Don't bother to check `zerocopy-derive` multiple times; that's what
# would happen if we ran this step once for each set of `zerocopy`
# features.
if: ${{ matrix.features == '' }}

- name: Build
run: cargo +${{ matrix.channel }} build --target ${{ matrix.target }} --features "${{ matrix.features }}" --verbose

- name: Build zerocopy-derive
run: cargo +${{ matrix.channel }} build --manifest-path ./zerocopy-derive/Cargo.toml --target ${{ matrix.target }} --verbose
# Don't bother to build `zerocopy-derive` multiple times; that's what
# would happen if we ran this step once for each set of `zerocopy`
# features.
if: ${{ matrix.features == '' }}

# When building tests for the i686 target, we need certain libraries which
# are not installed by default; `gcc-multilib` includes these libraries.
- name: Install gcc-multilib
run: sudo apt-get install gcc-multilib
if: ${{ contains(matrix.target, 'i686') }}

# Only run tests in the x86 architecture
- name: Run tests
# TODO(#21): Fix `test_new_error` on i686 and re-enable it here.
run: cargo +${{ matrix.channel }} test --target ${{ matrix.target }} --features "${{ matrix.features }}" --verbose -- ${{ contains(matrix.target, 'i686') && '--skip test_new_error' || '' }}
# Only run tests when targetting x86 (32- or 64-bit) - we're executing on
# x86_64, so we can't run tests for any non-x86 target.
if: ${{ contains(matrix.target, 'x86_64') || contains(matrix.target, 'i686') }}


- name: Run zerocopy-derive tests
run: cargo +${{ matrix.channel }} test --manifest-path ./zerocopy-derive/Cargo.toml --target ${{ matrix.target }} --verbose
# Don't bother to test `zerocopy-derive` multiple times; that's what would
# happen if we ran this step once for each set of `zerocopy` features.
# Also, only run tests when targetting x86 (32- or 64-bit) - we're
# executing on x86_64, so we can't run tests for any non-x86 target.
#
# TODO(https://github.com/dtolnay/trybuild/issues/184#issuecomment-1269097742):
# Run compile tests when building for other targets.
if: ${{ matrix.features == '' && (contains(matrix.target, 'x86_64') || contains(matrix.target, 'i686')) }}

- name: Run tests under Miri
# Skip the `ui` test since it invokes the compiler, which we can't do from
# Miri (and wouldn't want to do anyway).
Expand Down
22 changes: 22 additions & 0 deletions INTERNAL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Internal details

This file documents various internal details of zerocopy and its infrastructure
that consumers don't need to be concerned about. It focuses on details that
affect multiple files, and allows each affected code location to reference this
document rather than requiring us to repeat the same explanation in multiple
locations.

## CI and toolchain versions

In CI (`.github/workflows/ci.yml`), we pin to specific versions or dates of the
stable and nightly toolchains. The reason is twofold: First, `zerocopy-derive`'s
UI tests (see `zerocopy-derive/tests/trybuild.rs`) depend on the format of
rustc's error messages, and that format can change between toolchain versions
(we also maintain multiple copies of our UI tests - one for each toolchain
version pinned in CI - for this reason). Second, not all nightlies have a
working Miri, so we need to pin to one that does (see
https://rust-lang.github.io/rustup-components-history/).

Updating the versions pinned in CI may cause the UI tests to break. In order to
fix UI tests after a version update, set the environment variable
`TRYBUILD=overwrite` while running `cargo test`.
1 change: 1 addition & 0 deletions zerocopy-derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ quote = "1.0.10"
syn = { version = "1.0.5", features = ["visit"] }

[dev-dependencies]
rustversion = "1.0"
trybuild = "1.0"
zerocopy = { path = "../" }
22 changes: 21 additions & 1 deletion zerocopy-derive/tests/trybuild.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,28 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// UI tests depend on the exact error messages emitted by rustc, but those error
// messages are not stable, and sometimes change between Rust versions. Thus, we
// maintain one set of UI tests for each Rust version that we test in CI, and we
// pin to specific versions in CI (a specific stable version, a specific date of
// the nightly compiler, and a specific MSRV). Updating those pinned versions
// may also require updating these tests.
// - `tests/ui` - Contains the source of truth for our UI test source files
// (`.rs`), and contains `.err` and `.out` files for nightly and beta
// - `tests/ui-stable` - Contains symlinks to the `.rs` files in `tests/ui`, and
// contains `.err` and `.out` files for stable
// - `tests/ui-msrv` - Contains symlinks to the `.rs` files in `tests/ui`, and
// contains `.err` and `.out` files for MSRV

#[rustversion::any(nightly, beta)]
const SOURCE_FILES_GLOB: &str = "tests/ui/*.rs";
#[rustversion::all(stable, not(stable(1.56.1)))]
const SOURCE_FILES_GLOB: &str = "tests/ui-stable/*.rs";
#[rustversion::stable(1.56.1)]
const SOURCE_FILES_GLOB: &str = "tests/ui-msrv/*.rs";

#[test]
fn ui() {
let t = trybuild::TestCases::new();
t.compile_fail("tests/ui/*.rs");
t.compile_fail(SOURCE_FILES_GLOB);
}
1 change: 1 addition & 0 deletions zerocopy-derive/tests/ui-msrv/enum.rs
173 changes: 173 additions & 0 deletions zerocopy-derive/tests/ui-msrv/enum.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
error: unrecognized representation hint
--> tests/ui-msrv/enum.rs:15:8
|
15 | #[repr("foo")]
| ^^^^^

error: unrecognized representation hint
--> tests/ui-msrv/enum.rs:21:8
|
21 | #[repr(foo)]
| ^^^

error: unsupported representation for deriving FromBytes, AsBytes, or Unaligned on an enum
--> tests/ui-msrv/enum.rs:27:8
|
27 | #[repr(transparent)]
| ^^^^^^^^^^^

error: conflicting representation hints
--> tests/ui-msrv/enum.rs:33:1
|
33 | #[repr(u8, u16)]
| ^

error: must have a non-align #[repr(...)] attribute in order to guarantee this type's memory layout
--> tests/ui-msrv/enum.rs:38:10
|
38 | #[derive(FromBytes)]
| ^^^^^^^^^
|
= note: this error originates in the derive macro `FromBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

error: FromBytes requires repr of "u8", "u16", "i8", or "i16"
--> tests/ui-msrv/enum.rs:48:8
|
48 | #[repr(C)]
| ^

error: FromBytes requires repr of "u8", "u16", "i8", or "i16"
--> tests/ui-msrv/enum.rs:54:8
|
54 | #[repr(usize)]
| ^^^^^

error: FromBytes requires repr of "u8", "u16", "i8", or "i16"
--> tests/ui-msrv/enum.rs:60:8
|
60 | #[repr(isize)]
| ^^^^^

error: FromBytes requires repr of "u8", "u16", "i8", or "i16"
--> tests/ui-msrv/enum.rs:66:8
|
66 | #[repr(u32)]
| ^^^

error: FromBytes requires repr of "u8", "u16", "i8", or "i16"
--> tests/ui-msrv/enum.rs:72:8
|
72 | #[repr(i32)]
| ^^^

error: FromBytes requires repr of "u8", "u16", "i8", or "i16"
--> tests/ui-msrv/enum.rs:78:8
|
78 | #[repr(u64)]
| ^^^

error: FromBytes requires repr of "u8", "u16", "i8", or "i16"
--> tests/ui-msrv/enum.rs:84:8
|
84 | #[repr(i64)]
| ^^^

error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1)))
--> tests/ui-msrv/enum.rs:94:8
|
94 | #[repr(C)]
| ^

error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1)))
--> tests/ui-msrv/enum.rs:100:8
|
100 | #[repr(u16)]
| ^^^

error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1)))
--> tests/ui-msrv/enum.rs:106:8
|
106 | #[repr(i16)]
| ^^^

error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1)))
--> tests/ui-msrv/enum.rs:112:8
|
112 | #[repr(u32)]
| ^^^

error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1)))
--> tests/ui-msrv/enum.rs:118:8
|
118 | #[repr(i32)]
| ^^^

error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1)))
--> tests/ui-msrv/enum.rs:124:8
|
124 | #[repr(u64)]
| ^^^

error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1)))
--> tests/ui-msrv/enum.rs:130:8
|
130 | #[repr(i64)]
| ^^^

error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1)))
--> tests/ui-msrv/enum.rs:136:8
|
136 | #[repr(usize)]
| ^^^^^

error: Unaligned requires repr of "u8" or "i8", and no alignment (i.e., repr(align(N > 1)))
--> tests/ui-msrv/enum.rs:142:8
|
142 | #[repr(isize)]
| ^^^^^

error: cannot derive Unaligned with repr(align(N > 1))
--> tests/ui-msrv/enum.rs:148:12
|
148 | #[repr(u8, align(2))]
| ^^^^^^^^

error: cannot derive Unaligned with repr(align(N > 1))
--> tests/ui-msrv/enum.rs:154:12
|
154 | #[repr(i8, align(2))]
| ^^^^^^^^

error: cannot derive Unaligned with repr(align(N > 1))
--> tests/ui-msrv/enum.rs:160:18
|
160 | #[repr(align(1), align(2))]
| ^^^^^^^^

error: cannot derive Unaligned with repr(align(N > 1))
--> tests/ui-msrv/enum.rs:166:8
|
166 | #[repr(align(2), align(4))]
| ^^^^^^^^

error[E0565]: meta item in `repr` must be an identifier
--> tests/ui-msrv/enum.rs:15:8
|
15 | #[repr("foo")]
| ^^^^^

error[E0552]: unrecognized representation hint
--> tests/ui-msrv/enum.rs:21:8
|
21 | #[repr(foo)]
| ^^^

error[E0566]: conflicting representation hints
--> tests/ui-msrv/enum.rs:33:8
|
33 | #[repr(u8, u16)]
| ^^ ^^^
|
= note: `#[deny(conflicting_repr_hints)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #68585 <https://github.com/rust-lang/rust/issues/68585>
11 changes: 11 additions & 0 deletions zerocopy-derive/tests/ui-msrv/enum_from_bytes_u8_too_few.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: FromBytes only supported on repr(u8) enum with 256 variants
--> tests/ui-msrv/enum_from_bytes_u8_too_few.rs:11:1
|
11 | / #[repr(u8)]
12 | | enum Foo {
13 | | Variant0,
14 | | Variant1,
... |
267 | | Variant254,
268 | | }
| |_^
1 change: 1 addition & 0 deletions zerocopy-derive/tests/ui-msrv/late_compile_pass.rs
66 changes: 66 additions & 0 deletions zerocopy-derive/tests/ui-msrv/late_compile_pass.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
error[E0277]: the trait bound `AsBytes1: HasPadding<false>` is not satisfied
--> tests/ui-msrv/late_compile_pass.rs:27:10
|
27 | #[derive(AsBytes)]
| ^^^^^^^ the trait `HasPadding<false>` is not implemented for `AsBytes1`
|
= help: the following implementations were found:
<AsBytes1 as HasPadding<true>>
note: required by a bound in `assert_no_padding`
--> tests/ui-msrv/late_compile_pass.rs:27:10
|
27 | #[derive(AsBytes)]
| ^^^^^^^ required by this bound in `assert_no_padding`
= note: this error originates in the derive macro `AsBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `&'static str: FromBytes` is not satisfied
--> tests/ui-msrv/late_compile_pass.rs:18:10
|
18 | #[derive(FromBytes)]
| ^^^^^^^^^ the trait `FromBytes` is not implemented for `&'static str`
|
note: required by a bound in `ImplementsFromBytes`
--> tests/ui-msrv/late_compile_pass.rs:18:10
|
18 | #[derive(FromBytes)]
| ^^^^^^^^^ required by this bound in `ImplementsFromBytes`
= note: this error originates in the derive macro `FromBytes` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `u16: Unaligned` is not satisfied
--> tests/ui-msrv/late_compile_pass.rs:38:10
|
38 | #[derive(Unaligned)]
| ^^^^^^^^^ the trait `Unaligned` is not implemented for `u16`
|
note: required by a bound in `<Unaligned1 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
--> tests/ui-msrv/late_compile_pass.rs:38:10
|
38 | #[derive(Unaligned)]
| ^^^^^^^^^ required by this bound in `<Unaligned1 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
= note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `u16: Unaligned` is not satisfied
--> tests/ui-msrv/late_compile_pass.rs:46:10
|
46 | #[derive(Unaligned)]
| ^^^^^^^^^ the trait `Unaligned` is not implemented for `u16`
|
note: required by a bound in `<Unaligned2 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
--> tests/ui-msrv/late_compile_pass.rs:46:10
|
46 | #[derive(Unaligned)]
| ^^^^^^^^^ required by this bound in `<Unaligned2 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
= note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `u16: Unaligned` is not satisfied
--> tests/ui-msrv/late_compile_pass.rs:53:10
|
53 | #[derive(Unaligned)]
| ^^^^^^^^^ the trait `Unaligned` is not implemented for `u16`
|
note: required by a bound in `<Unaligned3 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
--> tests/ui-msrv/late_compile_pass.rs:53:10
|
53 | #[derive(Unaligned)]
| ^^^^^^^^^ required by this bound in `<Unaligned3 as Unaligned>::only_derive_is_allowed_to_implement_this_trait::ImplementsUnaligned`
= note: this error originates in the derive macro `Unaligned` (in Nightly builds, run with -Z macro-backtrace for more info)
Loading

0 comments on commit 2c3bf40

Please sign in to comment.