Skip to content

Commit 857afc7

Browse files
authored
Rollup merge of rust-lang#99212 - davidtwco:partial-stability-implies, r=michaelwoerister
introduce `implied_by` in `#[unstable]` attribute Requested by the library team [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/better.20support.20for.20partial.20stabilizations/near/285581519). If part of a feature is stabilized and a new feature is added for the remaining parts, then the `implied_by` meta-item can be added to `#[unstable]` to indicate which now-stable feature was used previously. ```diagnostic error: the feature `foo` has been partially stabilized since 1.62.0 and is succeeded by the feature `foobar` --> $DIR/stability-attribute-implies-using-unstable.rs:3:12 | LL | #![feature(foo)] | ^^^ | note: the lint level is defined here --> $DIR/stability-attribute-implies-using-stable.rs:2:9 | LL | #![deny(stable_features)] | ^^^^^^^^^^^^^^^ help: if you are using features which are still unstable, change to using `foobar` | LL | #![feature(foobar)] | ~~~~~~ help: if you are using features which are now stable, remove this line | LL - #![feature(foo)] | ``` When a `#![feature(..)]` attribute still exists for the now-stable attribute, then there this has two effects: - There will not be an stability error for uses of items from the implied feature which are still unstable (until the `#![feature(..)]` is removed or updated to the new feature). - There will be an improved diagnostic for the remaining use of the feature attribute for the now-stable feature. ```rust /// If part of a feature is stabilized and a new feature is added for the remaining parts, /// then the `implied_by` attribute is used to indicate which now-stable feature previously /// contained a item. /// /// ```pseudo-Rust /// #[unstable(feature = "foo", issue = "...")] /// fn foo() {} /// #[unstable(feature = "foo", issue = "...")] /// fn foobar() {} /// ``` /// /// ...becomes... /// /// ```pseudo-Rust /// #[stable(feature = "foo", since = "1.XX.X")] /// fn foo() {} /// #[unstable(feature = "foobar", issue = "...", implied_by = "foo")] /// fn foobar() {} /// ``` ``` In the Zulip discussion, this was envisioned as `implies` on `#[stable]` but I went with `implied_by` on `#[unstable]` because it means that only the unstable attribute needs to be changed in future, not the new stable attribute, which seems less error-prone. It also isn't particularly feasible for me to detect whether items from the implied feature are used and then only suggest updating _or_ removing the `#![feature(..)]` as appropriate, so I always do both. There's some new information in the cross-crate metadata as a result of this change, that's a little unfortunate, but without requiring that the `#[unstable]` and `#[stable]` attributes both contain the implication information, it's necessary: ```rust /// This mapping is necessary unless both the `#[stable]` and `#[unstable]` attributes should /// specify their implications (both `implies` and `implied_by`). If only one of the two /// attributes do (as in the current implementation, `implied_by` in `#[unstable]`), then this /// mapping is necessary for diagnostics. When a "unnecessary feature attribute" error is /// reported, only the `#[stable]` attribute information is available, so the map is necessary /// to know that the feature implies another feature. If it were reversed, and the `#[stable]` /// attribute had an `implies` meta item, then a map would be necessary when avoiding a "use of /// unstable feature" error for a feature that was implied. ``` I also change some comments to documentation comments in the compiler, add a helper for going from a `Span` to a `Span` for the entire line, and fix a incorrect part of the pre-existing stability attribute diagnostics. cc `@yaahc`
2 parents 14dbfeb + e587299 commit 857afc7

24 files changed

+357
-62
lines changed

compiler/rustc_attr/src/builtin.rs

