Disable re-exporting renamed traits under their old names in development and CI #960
Comments
joshlf
added a commit
that referenced
this issue
Feb 29, 2024
We should have done this as part of #960 originally; without it, our rollers produce .stderr files which are rejected by CI.
github-merge-queue bot
pushed a commit
that referenced
this issue
Feb 29, 2024
We should have done this as part of #960 originally; without it, our rollers produce .stderr files which are rejected by CI.
joshlf
added a commit
that referenced
this issue
Mar 1, 2024
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.
joshlf
added a commit
that referenced
this issue
Mar 1, 2024
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 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.
github-merge-queue bot
pushed a commit
that referenced
this issue
Mar 1, 2024
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 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In 0.8, we rename
AsBytestoIntoBytesandFromZeroestoFromZeros. In order to ease the transition to 0.8, we also export the old names via#[doc(hidden)] #[deprecated] pub use IntoBytes as AsBytes(and likewise forpub use FromZeros as FromZeroes).This causes problems for our UI tests. In particular, on our MSRV toolchain (1.56 as of this writing), error message suggestions sometimes use this
pub useinstead of the original name (i.e., suggestingAsBytesinstead ofIntoBytes). This on its own wouldn't be a problem, but the behavior seems to differ by platform - namely, when running on Mac, different error messages are generated than when running on Linux. This makes it easy to update these files during development in a way that won't pass CI (for example, when developing on Mac, since CI currently runs on Linux).If we simply disable these
pub usestatements when doing development and in CI, it should solve this problem. We already have machinery in ourcargo-zerocopyto emit custom--cfgflags, so adding another one would be a very small amount of work.The text was updated successfully, but these errors were encountered: