Skip to content

Commit 961bf7f

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #136434 - RalfJung:rustc_allowed_through_unstable_modules-deprecation-required, r=compiler-errors
rustc_allowed_through_unstable_modules: require deprecation message This changes the `#[rustc_allowed_through_unstable_modules]` attribute so that a deprecation message (ideally directing people towards the stable path) is required.
2 parents 5bc5827 + 3320e91 commit 961bf7f

File tree

12 files changed

+107
-125
lines changed

12 files changed

+107
-125
lines changed

Diff for: ‎compiler/rustc_attr_data_structures/src/stability.rs

+3-12
Original file line numberDiff line numberDiff line change
@@ -101,16 +101,6 @@ impl PartialConstStability {
101101
}
102102
}
103103

104-
#[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)]
105-
#[derive(HashStable_Generic)]
106-
pub enum AllowedThroughUnstableModules {
107-
/// This does not get a deprecation warning. We still generally would prefer people to use the
108-
/// fully stable path, and a warning will likely be emitted in the future.
109-
WithoutDeprecation,
110-
/// Emit the given deprecation warning.
111-
WithDeprecation(Symbol),
112-
}
113-
114104
/// The available stability levels.
115105
#[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)]
116106
#[derive(HashStable_Generic)]
@@ -147,8 +137,9 @@ pub enum StabilityLevel {
147137
Stable {
148138
/// Rust release which stabilized this feature.
149139
since: StableSince,
150-
/// This is `Some` if this item allowed to be referred to on stable via unstable modules.
151-
allowed_through_unstable_modules: Option<AllowedThroughUnstableModules>,
140+
/// This is `Some` if this item allowed to be referred to on stable via unstable modules;
141+
/// the `Symbol` is the deprecation message printed in that case.
142+
allowed_through_unstable_modules: Option<Symbol>,
152143
},
153144
}
154145

Diff for: ‎compiler/rustc_attr_parsing/src/attributes/stability.rs

+11-7
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ use rustc_ast::MetaItem;
66
use rustc_ast::attr::AttributeExt;
77
use rustc_ast_pretty::pprust;
88
use rustc_attr_data_structures::{
9-
AllowedThroughUnstableModules, ConstStability, DefaultBodyStability, Stability, StabilityLevel,
10-
StableSince, UnstableReason, VERSION_PLACEHOLDER,
9+
ConstStability, DefaultBodyStability, Stability, StabilityLevel, StableSince, UnstableReason,
10+
VERSION_PLACEHOLDER,
1111
};
1212
use rustc_errors::ErrorGuaranteed;
1313
use rustc_session::Session;
14-
use rustc_span::{Span, Symbol, sym};
14+
use rustc_span::{Span, Symbol, kw, sym};
1515

