Skip to content

rustc_allowed_through_unstable_modules: require deprecation message #136434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions compiler/rustc_attr_data_structures/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,6 @@ impl PartialConstStability {
}
}

#[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)]
#[derive(HashStable_Generic)]
pub enum AllowedThroughUnstableModules {
/// This does not get a deprecation warning. We still generally would prefer people to use the
/// fully stable path, and a warning will likely be emitted in the future.
WithoutDeprecation,
/// Emit the given deprecation warning.
WithDeprecation(Symbol),
}

/// The available stability levels.
#[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)]
#[derive(HashStable_Generic)]
Expand Down Expand Up @@ -147,8 +137,9 @@ pub enum StabilityLevel {
Stable {
/// Rust release which stabilized this feature.
since: StableSince,
/// This is `Some` if this item allowed to be referred to on stable via unstable modules.
allowed_through_unstable_modules: Option<AllowedThroughUnstableModules>,
/// This is `Some` if this item allowed to be referred to on stable via unstable modules;
/// the `Symbol` is the deprecation message printed in that case.
allowed_through_unstable_modules: Option<Symbol>,
},
}

Expand Down
18 changes: 11 additions & 7 deletions compiler/rustc_attr_parsing/src/attributes/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ use rustc_ast::MetaItem;
use rustc_ast::attr::AttributeExt;
use rustc_ast_pretty::pprust;
use rustc_attr_data_structures::{
AllowedThroughUnstableModules, ConstStability, DefaultBodyStability, Stability, StabilityLevel,
StableSince, UnstableReason, VERSION_PLACEHOLDER,
ConstStability, DefaultBodyStability, Stability, StabilityLevel, StableSince, UnstableReason,
VERSION_PLACEHOLDER,
};
use rustc_errors::ErrorGuaranteed;
use rustc_session::Session;
use rustc_span::{Span, Symbol, sym};
use rustc_span::{Span, Symbol, kw, sym};

