From 6f0b8f1a4b8f126f4e762655f927e22bf971513e Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 13 Jul 2022 10:09:37 +0100 Subject: [PATCH 1/6] attr/passes: comment -> doc comment Change some regular comments into documentation comments. Signed-off-by: David Wood --- compiler/rustc_attr/src/builtin.rs | 19 ++++++++++++++++--- compiler/rustc_passes/src/lib_features.rs | 10 +++++----- compiler/rustc_passes/src/stability.rs | 16 ++++++++-------- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index dcfbecedfe8..f344bdba5db 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -135,9 +135,22 @@ impl ConstStability { #[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)] #[derive(HashStable_Generic)] pub enum StabilityLevel { - // Reason for the current stability level and the relevant rust-lang issue - Unstable { reason: Option, issue: Option, is_soft: bool }, - Stable { since: Symbol, allowed_through_unstable_modules: bool }, + /// `#[unstable]` + Unstable { + /// Reason for the current stability level. + reason: Option, + /// Relevant `rust-lang/rust` issue. + issue: Option, + is_soft: bool, + }, + /// `#[stable]` + Stable { + /// Rust release which stabilized this feature. + since: Symbol, + /// Is this item allowed to be referred to on stable, despite being contained in unstable + /// modules? + allowed_through_unstable_modules: bool, + }, } impl StabilityLevel { diff --git a/compiler/rustc_passes/src/lib_features.rs b/compiler/rustc_passes/src/lib_features.rs index 26bfa4737a7..97ae8e6f325 100644 --- a/compiler/rustc_passes/src/lib_features.rs +++ b/compiler/rustc_passes/src/lib_features.rs @@ -1,8 +1,8 @@ -// Detecting lib features (i.e., features that are not lang features). -// -// These are declared using stability attributes (e.g., `#[stable (..)]` -// and `#[unstable (..)]`), but are not declared in one single location -// (unlike lang features), which means we need to collect them instead. +//! Detecting lib features (i.e., features that are not lang features). +//! +//! These are declared using stability attributes (e.g., `#[stable (..)]` and `#[unstable (..)]`), +//! but are not declared in one single location (unlike lang features), which means we need to +//! collect them instead. use rustc_ast::{Attribute, MetaItemKind}; use rustc_errors::struct_span_err; diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index 4e091c5b70d..da87f557a55 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -29,13 +29,13 @@ use std::num::NonZeroU32; #[derive(PartialEq)] enum AnnotationKind { - // Annotation is required if not inherited from unstable parents + /// Annotation is required if not inherited from unstable parents. Required, - // Annotation is useless, reject it + /// Annotation is useless, reject it. Prohibited, - // Deprecation annotation is useless, reject it. (Stability attribute is still required.) + /// Deprecation annotation is useless, reject it. (Stability attribute is still required.) DeprecationProhibited, - // Annotation itself is useless, but it can be propagated to children + /// Annotation itself is useless, but it can be propagated to children. Container, } @@ -83,7 +83,7 @@ impl InheritStability { } } -// A private tree-walker for producing an Index. +/// A private tree-walker for producing an `Index`. struct Annotator<'a, 'tcx> { tcx: TyCtxt<'tcx>, index: &'a mut Index, @@ -94,9 +94,9 @@ struct Annotator<'a, 'tcx> { } impl<'a, 'tcx> Annotator<'a, 'tcx> { - // Determine the stability for a node based on its attributes and inherited - // stability. The stability is recorded in the index and used as the parent. - // If the node is a function, `fn_sig` is its signature + /// Determine the stability for a node based on its attributes and inherited stability. The + /// stability is recorded in the index and used as the parent. If the node is a function, + /// `fn_sig` is its signature. fn annotate( &mut self, def_id: LocalDefId, From a1d5af24ece7d35d6f23a2782df5ffdaa132dd82 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 13 Jul 2022 10:36:12 +0100 Subject: [PATCH 2/6] attr: fix expected meta-item for `#[stable]` When an unexpected meta item is provided to `#[stable]`, the diagnostic lists "since" and "note" as expected meta-items, however the surrounding code actually expects "feature" and "since". Signed-off-by: David Wood --- compiler/rustc_attr/src/builtin.rs | 2 +- .../ui/stability-attribute/stability-attribute-sanity-2.stderr | 2 +- .../ui/stability-attribute/stability-attribute-sanity.stderr | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index f344bdba5db..16e56c6d851 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -404,7 +404,7 @@ where meta.span(), AttrError::UnknownMetaItem( pprust::path_to_string(&mi.path), - &["since", "note"], + &["feature", "since"], ), ); continue 'outer; diff --git a/src/test/ui/stability-attribute/stability-attribute-sanity-2.stderr b/src/test/ui/stability-attribute/stability-attribute-sanity-2.stderr index bd7b88da158..8dbcc6c97ef 100644 --- a/src/test/ui/stability-attribute/stability-attribute-sanity-2.stderr +++ b/src/test/ui/stability-attribute/stability-attribute-sanity-2.stderr @@ -8,7 +8,7 @@ error[E0541]: unknown meta item 'sinse' --> $DIR/stability-attribute-sanity-2.rs:10:25 | LL | #[stable(feature = "a", sinse = "1.0.0")] - | ^^^^^^^^^^^^^^^ expected one of `since`, `note` + | ^^^^^^^^^^^^^^^ expected one of `feature`, `since` error[E0545]: `issue` must be a non-zero numeric string or "none" --> $DIR/stability-attribute-sanity-2.rs:13:27 diff --git a/src/test/ui/stability-attribute/stability-attribute-sanity.stderr b/src/test/ui/stability-attribute/stability-attribute-sanity.stderr index fcb1eefddbc..079230b2a31 100644 --- a/src/test/ui/stability-attribute/stability-attribute-sanity.stderr +++ b/src/test/ui/stability-attribute/stability-attribute-sanity.stderr @@ -14,7 +14,7 @@ error[E0541]: unknown meta item 'reason' --> $DIR/stability-attribute-sanity.rs:8:42 | LL | #[stable(feature = "a", since = "b", reason)] - | ^^^^^^ expected one of `since`, `note` + | ^^^^^^ expected one of `feature`, `since` error[E0539]: incorrect meta item --> $DIR/stability-attribute-sanity.rs:11:29 From 224aec213d62aa11758c864c871c341b7e7e7681 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 13 Jul 2022 13:10:37 +0100 Subject: [PATCH 3/6] middle: add `implies_by` to `#[unstable]` If part of a feature is stabilized and a new feature is added for the remaining parts, then the `implied_by` attribute can be used to indicate which now-stable feature previously contained a item. If the now-stable feature is still active (if the user has only just updated rustc, for example) then there will not be an stability error for uses of the item from the implied feature. Signed-off-by: David Wood --- compiler/rustc_attr/src/builtin.rs | 28 ++++++++++++++++++- compiler/rustc_middle/src/middle/stability.rs | 11 +++++++- compiler/rustc_passes/src/stability.rs | 1 + compiler/rustc_resolve/src/macros.rs | 11 ++++++-- compiler/rustc_span/src/symbol.rs | 1 + .../auxiliary/stability-attribute-implies.rs | 8 ++++++ .../stability-attribute-implies-no-feature.rs | 13 +++++++++ ...bility-attribute-implies-no-feature.stderr | 21 ++++++++++++++ ...tability-attribute-implies-using-stable.rs | 15 ++++++++++ ...lity-attribute-implies-using-stable.stderr | 14 ++++++++++ ...bility-attribute-implies-using-unstable.rs | 17 +++++++++++ ...ty-attribute-implies-using-unstable.stderr | 14 ++++++++++ 12 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/stability-attribute/auxiliary/stability-attribute-implies.rs create mode 100644 src/test/ui/stability-attribute/stability-attribute-implies-no-feature.rs create mode 100644 src/test/ui/stability-attribute/stability-attribute-implies-no-feature.stderr create mode 100644 src/test/ui/stability-attribute/stability-attribute-implies-using-stable.rs create mode 100644 src/test/ui/stability-attribute/stability-attribute-implies-using-stable.stderr create mode 100644 src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.rs create mode 100644 src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.stderr diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index 16e56c6d851..64a6f91f022 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -142,6 +142,26 @@ pub enum StabilityLevel { /// Relevant `rust-lang/rust` issue. issue: Option, is_soft: bool, + /// 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() {} + /// ``` + implied_by: Option, }, /// `#[stable]` Stable { @@ -256,6 +276,7 @@ where let mut issue = None; let mut issue_num = None; let mut is_soft = false; + let mut implied_by = None; for meta in metas { let Some(mi) = meta.meta_item() else { handle_errors( @@ -321,6 +342,11 @@ where } is_soft = true; } + sym::implied_by => { + if !get(mi, &mut implied_by) { + continue 'outer; + } + } _ => { handle_errors( &sess.parse_sess, @@ -345,7 +371,7 @@ where ); continue; } - let level = Unstable { reason, issue: issue_num, is_soft }; + let level = Unstable { reason, issue: issue_num, is_soft, implied_by }; if sym::unstable == meta_name { stab = Some((Stability { level, feature }, attr.span)); } else { diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs index 96e068a3601..63f66be1507 100644 --- a/compiler/rustc_middle/src/middle/stability.rs +++ b/compiler/rustc_middle/src/middle/stability.rs @@ -423,7 +423,9 @@ impl<'tcx> TyCtxt<'tcx> { match stability { Some(Stability { - level: attr::Unstable { reason, issue, is_soft }, feature, .. + level: attr::Unstable { reason, issue, is_soft, implied_by }, + feature, + .. }) => { if span.allows_unstable(feature) { debug!("stability: skipping span={:?} since it is internal", span); @@ -433,6 +435,13 @@ impl<'tcx> TyCtxt<'tcx> { return EvalResult::Allow; } + // If this item was previously part of a now-stabilized feature which is still + // active (i.e. the user hasn't removed the attribute for the stabilized feature + // yet) then allow use of this item. + if let Some(implied_by) = implied_by && self.features().active(implied_by) { + return EvalResult::Allow; + } + // When we're compiling the compiler itself we may pull in // crates from crates.io, but those crates may depend on other // crates also pulled in from crates.io. We want to ideally be diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index da87f557a55..a15a42bd76f 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -637,6 +637,7 @@ fn stability_index(tcx: TyCtxt<'_>, (): ()) -> Index { reason: Some(Symbol::intern(reason)), issue: NonZeroU32::new(27812), is_soft: false, + implied_by: None, }, feature: sym::rustc_private, }; diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 54dd15270a1..2b5eb12a8a8 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -796,9 +796,16 @@ impl<'a> Resolver<'a> { ) { let span = path.span; if let Some(stability) = &ext.stability { - if let StabilityLevel::Unstable { reason, issue, is_soft } = stability.level { + if let StabilityLevel::Unstable { reason, issue, is_soft, implied_by } = stability.level + { let feature = stability.feature; - if !self.active_features.contains(&feature) && !span.allows_unstable(feature) { + + let is_allowed = |feature| { + self.active_features.contains(&feature) || span.allows_unstable(feature) + }; + let allowed_by_implication = + implied_by.map(|feature| is_allowed(feature)).unwrap_or(false); + if !is_allowed(feature) && !allowed_by_implication { let lint_buffer = &mut self.lint_buffer; let soft_handler = |lint, span, msg: &_| lint_buffer.buffer_lint(lint, node_id, span, msg); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index ec1d2c39b80..2ac1ecfe87e 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -800,6 +800,7 @@ symbols! { impl_lint_pass, impl_macros, impl_trait_in_bindings, + implied_by, import, import_shadowing, imported_main, diff --git a/src/test/ui/stability-attribute/auxiliary/stability-attribute-implies.rs b/src/test/ui/stability-attribute/auxiliary/stability-attribute-implies.rs new file mode 100644 index 00000000000..468be1bc144 --- /dev/null +++ b/src/test/ui/stability-attribute/auxiliary/stability-attribute-implies.rs @@ -0,0 +1,8 @@ +#![feature(staged_api)] +#![stable(feature = "stability_attribute_implies", since = "1.0.0")] + +#[stable(feature = "foo", since = "1.62.0")] +pub fn foo() {} + +#[unstable(feature = "foobar", issue = "1", implied_by = "foo")] +pub fn foobar() {} diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-no-feature.rs b/src/test/ui/stability-attribute/stability-attribute-implies-no-feature.rs new file mode 100644 index 00000000000..947f9f73eff --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-implies-no-feature.rs @@ -0,0 +1,13 @@ +// aux-build:stability-attribute-implies.rs + +// Tests that despite the `foobar` feature being implied by now-stable feature `foo`, if `foobar` +// isn't allowed in this crate then an error will be emitted. + +extern crate stability_attribute_implies; +use stability_attribute_implies::{foo, foobar}; +//~^ ERROR use of unstable library feature 'foobar' + +fn main() { + foo(); // no error - stable + foobar(); //~ ERROR use of unstable library feature 'foobar' +} diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-no-feature.stderr b/src/test/ui/stability-attribute/stability-attribute-implies-no-feature.stderr new file mode 100644 index 00000000000..c2331f6766c --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-implies-no-feature.stderr @@ -0,0 +1,21 @@ +error[E0658]: use of unstable library feature 'foobar' + --> $DIR/stability-attribute-implies-no-feature.rs:7:40 + | +LL | use stability_attribute_implies::{foo, foobar}; + | ^^^^^^ + | + = note: see issue #1 for more information + = help: add `#![feature(foobar)]` to the crate attributes to enable + +error[E0658]: use of unstable library feature 'foobar' + --> $DIR/stability-attribute-implies-no-feature.rs:12:5 + | +LL | foobar(); + | ^^^^^^ + | + = note: see issue #1 for more information + = help: add `#![feature(foobar)]` to the crate attributes to enable + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.rs b/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.rs new file mode 100644 index 00000000000..527639ec70b --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.rs @@ -0,0 +1,15 @@ +// aux-build:stability-attribute-implies.rs +#![deny(stable_features)] +#![feature(foo)] +//~^ ERROR the feature `foo` has been stable since 1.62.0 and no longer requires an attribute to enable + +// Tests that the use of `implied_by` in the `#[unstable]` attribute results in a diagnostic +// mentioning partial stabilization, and that given the implied unstable feature is unused (there +// is no `foobar` call), that the compiler suggests removing the flag. + +extern crate stability_attribute_implies; +use stability_attribute_implies::foo; + +fn main() { + foo(); +} diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.stderr b/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.stderr new file mode 100644 index 00000000000..c8767e85a68 --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.stderr @@ -0,0 +1,14 @@ +error: the feature `foo` has been stable since 1.62.0 and no longer requires an attribute to enable + --> $DIR/stability-attribute-implies-using-stable.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)] + | ^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.rs b/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.rs new file mode 100644 index 00000000000..d6ad4d3510e --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.rs @@ -0,0 +1,17 @@ +// aux-build:stability-attribute-implies.rs +#![deny(stable_features)] +#![feature(foo)] +//~^ ERROR the feature `foo` has been stable since 1.62.0 and no longer requires an attribute to enable + +// Tests that the use of `implied_by` in the `#[unstable]` attribute results in a diagnostic +// mentioning partial stabilization and that given the implied unstable feature is used (there is a +// `foobar` call), that the compiler suggests changing to that feature and doesn't error about its +// use. + +extern crate stability_attribute_implies; +use stability_attribute_implies::{foo, foobar}; + +fn main() { + foo(); + foobar(); // no error! +} diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.stderr b/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.stderr new file mode 100644 index 00000000000..35cbac6035c --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.stderr @@ -0,0 +1,14 @@ +error: the feature `foo` has been stable since 1.62.0 and no longer requires an attribute to enable + --> $DIR/stability-attribute-implies-using-unstable.rs:3:12 + | +LL | #![feature(foo)] + | ^^^ + | +note: the lint level is defined here + --> $DIR/stability-attribute-implies-using-unstable.rs:2:9 + | +LL | #![deny(stable_features)] + | ^^^^^^^^^^^^^^^ + +error: aborting due to previous error + From 97edb9f336e98c0597210092e2e0ef0ee1576e24 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 13 Jul 2022 15:02:23 +0100 Subject: [PATCH 4/6] span: add `span_extend_to_line` helper Adds a simple helper function to the `SourceMap` for extending a `Span` to encompass the entire line it is on - useful for suggestions where removing a line is the suggested action. Signed-off-by: David Wood --- compiler/rustc_span/src/source_map.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index afbb88e9233..b4a4424e876 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -718,6 +718,11 @@ impl SourceMap { sp } + /// Extends the given `Span` to contain the entire line it is on. + pub fn span_extend_to_line(&self, sp: Span) -> Span { + self.span_extend_to_prev_char(self.span_extend_to_next_char(sp, '\n', true), '\n', true) + } + /// Given a `Span`, tries to get a shorter span ending before the first occurrence of `char` /// `c`. pub fn span_until_char(&self, sp: Span, c: char) -> Span { From 6246d66c6da3064f658831c0ed8162df169a001e Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 13 Jul 2022 15:10:19 +0100 Subject: [PATCH 5/6] passes: improved partial stabilization diagnostic Improves the diagnostic when a feature attribute is specified unnecessarily but the feature implies another (i.e. it was partially stabilized) to refer to the implied feature. Signed-off-by: David Wood --- compiler/rustc_metadata/src/rmeta/decoder.rs | 7 +++ .../src/rmeta/decoder/cstore_impl.rs | 3 + compiler/rustc_metadata/src/rmeta/encoder.rs | 15 +++++ compiler/rustc_metadata/src/rmeta/mod.rs | 1 + compiler/rustc_middle/src/middle/stability.rs | 13 +++++ compiler/rustc_middle/src/query/mod.rs | 8 ++- compiler/rustc_passes/src/stability.rs | 58 ++++++++++++++++--- ...tability-attribute-implies-using-stable.rs | 2 +- ...lity-attribute-implies-using-stable.stderr | 10 +++- ...bility-attribute-implies-using-unstable.rs | 2 +- ...ty-attribute-implies-using-unstable.stderr | 10 +++- 11 files changed, 115 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index f0874f8f2da..aa5705d3fcd 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -951,6 +951,13 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { tcx.arena.alloc_from_iter(self.root.lib_features.decode(self)) } + /// Iterates over the stability implications in the given crate (when a `#[unstable]` attribute + /// has an `implied_by` meta item, then the mapping from the implied feature to the actual + /// feature is a stability implication). + fn get_stability_implications(self, tcx: TyCtxt<'tcx>) -> &'tcx [(Symbol, Symbol)] { + tcx.arena.alloc_from_iter(self.root.stability_implications.decode(self)) + } + /// Iterates over the language items in the given crate. fn get_lang_items(self, tcx: TyCtxt<'tcx>) -> &'tcx [(DefId, usize)] { tcx.arena.alloc_from_iter( diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 565eec18ea9..65cae29c58d 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -291,6 +291,9 @@ provide! { <'tcx> tcx, def_id, other, cdata, tcx.arena.alloc_slice(&result) } defined_lib_features => { cdata.get_lib_features(tcx) } + stability_implications => { + cdata.get_stability_implications(tcx).iter().copied().collect() + } is_intrinsic => { cdata.get_is_intrinsic(def_id.index) } defined_lang_items => { cdata.get_lang_items(tcx) } diagnostic_items => { cdata.get_diagnostic_items() } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 8e973009777..50d983754e8 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -538,6 +538,11 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let lib_features = self.encode_lib_features(); let lib_feature_bytes = self.position() - i; + // Encode the stability implications. + i = self.position(); + let stability_implications = self.encode_stability_implications(); + let stability_implications_bytes = self.position() - i; + // Encode the language items. i = self.position(); let lang_items = self.encode_lang_items(); @@ -686,6 +691,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { crate_deps, dylib_dependency_formats, lib_features, + stability_implications, lang_items, diagnostic_items, lang_items_missing, @@ -710,6 +716,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let computed_total_bytes = preamble_bytes + dep_bytes + lib_feature_bytes + + stability_implications_bytes + lang_item_bytes + diagnostic_item_bytes + native_lib_bytes @@ -761,6 +768,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { p("preamble", preamble_bytes); p("dep", dep_bytes); p("lib feature", lib_feature_bytes); + p("stability_implications", stability_implications_bytes); p("lang item", lang_item_bytes); p("diagnostic item", diagnostic_item_bytes); p("native lib", native_lib_bytes); @@ -1777,6 +1785,13 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { self.lazy_array(lib_features.to_vec()) } + fn encode_stability_implications(&mut self) -> LazyArray<(Symbol, Symbol)> { + empty_proc_macro!(self); + let tcx = self.tcx; + let implications = tcx.stability_implications(LOCAL_CRATE); + self.lazy_array(implications.iter().map(|(k, v)| (*k, *v))) + } + fn encode_diagnostic_items(&mut self) -> LazyArray<(Symbol, DefIndex)> { empty_proc_macro!(self); let tcx = self.tcx; diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index af1c09f4ae8..0f291f92647 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -226,6 +226,7 @@ pub(crate) struct CrateRoot { crate_deps: LazyArray, dylib_dependency_formats: LazyArray>, lib_features: LazyArray<(Symbol, Option)>, + stability_implications: LazyArray<(Symbol, Symbol)>, lang_items: LazyArray<(DefIndex, usize)>, lang_items_missing: LazyArray, diagnostic_items: LazyArray<(Symbol, DefIndex)>, diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs index 63f66be1507..0fbad3f0f0f 100644 --- a/compiler/rustc_middle/src/middle/stability.rs +++ b/compiler/rustc_middle/src/middle/stability.rs @@ -62,6 +62,19 @@ pub struct Index { pub stab_map: FxHashMap, pub const_stab_map: FxHashMap, pub depr_map: FxHashMap, + /// Mapping from feature name to feature name based on the `implied_by` field of `#[unstable]` + /// attributes. If a `#[unstable(feature = "implier", implied_by = "impliee")]` attribute + /// exists, then this map will have a `impliee -> implier` entry. + /// + /// 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. + pub implications: FxHashMap, } impl Index { diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 0581ef41f66..466a0fc25f7 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1634,11 +1634,15 @@ rustc_queries! { storage(ArenaCacheSelector<'tcx>) desc { "calculating the lib features map" } } - query defined_lib_features(_: CrateNum) - -> &'tcx [(Symbol, Option)] { + query defined_lib_features(_: CrateNum) -> &'tcx [(Symbol, Option)] { desc { "calculating the lib features defined in a crate" } separate_provide_extern } + query stability_implications(_: CrateNum) -> FxHashMap { + storage(ArenaCacheSelector<'tcx>) + desc { "calculating the implications between `#[unstable]` features defined in a crate" } + separate_provide_extern + } /// Whether the function is an intrinsic query is_intrinsic(def_id: DefId) -> bool { desc { |tcx| "is_intrinsic({})", tcx.def_path_str(def_id) } diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index a15a42bd76f..9cacda041f2 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -2,9 +2,9 @@ //! propagating default levels lexically from parent to children ast nodes. use attr::StabilityLevel; -use rustc_attr::{self as attr, ConstStability, Stability}; +use rustc_attr::{self as attr, ConstStability, Stability, Unstable}; use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; -use rustc_errors::struct_span_err; +use rustc_errors::{struct_span_err, Applicability}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID}; @@ -265,6 +265,10 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> { } } + if let Stability { level: Unstable { implied_by: Some(implied_by), .. }, feature } = stab { + self.index.implications.insert(implied_by, feature); + } + self.index.stab_map.insert(def_id, stab); stab }); @@ -610,6 +614,7 @@ fn stability_index(tcx: TyCtxt<'_>, (): ()) -> Index { stab_map: Default::default(), const_stab_map: Default::default(), depr_map: Default::default(), + implications: Default::default(), }; { @@ -668,6 +673,7 @@ pub(crate) fn provide(providers: &mut Providers) { *providers = Providers { check_mod_unstable_api_usage, stability_index, + stability_implications: |tcx, _| tcx.stability().implications.clone(), lookup_stability: |tcx, id| tcx.stability().local_stability(id.expect_local()), lookup_const_stability: |tcx, id| tcx.stability().local_const_stability(id.expect_local()), lookup_deprecation_entry: |tcx, id| { @@ -946,11 +952,18 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) { remaining_lib_features.remove(&sym::libc); remaining_lib_features.remove(&sym::test); + let mut implications = tcx.stability_implications(rustc_hir::def_id::LOCAL_CRATE).clone(); + for &cnum in tcx.crates(()) { + implications.extend(tcx.stability_implications(cnum)); + } + let check_features = |remaining_lib_features: &mut FxIndexMap<_, _>, defined_features: &[_]| { for &(feature, since) in defined_features { - if let Some(since) = since { - if let Some(span) = remaining_lib_features.get(&feature) { - // Warn if the user has enabled an already-stable lib feature. + if let Some(since) = since && let Some(span) = remaining_lib_features.get(&feature) { + // Warn if the user has enabled an already-stable lib feature. + if let Some(implies) = implications.get(&feature) { + unnecessary_partially_stable_feature_lint(tcx, *span, feature, *implies, since); + } else { unnecessary_stable_feature_lint(tcx, *span, feature, since); } } @@ -983,12 +996,41 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) { // don't lint about unused features. We should re-enable this one day! } +fn unnecessary_partially_stable_feature_lint( + tcx: TyCtxt<'_>, + span: Span, + feature: Symbol, + implies: Symbol, + since: Symbol, +) { + tcx.struct_span_lint_hir(lint::builtin::STABLE_FEATURES, hir::CRATE_HIR_ID, span, |lint| { + lint.build(&format!( + "the feature `{feature}` has been partially stabilized since {since} and is succeeded \ + by the feature `{implies}`" + )) + .span_suggestion( + span, + &format!( + "if you are using features which are still unstable, change to using `{implies}`" + ), + implies, + Applicability::MaybeIncorrect, + ) + .span_suggestion( + tcx.sess.source_map().span_extend_to_line(span), + "if you are using features which are now stable, remove this line", + "", + Applicability::MaybeIncorrect, + ) + .emit(); + }); +} + fn unnecessary_stable_feature_lint(tcx: TyCtxt<'_>, span: Span, feature: Symbol, since: Symbol) { tcx.struct_span_lint_hir(lint::builtin::STABLE_FEATURES, hir::CRATE_HIR_ID, span, |lint| { lint.build(&format!( - "the feature `{}` has been stable since {} and no longer requires \ - an attribute to enable", - feature, since + "the feature `{feature}` has been stable since {since} and no longer requires an \ + attribute to enable", )) .emit(); }); diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.rs b/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.rs index 527639ec70b..1a2d8e271de 100644 --- a/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.rs +++ b/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.rs @@ -1,7 +1,7 @@ // aux-build:stability-attribute-implies.rs #![deny(stable_features)] #![feature(foo)] -//~^ ERROR the feature `foo` has been stable since 1.62.0 and no longer requires an attribute to enable +//~^ ERROR the feature `foo` has been partially stabilized since 1.62.0 and is succeeded by the feature `foobar` // Tests that the use of `implied_by` in the `#[unstable]` attribute results in a diagnostic // mentioning partial stabilization, and that given the implied unstable feature is unused (there diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.stderr b/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.stderr index c8767e85a68..c9b3f07cc70 100644 --- a/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.stderr +++ b/src/test/ui/stability-attribute/stability-attribute-implies-using-stable.stderr @@ -1,4 +1,4 @@ -error: the feature `foo` has been stable since 1.62.0 and no longer requires an attribute to enable +error: the feature `foo` has been partially stabilized since 1.62.0 and is succeeded by the feature `foobar` --> $DIR/stability-attribute-implies-using-stable.rs:3:12 | LL | #![feature(foo)] @@ -9,6 +9,14 @@ note: the lint level is defined here | 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)] + | error: aborting due to previous error diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.rs b/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.rs index d6ad4d3510e..3c73c5abf3b 100644 --- a/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.rs +++ b/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.rs @@ -1,7 +1,7 @@ // aux-build:stability-attribute-implies.rs #![deny(stable_features)] #![feature(foo)] -//~^ ERROR the feature `foo` has been stable since 1.62.0 and no longer requires an attribute to enable +//~^ ERROR the feature `foo` has been partially stabilized since 1.62.0 and is succeeded by the feature `foobar` // Tests that the use of `implied_by` in the `#[unstable]` attribute results in a diagnostic // mentioning partial stabilization and that given the implied unstable feature is used (there is a diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.stderr b/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.stderr index 35cbac6035c..9a5c7ef5a47 100644 --- a/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.stderr +++ b/src/test/ui/stability-attribute/stability-attribute-implies-using-unstable.stderr @@ -1,4 +1,4 @@ -error: the feature `foo` has been stable since 1.62.0 and no longer requires an attribute to enable +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)] @@ -9,6 +9,14 @@ note: the lint level is defined here | 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)] + | error: aborting due to previous error From e5872990d13abb088397e23b226439b1b4926b91 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 20 Jul 2022 14:52:23 +0100 Subject: [PATCH 6/6] passes: check implied feature exists Add a check confirming that features referenced in `implied_by` meta items actually exist. Signed-off-by: David Wood --- compiler/rustc_middle/src/middle/mod.rs | 14 +++--- compiler/rustc_passes/src/lib_features.rs | 8 +-- compiler/rustc_passes/src/stability.rs | 50 ++++++++++++------- .../stability-attribute-implies-missing.rs | 10 ++++ ...stability-attribute-implies-missing.stderr | 8 +++ 5 files changed, 60 insertions(+), 30 deletions(-) create mode 100644 src/test/ui/stability-attribute/stability-attribute-implies-missing.rs create mode 100644 src/test/ui/stability-attribute/stability-attribute-implies-missing.stderr diff --git a/compiler/rustc_middle/src/middle/mod.rs b/compiler/rustc_middle/src/middle/mod.rs index fc35cafcc77..8dc68b1f5a8 100644 --- a/compiler/rustc_middle/src/middle/mod.rs +++ b/compiler/rustc_middle/src/middle/mod.rs @@ -3,14 +3,14 @@ pub mod dependency_format; pub mod exported_symbols; pub mod lang_items; pub mod lib_features { - use rustc_data_structures::fx::{FxHashMap, FxHashSet}; - use rustc_span::symbol::Symbol; + use rustc_data_structures::fx::FxHashMap; + use rustc_span::{symbol::Symbol, Span}; #[derive(HashStable, Debug)] pub struct LibFeatures { - // A map from feature to stabilisation version. - pub stable: FxHashMap, - pub unstable: FxHashSet, + /// A map from feature to stabilisation version. + pub stable: FxHashMap, + pub unstable: FxHashMap, } impl LibFeatures { @@ -18,8 +18,8 @@ pub mod lib_features { let mut all_features: Vec<_> = self .stable .iter() - .map(|(f, s)| (*f, Some(*s))) - .chain(self.unstable.iter().map(|f| (*f, None))) + .map(|(f, (s, _))| (*f, Some(*s))) + .chain(self.unstable.iter().map(|(f, _)| (*f, None))) .collect(); all_features.sort_unstable_by(|a, b| a.0.as_str().partial_cmp(b.0.as_str()).unwrap()); all_features diff --git a/compiler/rustc_passes/src/lib_features.rs b/compiler/rustc_passes/src/lib_features.rs index 97ae8e6f325..e05994f13e4 100644 --- a/compiler/rustc_passes/src/lib_features.rs +++ b/compiler/rustc_passes/src/lib_features.rs @@ -71,11 +71,11 @@ impl<'tcx> LibFeatureCollector<'tcx> { fn collect_feature(&mut self, feature: Symbol, since: Option, span: Span) { let already_in_stable = self.lib_features.stable.contains_key(&feature); - let already_in_unstable = self.lib_features.unstable.contains(&feature); + let already_in_unstable = self.lib_features.unstable.contains_key(&feature); match (since, already_in_stable, already_in_unstable) { (Some(since), _, false) => { - if let Some(prev_since) = self.lib_features.stable.get(&feature) { + if let Some((prev_since, _)) = self.lib_features.stable.get(&feature) { if *prev_since != since { self.span_feature_error( span, @@ -89,10 +89,10 @@ impl<'tcx> LibFeatureCollector<'tcx> { } } - self.lib_features.stable.insert(feature, since); + self.lib_features.stable.insert(feature, (since, span)); } (None, false, _) => { - self.lib_features.unstable.insert(feature); + self.lib_features.unstable.insert(feature, span); } (Some(_), _, true) | (None, true, _) => { self.span_feature_error( diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index 9cacda041f2..81b04c414ed 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -3,7 +3,7 @@ use attr::StabilityLevel; use rustc_attr::{self as attr, ConstStability, Stability, Unstable}; -use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; use rustc_errors::{struct_span_err, Applicability}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; @@ -952,19 +952,45 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) { remaining_lib_features.remove(&sym::libc); remaining_lib_features.remove(&sym::test); + // We always collect the lib features declared in the current crate, even if there are + // no unknown features, because the collection also does feature attribute validation. + let local_defined_features = tcx.lib_features(()); + let mut all_lib_features: FxHashMap<_, _> = + local_defined_features.to_vec().iter().map(|el| *el).collect(); let mut implications = tcx.stability_implications(rustc_hir::def_id::LOCAL_CRATE).clone(); for &cnum in tcx.crates(()) { implications.extend(tcx.stability_implications(cnum)); + all_lib_features.extend(tcx.defined_lib_features(cnum).iter().map(|el| *el)); } - let check_features = |remaining_lib_features: &mut FxIndexMap<_, _>, defined_features: &[_]| { - for &(feature, since) in defined_features { + // Check that every feature referenced by an `implied_by` exists (for features defined in the + // local crate). + for (implied_by, feature) in tcx.stability_implications(rustc_hir::def_id::LOCAL_CRATE) { + // Only `implied_by` needs to be checked, `feature` is guaranteed to exist. + if !all_lib_features.contains_key(implied_by) { + let span = local_defined_features + .stable + .get(feature) + .map(|(_, span)| span) + .or_else(|| local_defined_features.unstable.get(feature)) + .expect("feature that implied another does not exist"); + tcx.sess + .struct_span_err( + *span, + format!("feature `{implied_by}` implying `{feature}` does not exist"), + ) + .emit(); + } + } + + if !remaining_lib_features.is_empty() { + for (feature, since) in all_lib_features.iter() { if let Some(since) = since && let Some(span) = remaining_lib_features.get(&feature) { // Warn if the user has enabled an already-stable lib feature. if let Some(implies) = implications.get(&feature) { - unnecessary_partially_stable_feature_lint(tcx, *span, feature, *implies, since); + unnecessary_partially_stable_feature_lint(tcx, *span, *feature, *implies, *since); } else { - unnecessary_stable_feature_lint(tcx, *span, feature, since); + unnecessary_stable_feature_lint(tcx, *span, *feature, *since); } } remaining_lib_features.remove(&feature); @@ -972,20 +998,6 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) { break; } } - }; - - // We always collect the lib features declared in the current crate, even if there are - // no unknown features, because the collection also does feature attribute validation. - let local_defined_features = tcx.lib_features(()).to_vec(); - if !remaining_lib_features.is_empty() { - check_features(&mut remaining_lib_features, &local_defined_features); - - for &cnum in tcx.crates(()) { - if remaining_lib_features.is_empty() { - break; - } - check_features(&mut remaining_lib_features, tcx.defined_lib_features(cnum)); - } } for (feature, span) in remaining_lib_features { diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-missing.rs b/src/test/ui/stability-attribute/stability-attribute-implies-missing.rs new file mode 100644 index 00000000000..61387853672 --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-implies-missing.rs @@ -0,0 +1,10 @@ +#![feature(staged_api)] +#![stable(feature = "stability_attribute_implies", since = "1.0.0")] + +// Tests that `implied_by = "bar"` results in an error being emitted if `bar` does not exist. + +#[unstable(feature = "foobar", issue = "1", implied_by = "bar")] +//~^ ERROR feature `bar` implying `foobar` does not exist +pub fn foobar() {} + +fn main() {} diff --git a/src/test/ui/stability-attribute/stability-attribute-implies-missing.stderr b/src/test/ui/stability-attribute/stability-attribute-implies-missing.stderr new file mode 100644 index 00000000000..ff1856f1763 --- /dev/null +++ b/src/test/ui/stability-attribute/stability-attribute-implies-missing.stderr @@ -0,0 +1,8 @@ +error: feature `bar` implying `foobar` does not exist + --> $DIR/stability-attribute-implies-missing.rs:6:1 + | +LL | #[unstable(feature = "foobar", issue = "1", implied_by = "bar")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error +