+44-5
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,42 @@ impl ConstStability {
135135
#[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)]
136136
#[derive(HashStable_Generic)]
137137
pub enum StabilityLevel {
138-
// Reason for the current stability level and the relevant rust-lang issue
139-
Unstable { reason: Option<Symbol>, issue: Option<NonZeroU32>, is_soft: bool },
140-
Stable { since: Symbol, allowed_through_unstable_modules: bool },
138+
/// `#[unstable]`
139+
Unstable {
140+
/// Reason for the current stability level.
141+
reason: Option<Symbol>,
142+
/// Relevant `rust-lang/rust` issue.
143+
issue: Option<NonZeroU32>,
144+
is_soft: bool,
145+
/// If part of a feature is stabilized and a new feature is added for the remaining parts,
146+
/// then the `implied_by` attribute is used to indicate which now-stable feature previously
147+
/// contained a item.
148+
///
149+
/// ```pseudo-Rust
150+
/// #[unstable(feature = "foo", issue = "...")]
151+
/// fn foo() {}
152+
/// #[unstable(feature = "foo", issue = "...")]
153+
/// fn foobar() {}
154+
/// ```
155+
///
156+
/// ...becomes...
157+
///
158+
/// ```pseudo-Rust
159+
/// #[stable(feature = "foo", since = "1.XX.X")]
160+
/// fn foo() {}
161+
/// #[unstable(feature = "foobar", issue = "...", implied_by = "foo")]
162+
/// fn foobar() {}
163+
/// ```
164+
implied_by: Option<Symbol>,
165+
},
166+
/// `#[stable]`
167+
Stable {
168+
/// Rust release which stabilized this feature.
169+
since: Symbol,
170+
/// Is this item allowed to be referred to on stable, despite being contained in unstable
171+
/// modules?
172+
allowed_through_unstable_modules: bool,
173+
},
141174
}
142175

143176
impl StabilityLevel {
@@ -243,6 +276,7 @@ where
243276
let mut issue = None;
244277
let mut issue_num = None;
245278
let mut is_soft = false;
279+
let mut implied_by = None;
246280
for meta in metas {
247281
let Some(mi) = meta.meta_item() else {
248282
handle_errors(
@@ -308,6 +342,11 @@ where
308342
}
309343
is_soft = true;
310344
}
345+
sym::implied_by => {
346+
if !get(mi, &mut implied_by) {
347+
continue 'outer;
348+
}
349+
}
311350
_ => {
312351
handle_errors(
313352
&sess.parse_sess,
@@ -332,7 +371,7 @@ where
332371
);
333372
continue;
334373
}
335-
let level = Unstable { reason, issue: issue_num, is_soft };
374+
let level = Unstable { reason, issue: issue_num, is_soft, implied_by };
336375
if sym::unstable == meta_name {
337376
stab = Some((Stability { level, feature }, attr.span));
338377
} else {
@@ -391,7 +430,7 @@ where
391430
meta.span(),
392431
AttrError::UnknownMetaItem(
393432
pprust::path_to_string(&mi.path),
394-
&["since", "note"],
433+
&["feature", "since"],
395434
),
396435
);
397436
continue 'outer;

compiler/rustc_metadata/src/rmeta/decoder.rs

+7
Original file line numberDiff line numberDiff line change
@@ -951,6 +951,13 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
951951
tcx.arena.alloc_from_iter(self.root.lib_features.decode(self))
952952
}
953953

954+
/// Iterates over the stability implications in the given crate (when a `#[unstable]` attribute
955+
/// has an `implied_by` meta item, then the mapping from the implied feature to the actual
956+
/// feature is a stability implication).
957+
fn get_stability_implications(self, tcx: TyCtxt<'tcx>) -> &'tcx [(Symbol, Symbol)] {
958+
tcx.arena.alloc_from_iter(self.root.stability_implications.decode(self))
959+
}
960+
954961
/// Iterates over the language items in the given crate.
955962
fn get_lang_items(self, tcx: TyCtxt<'tcx>) -> &'tcx [(DefId, usize)] {
956963
tcx.arena.alloc_from_iter(

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

+3
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,9 @@ provide! { <'tcx> tcx, def_id, other, cdata,
291291
tcx.arena.alloc_slice(&result)
292292
}
293293
defined_lib_features => { cdata.get_lib_features(tcx) }
294+
stability_implications => {
295+
cdata.get_stability_implications(tcx).iter().copied().collect()
296+
}
294297
is_intrinsic => { cdata.get_is_intrinsic(def_id.index) }
295298
defined_lang_items => { cdata.get_lang_items(tcx) }
296299
diagnostic_items => { cdata.get_diagnostic_items() }

compiler/rustc_metadata/src/rmeta/encoder.rs

+15
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,11 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
538538
let lib_features = self.encode_lib_features();
539539
let lib_feature_bytes = self.position() - i;
540540

541+
// Encode the stability implications.
542+
i = self.position();
543+
let stability_implications = self.encode_stability_implications();
544+
let stability_implications_bytes = self.position() - i;
545+
541546
// Encode the language items.
542547
i = self.position();
543548
let lang_items = self.encode_lang_items();
@@ -686,6 +691,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
686691
crate_deps,
687692
dylib_dependency_formats,
688693
lib_features,
694+
stability_implications,
689695
lang_items,
690696
diagnostic_items,
691697
lang_items_missing,
@@ -710,6 +716,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
710716
let computed_total_bytes = preamble_bytes
711717
+ dep_bytes
712718
+ lib_feature_bytes
719+
+ stability_implications_bytes
713720
+ lang_item_bytes
714721
+ diagnostic_item_bytes
715722
+ native_lib_bytes
@@ -761,6 +768,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
761768
p("preamble", preamble_bytes);
762769
p("dep", dep_bytes);
763770
p("lib feature", lib_feature_bytes);
771+
p("stability_implications", stability_implications_bytes);
764772
p("lang item", lang_item_bytes);
765773
p("diagnostic item", diagnostic_item_bytes);
766774
p("native lib", native_lib_bytes);
@@ -1777,6 +1785,13 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
17771785
self.lazy_array(lib_features.to_vec())
17781786
}
17791787

1788+
fn encode_stability_implications(&mut self) -> LazyArray<(Symbol, Symbol)> {
1789+
empty_proc_macro!(self);
1790+
let tcx = self.tcx;
1791+
let implications = tcx.stability_implications(LOCAL_CRATE);
1792+
self.lazy_array(implications.iter().map(|(k, v)| (*k, *v)))
1793+
}
1794+
17801795
fn encode_diagnostic_items(&mut self) -> LazyArray<(Symbol, DefIndex)> {
17811796
empty_proc_macro!(self);
17821797
let tcx = self.tcx;

compiler/rustc_metadata/src/rmeta/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ pub(crate) struct CrateRoot {
226226
crate_deps: LazyArray<CrateDep>,
227227
dylib_dependency_formats: LazyArray<Option<LinkagePreference>>,
228228
lib_features: LazyArray<(Symbol, Option<Symbol>)>,
229+
stability_implications: LazyArray<(Symbol, Symbol)>,
229230
lang_items: LazyArray<(DefIndex, usize)>,
230231
lang_items_missing: LazyArray<lang_items::LangItem>,
231232
diagnostic_items: LazyArray<(Symbol, DefIndex)>,

compiler/rustc_middle/src/middle/mod.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,23 @@ pub mod dependency_format;
33
pub mod exported_symbols;
44
pub mod lang_items;
55
pub mod lib_features {
6-
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
7-
use rustc_span::symbol::Symbol;
6+
use rustc_data_structures::fx::FxHashMap;
7+
use rustc_span::{symbol::Symbol, Span};
88

99
#[derive(HashStable, Debug)]
1010
pub struct LibFeatures {
11-
// A map from feature to stabilisation version.
12-
pub stable: FxHashMap<Symbol, Symbol>,
13-
pub unstable: FxHashSet<Symbol>,
11+
/// A map from feature to stabilisation version.
12+
pub stable: FxHashMap<Symbol, (Symbol, Span)>,
13+
pub unstable: FxHashMap<Symbol, Span>,
1414
}
1515

1616
impl LibFeatures {
1717
pub fn to_vec(&self) -> Vec<(Symbol, Option<Symbol>)> {
1818
let mut all_features: Vec<_> = self
1919
.stable
2020
.iter()
21-
.map(|(f, s)| (*f, Some(*s)))
22-
.chain(self.unstable.iter().map(|f| (*f, None)))
21+
.map(|(f, (s, _))| (*f, Some(*s)))
22+
.chain(self.unstable.iter().map(|(f, _)| (*f, None)))
2323
.collect();
2424
all_features.sort_unstable_by(|a, b| a.0.as_str().partial_cmp(b.0.as_str()).unwrap());
2525
all_features

compiler/rustc_middle/src/middle/stability.rs

+23-1
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,19 @@ pub struct Index {
6262
pub stab_map: FxHashMap<LocalDefId, Stability>,
6363
pub const_stab_map: FxHashMap<LocalDefId, ConstStability>,
6464
pub depr_map: FxHashMap<LocalDefId, DeprecationEntry>,
65+
/// Mapping from feature name to feature name based on the `implied_by` field of `#[unstable]`
66+
/// attributes. If a `#[unstable(feature = "implier", implied_by = "impliee")]` attribute
67+
/// exists, then this map will have a `impliee -> implier` entry.
68+
///
69+
/// This mapping is necessary unless both the `#[stable]` and `#[unstable]` attributes should
70+
/// specify their implications (both `implies` and `implied_by`). If only one of the two
71+
/// attributes do (as in the current implementation, `implied_by` in `#[unstable]`), then this
72+
/// mapping is necessary for diagnostics. When a "unnecessary feature attribute" error is
73+
/// reported, only the `#[stable]` attribute information is available, so the map is necessary
74+
/// to know that the feature implies another feature. If it were reversed, and the `#[stable]`
75+
/// attribute had an `implies` meta item, then a map would be necessary when avoiding a "use of
76+
/// unstable feature" error for a feature that was implied.
77+
pub implications: FxHashMap<Symbol, Symbol>,
6578
}
6679

6780
impl Index {
@@ -423,7 +436,9 @@ impl<'tcx> TyCtxt<'tcx> {
423436

424437
match stability {
425438
Some(Stability {
426-
level: attr::Unstable { reason, issue, is_soft }, feature, ..
439+
level: attr::Unstable { reason, issue, is_soft, implied_by },
440+
feature,
441+
..
427442
}) => {
428443
if span.allows_unstable(feature) {
429444
debug!("stability: skipping span={:?} since it is internal", span);
@@ -433,6 +448,13 @@ impl<'tcx> TyCtxt<'tcx> {
433448
return EvalResult::Allow;
434449
}
435450

451+
// If this item was previously part of a now-stabilized feature which is still
452+
// active (i.e. the user hasn't removed the attribute for the stabilized feature
453+
// yet) then allow use of this item.
454+
if let Some(implied_by) = implied_by && self.features().active(implied_by) {
455+
return EvalResult::Allow;
456+
}
457+
436458
// When we're compiling the compiler itself we may pull in
437459
// crates from crates.io, but those crates may depend on other
438460
// crates also pulled in from crates.io. We want to ideally be

compiler/rustc_middle/src/query/mod.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -1634,11 +1634,15 @@ rustc_queries! {
16341634
storage(ArenaCacheSelector<'tcx>)
16351635
desc { "calculating the lib features map" }
16361636
}
1637-
query defined_lib_features(_: CrateNum)
1638-
-> &'tcx [(Symbol, Option<Symbol>)] {
1637+
query defined_lib_features(_: CrateNum) -> &'tcx [(Symbol, Option<Symbol>)] {
16391638
desc { "calculating the lib features defined in a crate" }
16401639
separate_provide_extern
16411640
}
1641+
query stability_implications(_: CrateNum) -> FxHashMap<Symbol, Symbol> {
1642+
storage(ArenaCacheSelector<'tcx>)
1643+
desc { "calculating the implications between `#[unstable]` features defined in a crate" }
1644+
separate_provide_extern
1645+
}
16421646
/// Whether the function is an intrinsic
16431647
query is_intrinsic(def_id: DefId) -> bool {
16441648
desc { |tcx| "is_intrinsic({})", tcx.def_path_str(def_id) }

compiler/rustc_passes/src/lib_features.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
// Detecting lib features (i.e., features that are not lang features).
2-
//
3-
// These are declared using stability attributes (e.g., `#[stable (..)]`
4-
// and `#[unstable (..)]`), but are not declared in one single location
5-
// (unlike lang features), which means we need to collect them instead.
1+
//! Detecting lib features (i.e., features that are not lang features).
2+
//!
3+
//! These are declared using stability attributes (e.g., `#[stable (..)]` and `#[unstable (..)]`),
4+
//! but are not declared in one single location (unlike lang features), which means we need to
5+
//! collect them instead.
66
77
use rustc_ast::{Attribute, MetaItemKind};
88
use rustc_errors::struct_span_err;
@@ -71,11 +71,11 @@ impl<'tcx> LibFeatureCollector<'tcx> {
7171

7272
fn collect_feature(&mut self, feature: Symbol, since: Option<Symbol>, span: Span) {
7373
let already_in_stable = self.lib_features.stable.contains_key(&feature);
74-
let already_in_unstable = self.lib_features.unstable.contains(&feature);
74+
let already_in_unstable = self.lib_features.unstable.contains_key(&feature);
7575

7676
match (since, already_in_stable, already_in_unstable) {
7777
(Some(since), _, false) => {
78-
if let Some(prev_since) = self.lib_features.stable.get(&feature) {
78+
if let Some((prev_since, _)) = self.lib_features.stable.get(&feature) {
7979
if *prev_since != since {
8080
self.span_feature_error(
8181
span,
@@ -89,10 +89,10 @@ impl<'tcx> LibFeatureCollector<'tcx> {
8989
}
9090
}
9191

92-
self.lib_features.stable.insert(feature, since);
92+
self.lib_features.stable.insert(feature, (since, span));
9393
}
9494
(None, false, _) => {
95-
self.lib_features.unstable.insert(feature);
95+
self.lib_features.unstable.insert(feature, span);
9696
}
9797
(Some(_), _, true) | (None, true, _) => {
9898
self.span_feature_error(

0 commit comments

Comments
 (0)