use crate::attributes::util::UnsupportedLiteralReason;
use crate::{parse_version, session_diagnostics};
Expand All @@ -29,10 +29,14 @@ pub fn find_stability(
for attr in attrs {
match attr.name_or_empty() {
sym::rustc_allowed_through_unstable_modules => {
allowed_through_unstable_modules = Some(match attr.value_str() {
Some(msg) => AllowedThroughUnstableModules::WithDeprecation(msg),
None => AllowedThroughUnstableModules::WithoutDeprecation,
})
// The value is mandatory, but avoid ICEs in case such code reaches this function.
allowed_through_unstable_modules = Some(attr.value_str().unwrap_or_else(|| {
sess.dcx().span_delayed_bug(
item_sp,
"`#[rustc_allowed_through_unstable_modules]` without deprecation message",
);
kw::Empty
}))
}
sym::unstable => {
if stab.is_some() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
EncodeCrossCrate::No, "allow_internal_unsafe side-steps the unsafe_code lint",
),
rustc_attr!(
rustc_allowed_through_unstable_modules, Normal, template!(Word, NameValueStr: "deprecation message"),
rustc_allowed_through_unstable_modules, Normal, template!(NameValueStr: "deprecation message"),
WarnFollowing, EncodeCrossCrate::No,
"rustc_allowed_through_unstable_modules special cases accidental stabilizations of stable items \
through unstable paths"
Expand Down
154 changes: 75 additions & 79 deletions compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use std::mem::replace;
use std::num::NonZero;

use rustc_attr_parsing::{
self as attr, AllowedThroughUnstableModules, ConstStability, DeprecatedSince, Stability,
StabilityLevel, StableSince, UnstableReason, VERSION_PLACEHOLDER,
self as attr, ConstStability, DeprecatedSince, Stability, StabilityLevel, StableSince,
UnstableReason, VERSION_PLACEHOLDER,
};
use rustc_data_structures::fx::FxIndexMap;
use rustc_data_structures::unord::{ExtendUnord, UnordMap, UnordSet};
Expand Down Expand Up @@ -880,91 +880,87 @@ impl<'tcx> Visitor<'tcx> for Checker<'tcx> {

if item_is_allowed {
// The item itself is allowed; check whether the path there is also allowed.
let is_allowed_through_unstable_modules: Option<AllowedThroughUnstableModules> =
let is_allowed_through_unstable_modules: Option<Symbol> =
self.tcx.lookup_stability(def_id).and_then(|stab| match stab.level {
StabilityLevel::Stable { allowed_through_unstable_modules, .. } => {
allowed_through_unstable_modules
}
_ => None,
});

if is_allowed_through_unstable_modules.is_none() {
// Check parent modules stability as well if the item the path refers to is itself
// stable. We only emit warnings for unstable path segments if the item is stable
// or allowed because stability is often inherited, so the most common case is that
// both the segments and the item are unstable behind the same feature flag.
//
// We check here rather than in `visit_path_segment` to prevent visiting the last
// path segment twice
//
// We include special cases via #[rustc_allowed_through_unstable_modules] for items
// that were accidentally stabilized through unstable paths before this check was
// added, such as `core::intrinsics::transmute`
let parents = path.segments.iter().rev().skip(1);
for path_segment in parents {
if let Some(def_id) = path_segment.res.opt_def_id() {
// use `None` for id to prevent deprecation check
self.tcx.check_stability_allow_unstable(
def_id,
None,
path.span,
None,
if is_unstable_reexport(self.tcx, id) {
AllowUnstable::Yes
} else {
AllowUnstable::No
},
);
}
}
} else if let Some(AllowedThroughUnstableModules::WithDeprecation(deprecation)) =
is_allowed_through_unstable_modules
{
// Similar to above, but we cannot use `check_stability_allow_unstable` as that would
// immediately show the stability error. We just want to know the result and disaplay
// our own kind of error.
let parents = path.segments.iter().rev().skip(1);
for path_segment in parents {
if let Some(def_id) = path_segment.res.opt_def_id() {
// use `None` for id to prevent deprecation check
let eval_result = self.tcx.eval_stability_allow_unstable(
def_id,
None,
path.span,
None,
if is_unstable_reexport(self.tcx, id) {
AllowUnstable::Yes
} else {
AllowUnstable::No
},
);
let is_allowed = matches!(eval_result, EvalResult::Allow);
if !is_allowed {
// Calculating message for lint involves calling `self.def_path_str`,
// which will by default invoke the expensive `visible_parent_map` query.
// Skip all that work if the lint is allowed anyway.
if self.tcx.lint_level_at_node(DEPRECATED, id).0
== lint::Level::Allow
{
return;
}
// Show a deprecation message.
let def_path =
with_no_trimmed_paths!(self.tcx.def_path_str(def_id));
let def_kind = self.tcx.def_descr(def_id);
let diag = Deprecated {
sub: None,
kind: def_kind.to_owned(),
path: def_path,
note: Some(deprecation),
since_kind: lint::DeprecatedSinceKind::InEffect,
};
self.tcx.emit_node_span_lint(
DEPRECATED,
id,
method_span.unwrap_or(path.span),
diag,
// Check parent modules stability as well if the item the path refers to is itself
// stable. We only emit errors for unstable path segments if the item is stable
// or allowed because stability is often inherited, so the most common case is that
// both the segments and the item are unstable behind the same feature flag.
//
// We check here rather than in `visit_path_segment` to prevent visiting the last
// path segment twice
//
// We include special cases via #[rustc_allowed_through_unstable_modules] for items
// that were accidentally stabilized through unstable paths before this check was
// added, such as `core::intrinsics::transmute`
let parents = path.segments.iter().rev().skip(1);
for path_segment in parents {
if let Some(def_id) = path_segment.res.opt_def_id() {
match is_allowed_through_unstable_modules {
None => {
// Emit a hard stability error if this path is not stable.

// use `None` for id to prevent deprecation check
self.tcx.check_stability_allow_unstable(
def_id,
None,
path.span,
None,
if is_unstable_reexport(self.tcx, id) {
AllowUnstable::Yes
} else {
AllowUnstable::No
},
);
}
Some(deprecation) => {
// Call the stability check directly so that we can control which
// diagnostic is emitted.
let eval_result = self.tcx.eval_stability_allow_unstable(
def_id,
None,
path.span,
None,
if is_unstable_reexport(self.tcx, id) {
AllowUnstable::Yes
} else {
AllowUnstable::No
},
);
let is_allowed = matches!(eval_result, EvalResult::Allow);
if !is_allowed {
// Calculating message for lint involves calling `self.def_path_str`,
// which will by default invoke the expensive `visible_parent_map` query.
// Skip all that work if the lint is allowed anyway.
if self.tcx.lint_level_at_node(DEPRECATED, id).0
== lint::Level::Allow
{
return;
}
// Show a deprecation message.
let def_path =
with_no_trimmed_paths!(self.tcx.def_path_str(def_id));
let def_kind = self.tcx.def_descr(def_id);
let diag = Deprecated {
sub: None,
kind: def_kind.to_owned(),
path: def_path,
note: Some(deprecation),
since_kind: lint::DeprecatedSinceKind::InEffect,
};
self.tcx.emit_node_span_lint(
DEPRECATED,
id,
method_span.unwrap_or(path.span),
diag,
);
}
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion library/core/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ pub mod simd;
use crate::sync::atomic::{self, AtomicBool, AtomicI32, AtomicIsize, AtomicU32, Ordering};

#[stable(feature = "drop_in_place", since = "1.8.0")]
#[rustc_allowed_through_unstable_modules]
#[cfg_attr(bootstrap, rustc_allowed_through_unstable_modules)]
#[cfg_attr(
not(bootstrap),
rustc_allowed_through_unstable_modules = "import this function via `std::ptr` instead"
)]
#[deprecated(note = "no longer an intrinsic - use `ptr::drop_in_place` directly", since = "1.52.0")]
#[inline]
pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
Expand Down
12 changes: 2 additions & 10 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ use std::{fmt, iter};

use arrayvec::ArrayVec;
use rustc_abi::{ExternAbi, VariantIdx};
use rustc_attr_parsing::{
AllowedThroughUnstableModules, ConstStability, Deprecation, Stability, StableSince,
};
use rustc_attr_parsing::{ConstStability, Deprecation, Stability, StableSince};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId};
Expand Down Expand Up @@ -411,15 +409,9 @@ impl Item {
..
} = stab.level
{
let note = match note {
AllowedThroughUnstableModules::WithDeprecation(note) => Some(note),
// FIXME: Would be better to say *something* here about the *path* being
// deprecated rather than the item.
AllowedThroughUnstableModules::WithoutDeprecation => None,
};
Some(Deprecation {
since: rustc_attr_parsing::DeprecatedSince::Unspecified,
note,
note: Some(note),
suggestion: None,
})
} else {
Expand Down
4 changes: 2 additions & 2 deletions tests/rustdoc/inline_local/fully-stable-path-is-better.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ pub mod stb1 {
#[unstable(feature = "uns", issue = "135003")]
pub mod uns {
#[stable(since = "1.0", feature = "stb1")]
#[rustc_allowed_through_unstable_modules]
#[rustc_allowed_through_unstable_modules = "use stable path instead"]
pub struct Inside1;
#[stable(since = "1.0", feature = "stb2")]
#[rustc_allowed_through_unstable_modules]
#[rustc_allowed_through_unstable_modules = "use stable path instead"]
pub struct Inside2;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/rustdoc/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub mod stable_later {
}

#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_allowed_through_unstable_modules]
#[rustc_allowed_through_unstable_modules = "use stable path instead"]
pub mod stable_earlier1 {
//@ has stability/stable_earlier1/struct.StableInUnstable.html \
// '//div[@class="main-heading"]//span[@class="since"]' '1.0.0'
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/error-codes/E0789.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#![feature(staged_api)]
#![unstable(feature = "foo_module", reason = "...", issue = "123")]

#[rustc_allowed_through_unstable_modules]
#[rustc_allowed_through_unstable_modules = "use stable path instead"]
// #[stable(feature = "foo", since = "1.0")]
struct Foo;
//~^ ERROR `rustc_allowed_through_unstable_modules` attribute must be paired with a `stable` attribute
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/stability-attribute/allowed-through-unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@

extern crate allowed_through_unstable_core;

use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstable;
use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstableWithDeprecation; //~WARN use of deprecated module `allowed_through_unstable_core::unstable_module`: use the new path instead
use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstable; //~WARN use of deprecated module `allowed_through_unstable_core::unstable_module`: use the new path instead
use allowed_through_unstable_core::unstable_module::NewStableTraitNotAllowedThroughUnstable; //~ ERROR use of unstable library feature `unstable_test_feature`
8 changes: 4 additions & 4 deletions tests/ui/stability-attribute/allowed-through-unstable.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
warning: use of deprecated module `allowed_through_unstable_core::unstable_module`: use the new path instead
--> $DIR/allowed-through-unstable.rs:9:53
--> $DIR/allowed-through-unstable.rs:8:53
|
LL | use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstableWithDeprecation;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstable;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(deprecated)]` on by default

error[E0658]: use of unstable library feature `unstable_test_feature`
--> $DIR/allowed-through-unstable.rs:10:5
--> $DIR/allowed-through-unstable.rs:9:5
|
LL | use allowed_through_unstable_core::unstable_module::NewStableTraitNotAllowedThroughUnstable;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,9 @@

#[unstable(feature = "unstable_test_feature", issue = "1")]
pub mod unstable_module {
#[stable(feature = "stable_test_feature", since = "1.2.0")]
#[rustc_allowed_through_unstable_modules]
pub trait OldStableTraitAllowedThoughUnstable {}

#[stable(feature = "stable_test_feature", since = "1.2.0")]
#[rustc_allowed_through_unstable_modules = "use the new path instead"]
pub trait OldStableTraitAllowedThoughUnstableWithDeprecation {}
pub trait OldStableTraitAllowedThoughUnstable {}

#[stable(feature = "stable_test_feature", since = "1.2.0")]
pub trait NewStableTraitNotAllowedThroughUnstable {}
Expand Down
Loading