Skip to content

Commit

Permalink
rustc_allowed_through_unstable_modules: require deprecation message
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Feb 2, 2025
1 parent 8239a37 commit 55bb0d9
Show file tree
Hide file tree
Showing 12 changed files with 101 additions and 125 deletions.
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
12 changes: 5 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,8 @@ 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(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

0 comments on commit 55bb0d9

Please sign in to comment.