1616
use crate::attributes::util::UnsupportedLiteralReason;
1717
use crate::{parse_version, session_diagnostics};
@@ -29,10 +29,14 @@ pub fn find_stability(
2929
for attr in attrs {
3030
match attr.name_or_empty() {
3131
sym::rustc_allowed_through_unstable_modules => {
32-
allowed_through_unstable_modules = Some(match attr.value_str() {
33-
Some(msg) => AllowedThroughUnstableModules::WithDeprecation(msg),
34-
None => AllowedThroughUnstableModules::WithoutDeprecation,
35-
})
32+
// The value is mandatory, but avoid ICEs in case such code reaches this function.
33+
allowed_through_unstable_modules = Some(attr.value_str().unwrap_or_else(|| {
34+
sess.dcx().span_delayed_bug(
35+
item_sp,
36+
"`#[rustc_allowed_through_unstable_modules]` without deprecation message",
37+
);
38+
kw::Empty
39+
}))
3640
}
3741
sym::unstable => {
3842
if stab.is_some() {

Diff for: ‎compiler/rustc_feature/src/builtin_attrs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
616616
EncodeCrossCrate::No, "allow_internal_unsafe side-steps the unsafe_code lint",
617617
),
618618
rustc_attr!(
619-
rustc_allowed_through_unstable_modules, Normal, template!(Word, NameValueStr: "deprecation message"),
619+
rustc_allowed_through_unstable_modules, Normal, template!(NameValueStr: "deprecation message"),
620620
WarnFollowing, EncodeCrossCrate::No,
621621
"rustc_allowed_through_unstable_modules special cases accidental stabilizations of stable items \
622622
through unstable paths"

Diff for: ‎compiler/rustc_passes/src/stability.rs

+75-79
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ use std::mem::replace;
55
use std::num::NonZero;
66

77
use rustc_attr_parsing::{
8-
self as attr, AllowedThroughUnstableModules, ConstStability, DeprecatedSince, Stability,
9-
StabilityLevel, StableSince, UnstableReason, VERSION_PLACEHOLDER,
8+
self as attr, ConstStability, DeprecatedSince, Stability, StabilityLevel, StableSince,
9+
UnstableReason, VERSION_PLACEHOLDER,
1010
};
1111
use rustc_data_structures::fx::FxIndexMap;
1212
use rustc_data_structures::unord::{ExtendUnord, UnordMap, UnordSet};
@@ -880,91 +880,87 @@ impl<'tcx> Visitor<'tcx> for Checker<'tcx> {
880880

881881
if item_is_allowed {
882882
// The item itself is allowed; check whether the path there is also allowed.
883-
let is_allowed_through_unstable_modules: Option<AllowedThroughUnstableModules> =
883+
let is_allowed_through_unstable_modules: Option<Symbol> =
884884
self.tcx.lookup_stability(def_id).and_then(|stab| match stab.level {
885885
StabilityLevel::Stable { allowed_through_unstable_modules, .. } => {
886886
allowed_through_unstable_modules
887887
}
888888
_ => None,
889889
});
890890

891-
if is_allowed_through_unstable_modules.is_none() {
892-
// Check parent modules stability as well if the item the path refers to is itself
893-
// stable. We only emit warnings for unstable path segments if the item is stable
894-
// or allowed because stability is often inherited, so the most common case is that
895-
// both the segments and the item are unstable behind the same feature flag.
896-
//
897-
// We check here rather than in `visit_path_segment` to prevent visiting the last
898-
// path segment twice
899-
//
900-
// We include special cases via #[rustc_allowed_through_unstable_modules] for items
901-
// that were accidentally stabilized through unstable paths before this check was
902-
// added, such as `core::intrinsics::transmute`
903-
let parents = path.segments.iter().rev().skip(1);
904-
for path_segment in parents {
905-
if let Some(def_id) = path_segment.res.opt_def_id() {
906-
// use `None` for id to prevent deprecation check
907-
self.tcx.check_stability_allow_unstable(
908-
def_id,
909-
None,
910-
path.span,
911-
None,
912-
if is_unstable_reexport(self.tcx, id) {
913-
AllowUnstable::Yes
914-
} else {
915-
AllowUnstable::No
916-
},
917-
);
918-
}
919-
}
920-
} else if let Some(AllowedThroughUnstableModules::WithDeprecation(deprecation)) =
921-
is_allowed_through_unstable_modules
922-
{
923-
// Similar to above, but we cannot use `check_stability_allow_unstable` as that would
924-
// immediately show the stability error. We just want to know the result and disaplay
925-
// our own kind of error.
926-
let parents = path.segments.iter().rev().skip(1);
927-
for path_segment in parents {
928-
if let Some(def_id) = path_segment.res.opt_def_id() {
929-
// use `None` for id to prevent deprecation check
930-
let eval_result = self.tcx.eval_stability_allow_unstable(
931-
def_id,
932-
None,
933-
path.span,
934-
None,
935-
if is_unstable_reexport(self.tcx, id) {
936-
AllowUnstable::Yes
937-
} else {
938-
AllowUnstable::No
939-
},
940-
);
941-
let is_allowed = matches!(eval_result, EvalResult::Allow);
942-
if !is_allowed {
943-
// Calculating message for lint involves calling `self.def_path_str`,
944-
// which will by default invoke the expensive `visible_parent_map` query.
945-
// Skip all that work if the lint is allowed anyway.
946-
if self.tcx.lint_level_at_node(DEPRECATED, id).0
947-
== lint::Level::Allow
948-
{
949-
return;
950-
}
951-
// Show a deprecation message.
952-
let def_path =
953-
with_no_trimmed_paths!(self.tcx.def_path_str(def_id));
954-
let def_kind = self.tcx.def_descr(def_id);
955-
let diag = Deprecated {
956-
sub: None,
957-
kind: def_kind.to_owned(),
958-
path: def_path,
959-
note: Some(deprecation),
960-
since_kind: lint::DeprecatedSinceKind::InEffect,
961-
};
962-
self.tcx.emit_node_span_lint(
963-
DEPRECATED,
964-
id,
965-
method_span.unwrap_or(path.span),
966-
diag,
891+
// Check parent modules stability as well if the item the path refers to is itself
892+
// stable. We only emit errors for unstable path segments if the item is stable
893+
// or allowed because stability is often inherited, so the most common case is that
894+
// both the segments and the item are unstable behind the same feature flag.
895+
//
896+
// We check here rather than in `visit_path_segment` to prevent visiting the last
897+
// path segment twice
898+
//
899+
// We include special cases via #[rustc_allowed_through_unstable_modules] for items
900+
// that were accidentally stabilized through unstable paths before this check was
901+
// added, such as `core::intrinsics::transmute`
902+
let parents = path.segments.iter().rev().skip(1);
903+
for path_segment in parents {
904+
if let Some(def_id) = path_segment.res.opt_def_id() {
905+
match is_allowed_through_unstable_modules {
906+
None => {
907+
// Emit a hard stability error if this path is not stable.
908+
909+
// use `None` for id to prevent deprecation check
910+
self.tcx.check_stability_allow_unstable(
911+
def_id,
912+
None,
913+
path.span,
914+
None,
915+
if is_unstable_reexport(self.tcx, id) {
916+
AllowUnstable::Yes
917+
} else {
918+
AllowUnstable::No
919+
},
920+
);
921+
}
922+
Some(deprecation) => {
923+
// Call the stability check directly so that we can control which
924+
// diagnostic is emitted.
925+
let eval_result = self.tcx.eval_stability_allow_unstable(
926+
def_id,
927+
None,
928+
path.span,
929+
None,
930+
if is_unstable_reexport(self.tcx, id) {
931+
AllowUnstable::Yes
932+
} else {
933+
AllowUnstable::No
934+
},
967935
);
936+
let is_allowed = matches!(eval_result, EvalResult::Allow);
937+
if !is_allowed {
938+
// Calculating message for lint involves calling `self.def_path_str`,
939+
// which will by default invoke the expensive `visible_parent_map` query.
940+
// Skip all that work if the lint is allowed anyway.
941+
if self.tcx.lint_level_at_node(DEPRECATED, id).0
942+
== lint::Level::Allow
943+
{
944+
return;
945+
}
946+
// Show a deprecation message.
947+
let def_path =
948+
with_no_trimmed_paths!(self.tcx.def_path_str(def_id));
949+
let def_kind = self.tcx.def_descr(def_id);
950+
let diag = Deprecated {
951+
sub: None,
952+
kind: def_kind.to_owned(),
953+
path: def_path,
954+
note: Some(deprecation),
955+
since_kind: lint::DeprecatedSinceKind::InEffect,
956+
};
957+
self.tcx.emit_node_span_lint(
958+
DEPRECATED,
959+
id,
960+
method_span.unwrap_or(path.span),
961+
diag,
962+
);
963+
}
968964
}
969965
}
970966
}

Diff for: ‎library/core/src/intrinsics/mod.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,11 @@ pub mod simd;
7878
use crate::sync::atomic::{self, AtomicBool, AtomicI32, AtomicIsize, AtomicU32, Ordering};
7979

8080
#[stable(feature = "drop_in_place", since = "1.8.0")]
81-
#[rustc_allowed_through_unstable_modules]
81+
#[cfg_attr(bootstrap, rustc_allowed_through_unstable_modules)]
82+
#[cfg_attr(
83+
not(bootstrap),
84+
rustc_allowed_through_unstable_modules = "import this function via `std::ptr` instead"
85+
)]
8286
#[deprecated(note = "no longer an intrinsic - use `ptr::drop_in_place` directly", since = "1.52.0")]
8387
#[inline]
8488
pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {

Diff for: ‎src/librustdoc/clean/types.rs

+2-10
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ use std::{fmt, iter};
55

66
use arrayvec::ArrayVec;
77
use rustc_abi::{ExternAbi, VariantIdx};
8-
use rustc_attr_parsing::{
9-
AllowedThroughUnstableModules, ConstStability, Deprecation, Stability, StableSince,
10-
};
8+
use rustc_attr_parsing::{ConstStability, Deprecation, Stability, StableSince};
119
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
1210
use rustc_hir::def::{CtorKind, DefKind, Res};
1311
use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId};
@@ -411,15 +409,9 @@ impl Item {
411409
..
412410
} = stab.level
413411
{
414-
let note = match note {
415-
AllowedThroughUnstableModules::WithDeprecation(note) => Some(note),
416-
// FIXME: Would be better to say *something* here about the *path* being
417-
// deprecated rather than the item.
418-
AllowedThroughUnstableModules::WithoutDeprecation => None,
419-
};
420412
Some(Deprecation {
421413
since: rustc_attr_parsing::DeprecatedSince::Unspecified,
422-
note,
414+
note: Some(note),
423415
suggestion: None,
424416
})
425417
} else {

Diff for: ‎tests/rustdoc/inline_local/fully-stable-path-is-better.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ pub mod stb1 {
1616
#[unstable(feature = "uns", issue = "135003")]
1717
pub mod uns {
1818
#[stable(since = "1.0", feature = "stb1")]
19-
#[rustc_allowed_through_unstable_modules]
19+
#[rustc_allowed_through_unstable_modules = "use stable path instead"]
2020
pub struct Inside1;
2121
#[stable(since = "1.0", feature = "stb2")]
22-
#[rustc_allowed_through_unstable_modules]
22+
#[rustc_allowed_through_unstable_modules = "use stable path instead"]
2323
pub struct Inside2;
2424
}
2525

Diff for: ‎tests/rustdoc/stability.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ pub mod stable_later {
8585
}
8686

8787
#[stable(feature = "rust1", since = "1.0.0")]
88-
#[rustc_allowed_through_unstable_modules]
88+
#[rustc_allowed_through_unstable_modules = "use stable path instead"]
8989
pub mod stable_earlier1 {
9090
//@ has stability/stable_earlier1/struct.StableInUnstable.html \
9191
// '//div[@class="main-heading"]//span[@class="since"]' '1.0.0'

Diff for: ‎tests/ui/error-codes/E0789.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#![feature(staged_api)]
55
#![unstable(feature = "foo_module", reason = "...", issue = "123")]
66

7-
#[rustc_allowed_through_unstable_modules]
7+
#[rustc_allowed_through_unstable_modules = "use stable path instead"]
88
// #[stable(feature = "foo", since = "1.0")]
99
struct Foo;
1010
//~^ ERROR `rustc_allowed_through_unstable_modules` attribute must be paired with a `stable` attribute

Diff for: ‎tests/ui/stability-attribute/allowed-through-unstable.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,5 @@
55

66
extern crate allowed_through_unstable_core;
77

8-
use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstable;
9-
use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstableWithDeprecation; //~WARN use of deprecated module `allowed_through_unstable_core::unstable_module`: use the new path instead
8+
use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstable; //~WARN use of deprecated module `allowed_through_unstable_core::unstable_module`: use the new path instead
109
use allowed_through_unstable_core::unstable_module::NewStableTraitNotAllowedThroughUnstable; //~ ERROR use of unstable library feature `unstable_test_feature`

Diff for: ‎tests/ui/stability-attribute/allowed-through-unstable.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
warning: use of deprecated module `allowed_through_unstable_core::unstable_module`: use the new path instead
2-
--> $DIR/allowed-through-unstable.rs:9:53
2+
--> $DIR/allowed-through-unstable.rs:8:53
33
|
4-
LL | use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstableWithDeprecation;
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4+
LL | use allowed_through_unstable_core::unstable_module::OldStableTraitAllowedThoughUnstable;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `#[warn(deprecated)]` on by default
88

99
error[E0658]: use of unstable library feature `unstable_test_feature`
10-
--> $DIR/allowed-through-unstable.rs:10:5
10+
--> $DIR/allowed-through-unstable.rs:9:5
1111
|
1212
LL | use allowed_through_unstable_core::unstable_module::NewStableTraitNotAllowedThroughUnstable;
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Diff for: ‎tests/ui/stability-attribute/auxiliary/allowed-through-unstable-core.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,9 @@
55

66
#[unstable(feature = "unstable_test_feature", issue = "1")]
77
pub mod unstable_module {
8-
#[stable(feature = "stable_test_feature", since = "1.2.0")]
9-
#[rustc_allowed_through_unstable_modules]
10-
pub trait OldStableTraitAllowedThoughUnstable {}
11-
128
#[stable(feature = "stable_test_feature", since = "1.2.0")]
139
#[rustc_allowed_through_unstable_modules = "use the new path instead"]
14-
pub trait OldStableTraitAllowedThoughUnstableWithDeprecation {}
10+
pub trait OldStableTraitAllowedThoughUnstable {}
1511

1612
#[stable(feature = "stable_test_feature", since = "1.2.0")]
1713
pub trait NewStableTraitNotAllowedThroughUnstable {}

0 commit comments

Comments
 (0)