Skip to content

Commit

Permalink
Make old names errors instead of warnings
Browse files Browse the repository at this point in the history
Previously, we re-exported renamed traits as their old names via `pub
use`. Despite including `#[deprecated]` attributes on each `pub use`, it
still caused problems for us as described in #960.

In this commit, we replace `pub use` with just `use`. This has the
effect of making it so that using the old names is an error rather than
a warning, which accomplishes two things: first, it ensures that the
issue described in #960 can't happen, as the compiler will never
generate suggestions to write code which generates errors. Second, it
ensures that callers are forced to update to the new names, which makes
it less likely for us to cause breakage when we eventually remove the
`use` path.

Note that, despite no longer having an explicit deprecation warning that
explains what's happening, the error message still provides plenty of
breadcrumb as to what happened. Consider this example error message:

  error[E0603]: trait `FromZeroes` is private
      --> examples/deprecated.rs:1:15
       |
  1    | use zerocopy::FromZeroes;
       |               ^^^^^^^^^^ private trait
       |
  note: the trait `FromZeroes` is defined here
      --> /Users/josh/workspace/zerocopy/src/lib.rs:1845:5
       |
  1845 | use FromZeros as FromZeroes;

The fact that the original source code is included makes it easy to see
what's gone wrong - there's another item called `FromZeros` that can be
used instead.
  • Loading branch information
joshlf committed Mar 1, 2024
1 parent 2b1a996 commit 2babcd4
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 20 deletions.
7 changes: 2 additions & 5 deletions .github/workflows/roll-pinned-toolchain-versions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,9 @@ jobs:
cargo "+$VERSION_FOR_CARGO" fix --allow-dirty --tests --package zerocopy $ZEROCOPY_FEATURES || true
cargo "+$VERSION_FOR_CARGO" fix --allow-dirty --tests --package zerocopy-derive || true
# See #960 for an explanation of this `--cfg`.
RUSTFLAGS="--cfg __INTERNAL_USE_ONLY_DISABLE_DEPRECATED_TRAIT_ALIASES"
# Update `.stderr` files as needed for the new version.
RUSTFLAGS="$RUSTFLAGS" TRYBUILD=overwrite cargo "+$VERSION_FOR_CARGO" test --package zerocopy $ZEROCOPY_FEATURES
RUSTFLAGS="$RUSTFLAGS" TRYBUILD=overwrite cargo "+$VERSION_FOR_CARGO" test --package zerocopy-derive
TRYBUILD=overwrite cargo "+$VERSION_FOR_CARGO" test --package zerocopy $ZEROCOPY_FEATURES
TRYBUILD=overwrite cargo "+$VERSION_FOR_CARGO" test --package zerocopy-derive
}
if [ "${{ matrix.toolchain }}" == stable ]; then
Expand Down
50 changes: 39 additions & 11 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1823,12 +1823,26 @@ pub unsafe trait FromZeros: TryFromBytes {
}
}

/// Deprecated: prefer [`FromZeros`] instead.
#[deprecated(since = "0.8.0", note = "`FromZeroes` was renamed to `FromZeros`")]
#[doc(hidden)]
// See #960 for why we do this.
#[cfg(not(__INTERNAL_USE_ONLY_DISABLE_DEPRECATED_TRAIT_ALIASES))]
pub use FromZeros as FromZeroes;
// This exists so that code which was written against the old name will get a
// less confusing error message when they upgrade to a more recent version of
// zerocopy. On our MSRV toolchain, the error message reads:
//
// error[E0603]: trait `FromZeroes` is private
// --> examples/deprecated.rs:1:15
// |
// 1 | use zerocopy::FromZeroes;
// | ^^^^^^^^^^ private trait
// |
// note: the trait `FromZeroes` is defined here
// --> /Users/josh/workspace/zerocopy/src/lib.rs:1845:5
// |
// 1845 | use FromZeros as FromZeroes;
// | ^^^^^^^^^^^^^^^^^^^^^^^
//
// The "note" provides enough context to make it easy to figure out how to fix
// the error.
#[allow(unused)]
use FromZeros as FromZeroes;

/// Analyzes whether a type is [`FromBytes`].
///
Expand Down Expand Up @@ -3180,11 +3194,25 @@ pub unsafe trait IntoBytes {
}
}

/// Deprecated: prefer [`IntoBytes`] instead.
#[deprecated(since = "0.8.0", note = "`AsBytes` was renamed to `IntoBytes`")]
#[doc(hidden)]
// See #960 for why we do this.
#[cfg(not(__INTERNAL_USE_ONLY_DISABLE_DEPRECATED_TRAIT_ALIASES))]
// This exists so that code which was written against the old name will get a
// less confusing error message when they upgrade to a more recent version of
// zerocopy. On our MSRV toolchain, the error message reads:
//
// error[E0603]: trait `AsBytes` is private
// --> examples/deprecated.rs:1:15
// |
// 1 | use zerocopy::AsBytes;
// | ^^^^^^^ private trait
// |
// note: the trait `AsBytes` is defined here
// --> /Users/josh/workspace/zerocopy/src/lib.rs:3216:5
// |
// 3216 | use IntoBytes as AsBytes;
// | ^^^^^^^^^^^^^^^^^^^^
//
// The "note" provides enough context to make it easy to figure out how to fix
// the error.
#[allow(unused)]
pub use IntoBytes as AsBytes;

/// Types with no alignment requirement.
Expand Down
6 changes: 2 additions & 4 deletions tools/cargo-zerocopy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,10 @@ fn install_toolchain_or_exit(versions: &Versions, name: &str) -> Result<(), Erro
}

fn get_rustflags(name: &str) -> &'static str {
// See #960 for an explanation of why we emit --cfg
// __INTERNAL_USE_ONLY_DISABLE_DEPRECATED_TRAIT_ALIASES.
if name == "nightly" {
"--cfg __INTERNAL_USE_ONLY_NIGHTLY_FEATURES_IN_TESTS --cfg __INTERNAL_USE_ONLY_DISABLE_DEPRECATED_TRAIT_ALIASES "
"--cfg __INTERNAL_USE_ONLY_NIGHTLY_FEATURES_IN_TESTS "
} else {
"--cfg __INTERNAL_USE_ONLY_DISABLE_DEPRECATED_TRAIT_ALIASES "
""
}
}

Expand Down

0 comments on commit 2babcd4

Please sign in to comment.