From 6f28bda042be75ece4a09565ce585ce9917c5675 Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 27 Oct 2024 19:03:10 -0700 Subject: [PATCH] Add sanity checks for `reason` and `soft` on unstable attributes `reason` must appear at most once: it's the reason for the item being unstable, rather than a particular feature. This simplifies diagnostic formatting. `soft` must either be on all or no unstable attributes: it doesn't make sense for something to be partially soft, and allowing inconsistent softness markers would risk an item accidentally becoming properly unstable as its features stabilize. --- compiler/rustc_attr/messages.ftl | 6 ++++ compiler/rustc_attr/src/builtin.rs | 16 +++++++-- .../rustc_attr/src/session_diagnostics.rs | 14 ++++++++ .../stability-attribute/mixed-levels.stderr | 2 -- .../multiple-stability-attribute-sanity.rs | 24 +++++++++++++ ...multiple-stability-attribute-sanity.stderr | 36 +++++++++++++++++++ 6 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 tests/ui/stability-attribute/multiple-stability-attribute-sanity.rs create mode 100644 tests/ui/stability-attribute/multiple-stability-attribute-sanity.stderr diff --git a/compiler/rustc_attr/messages.ftl b/compiler/rustc_attr/messages.ftl index d8a1d18b5e7d0..0f6ebcf32e4b1 100644 --- a/compiler/rustc_attr/messages.ftl +++ b/compiler/rustc_attr/messages.ftl @@ -85,6 +85,9 @@ attr_multiple_item = attr_multiple_stability_levels = multiple stability levels for feature `{$feature}` +attr_multiple_unstable_reasons = + multiple reasons provided for unstability + attr_non_ident_feature = 'feature' is not an identifier @@ -97,6 +100,9 @@ attr_rustc_const_stable_indirect_pairing = attr_rustc_promotable_pairing = `rustc_promotable` attribute must be paired with either a `rustc_const_unstable` or a `rustc_const_stable` attribute +attr_soft_inconsistent = + `soft` must be present on either none or all of an item's `unstable` attributes + attr_soft_no_args = `soft` should not have any arguments diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index fac5b4adf2abf..c49791d81e77e 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -416,11 +416,21 @@ fn add_level( (_, UnstableReason::None) => {} (reason @ UnstableReason::None, _) => *reason = new_reason, _ => { - // TODO: sanity check for only one reason + sess.dcx() + .emit_err(session_diagnostics::MultipleUnstableReasons { span: attr.span }); } } - // TODO: sanity check for is_soft consistency - *is_soft |= new_soft; + // If any unstable attributes are marked 'soft', all should be. This keeps soft-unstable + // items from accidentally being made properly unstable as attributes are removed. + if *is_soft != new_soft { + let spans = stab_spans + .iter() + .filter(|(stab, _)| stab.is_unstable()) + .map(|&(_, sp)| sp) + .chain([attr.span]) + .collect(); + sess.dcx().emit_err(session_diagnostics::SoftInconsistent { spans }); + } } // an item with some stable and some unstable features is unstable (Some(Unstable { .. }), Stable { .. }) => {} diff --git a/compiler/rustc_attr/src/session_diagnostics.rs b/compiler/rustc_attr/src/session_diagnostics.rs index 1bf11e7196b4b..c6d388657bd3a 100644 --- a/compiler/rustc_attr/src/session_diagnostics.rs +++ b/compiler/rustc_attr/src/session_diagnostics.rs @@ -83,6 +83,13 @@ pub(crate) struct MultipleStabilityLevels { pub feature: Symbol, } +#[derive(Diagnostic)] +#[diag(attr_multiple_unstable_reasons)] +pub(crate) struct MultipleUnstableReasons { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(attr_invalid_issue_string, code = E0545)] pub(crate) struct InvalidIssueString { @@ -400,6 +407,13 @@ pub(crate) struct SoftNoArgs { pub span: Span, } +#[derive(Diagnostic)] +#[diag(attr_soft_inconsistent)] +pub(crate) struct SoftInconsistent { + #[primary_span] + pub spans: Vec, +} + #[derive(Diagnostic)] #[diag(attr_unknown_version_literal)] pub(crate) struct UnknownVersionLiteral { diff --git a/tests/ui/stability-attribute/mixed-levels.stderr b/tests/ui/stability-attribute/mixed-levels.stderr index 3d3e26f4e08a8..745c6751bb442 100644 --- a/tests/ui/stability-attribute/mixed-levels.stderr +++ b/tests/ui/stability-attribute/mixed-levels.stderr @@ -13,9 +13,7 @@ error: `const_unstable_fn` is not yet stable as a const fn LL | const USE_UNSTABLE: () = mixed_levels::const_unstable_fn(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: use of unstable library feature `unstable_c` = help: add `#![feature(unstable_c)]` to the crate attributes to enable - = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error: aborting due to 2 previous errors diff --git a/tests/ui/stability-attribute/multiple-stability-attribute-sanity.rs b/tests/ui/stability-attribute/multiple-stability-attribute-sanity.rs new file mode 100644 index 0000000000000..f4c02d69ab325 --- /dev/null +++ b/tests/ui/stability-attribute/multiple-stability-attribute-sanity.rs @@ -0,0 +1,24 @@ +//! Checks that multiple stability attributes are used correctly together + +#![feature(staged_api)] + +#![stable(feature = "stable_test_feature", since = "1.0.0")] + +#[unstable(feature = "a", issue = "none", reason = "reason 1")] +#[unstable(feature = "b", issue = "none", reason = "reason 2")] //~ ERROR multiple reasons provided for unstability +fn f1() { } + +#[unstable(feature = "a", issue = "none", reason = "reason 1")] +#[unstable(feature = "b", issue = "none", reason = "reason 2")] //~ ERROR multiple reasons provided for unstability +#[unstable(feature = "c", issue = "none", reason = "reason 3")] //~ ERROR multiple reasons provided for unstability +fn f2() { } + +#[unstable(feature = "a", issue = "none")] //~ ERROR `soft` must be present on either none or all of an item's `unstable` attributes +#[unstable(feature = "b", issue = "none", soft)] +fn f3() { } + +#[unstable(feature = "a", issue = "none", soft)] //~ ERROR `soft` must be present on either none or all of an item's `unstable` attributes +#[unstable(feature = "b", issue = "none")] +fn f4() { } + +fn main() { } diff --git a/tests/ui/stability-attribute/multiple-stability-attribute-sanity.stderr b/tests/ui/stability-attribute/multiple-stability-attribute-sanity.stderr new file mode 100644 index 0000000000000..a7cb4849d1245 --- /dev/null +++ b/tests/ui/stability-attribute/multiple-stability-attribute-sanity.stderr @@ -0,0 +1,36 @@ +error: multiple reasons provided for unstability + --> $DIR/multiple-stability-attribute-sanity.rs:8:1 + | +LL | #[unstable(feature = "b", issue = "none", reason = "reason 2")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: multiple reasons provided for unstability + --> $DIR/multiple-stability-attribute-sanity.rs:12:1 + | +LL | #[unstable(feature = "b", issue = "none", reason = "reason 2")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: multiple reasons provided for unstability + --> $DIR/multiple-stability-attribute-sanity.rs:13:1 + | +LL | #[unstable(feature = "c", issue = "none", reason = "reason 3")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `soft` must be present on either none or all of an item's `unstable` attributes + --> $DIR/multiple-stability-attribute-sanity.rs:16:1 + | +LL | #[unstable(feature = "a", issue = "none")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #[unstable(feature = "b", issue = "none", soft)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `soft` must be present on either none or all of an item's `unstable` attributes + --> $DIR/multiple-stability-attribute-sanity.rs:20:1 + | +LL | #[unstable(feature = "a", issue = "none", soft)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #[unstable(feature = "b", issue = "none")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors +