Auto merge of rust-lang#116773 - dtolnay:validatestable, r=compiler-errors

bors · bors · commit 07a4b7e2a9a4 · 2023-10-24T15:06:20.000Z
Validate `feature` and `since` values inside `#[stable(…)]`

Previously the string passed to `#[unstable(feature = "...")]` would be validated as an identifier, but not `#[stable(feature = "...")]`. In the standard library there were `stable` attributes containing the empty string, and kebab-case string, neither of which should be allowed.

Pre-existing validation of `unstable`:

```rust
// src/lib.rs

#![allow(internal_features)]
#![feature(staged_api)]
#![unstable(feature = "kebab-case", issue = "none")]

#[unstable(feature = "kebab-case", issue = "none")]
pub struct Struct;
```

```console
error[E0546]: 'feature' is not an identifier
 --&gt; src/lib.rs:5:1
  |
5 | #![unstable(feature = "kebab-case", issue = "none")]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

For an `unstable` attribute, the need for an identifier is obvious because the downstream code needs to write a `#![feature(...)]` attribute containing that identifier. `#![feature(kebab-case)]` is not valid syntax and `#![feature(kebab_case)]` would not work if that is not the name of the feature.

Having a valid identifier even in `stable` is less essential but still useful because it allows for informative diagnostic about the stabilization of a feature. Compare:

```rust
// src/lib.rs

#![allow(internal_features)]
#![feature(staged_api)]
#![stable(feature = "kebab-case", since = "1.0.0")]

#[stable(feature = "kebab-case", since = "1.0.0")]
pub struct Struct;
```

```rust
// src/main.rs

#![feature(kebab_case)]

use repro::Struct;

fn main() {}
```

```console
error[E0635]: unknown feature `kebab_case`
 --&gt; src/main.rs:3:12
  |
3 | #![feature(kebab_case)]
  |            ^^^^^^^^^^
```

vs the situation if we correctly use `feature = "snake_case"` and `#![feature(snake_case)]`, as enforced by this PR:

```console
warning: the feature `snake_case` has been stable since 1.0.0 and no longer requires an attribute to enable
 --&gt; src/main.rs:3:12
  |
3 | #![feature(snake_case)]
  |            ^^^^^^^^^^
  |
  = note: `#[warn(stable_features)]` on by default
```
diff --git a/compiler/rustc_attr/messages.ftl b/compiler/rustc_attr/messages.ftl
@@ -58,6 +58,9 @@ attr_invalid_repr_hint_no_paren =
 attr_invalid_repr_hint_no_value =
     invalid representation hint: `{$name}` does not take a value
 
+attr_invalid_since =
+    'since' must be a Rust version number, such as "1.31.0"
+
 attr_missing_feature =
     missing 'feature'
 
diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs
@@ -3,6 +3,7 @@
 use rustc_ast::{self as ast, attr};
 use rustc_ast::{Attribute, LitKind, MetaItem, MetaItemKind, MetaItemLit, NestedMetaItem, NodeId};
 use rustc_ast_pretty::pprust;
+use rustc_errors::ErrorGuaranteed;
 use rustc_feature::{find_gated_cfg, is_builtin_attr_name, Features, GatedCfg};
 use rustc_macros::HashStable_Generic;
 use rustc_session::config::ExpectedValues;
@@ -361,25 +362,32 @@ fn parse_stability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabilit
         }
     }
 
-    if let Some(s) = since
-        && s.as_str() == VERSION_PLACEHOLDER
-    {
-        since = Some(rust_version_symbol());
-    }
+    let feature = match feature {
+        Some(feature) if rustc_lexer::is_ident(feature.as_str()) => Ok(feature),
+        Some(_bad_feature) => {
+            Err(sess.emit_err(session_diagnostics::NonIdentFeature { span: attr.span }))
+        }
+        None => Err(sess.emit_err(session_diagnostics::MissingFeature { span: attr.span })),
+    };
+
+    let since = if let Some(since) = since {
+        if since.as_str() == VERSION_PLACEHOLDER {
+            Ok(rust_version_symbol())
+        } else if parse_version(since.as_str(), false).is_some() {
+            Ok(since)
+        } else {
+            Err(sess.emit_err(session_diagnostics::InvalidSince { span: attr.span }))
+        }
+    } else {
+        Err(sess.emit_err(session_diagnostics::MissingSince { span: attr.span }))
+    };
 
     match (feature, since) {
-        (Some(feature), Some(since)) => {
+        (Ok(feature), Ok(since)) => {
             let level = StabilityLevel::Stable { since, allowed_through_unstable_modules: false };
             Some((feature, level))
         }
-        (None, _) => {
-            sess.emit_err(session_diagnostics::MissingFeature { span: attr.span });
-            None
-        }
-        _ => {
-            sess.emit_err(session_diagnostics::MissingSince { span: attr.span });
-            None
-        }
+        (Err(ErrorGuaranteed { .. }), _) | (_, Err(ErrorGuaranteed { .. })) => None,
     }
 }
 
@@ -451,12 +459,19 @@ fn parse_unstability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabil
         }
     }
 
-    match (feature, reason, issue) {
-        (Some(feature), reason, Some(_)) => {
-            if !rustc_lexer::is_ident(feature.as_str()) {
-                sess.emit_err(session_diagnostics::NonIdentFeature { span: attr.span });
-                return None;
-            }
+    let feature = match feature {
+        Some(feature) if rustc_lexer::is_ident(feature.as_str()) => Ok(feature),
+        Some(_bad_feature) => {
+            Err(sess.emit_err(session_diagnostics::NonIdentFeature { span: attr.span }))
+        }
+        None => Err(sess.emit_err(session_diagnostics::MissingFeature { span: attr.span })),
+    };
+
+    let issue =
+        issue.ok_or_else(|| sess.emit_err(session_diagnostics::MissingIssue { span: attr.span }));
+
+    match (feature, issue) {
+        (Ok(feature), Ok(_)) => {
             let level = StabilityLevel::Unstable {
                 reason: UnstableReason::from_opt_reason(reason),
                 issue: issue_num,
@@ -465,14 +480,7 @@ fn parse_unstability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabil
             };
             Some((feature, level))
         }
-        (None, _, _) => {
-            sess.emit_err(session_diagnostics::MissingFeature { span: attr.span });
-            return None;
-        }
-        _ => {
-            sess.emit_err(session_diagnostics::MissingIssue { span: attr.span });
-            return None;
-        }
+        (Err(ErrorGuaranteed { .. }), _) | (_, Err(ErrorGuaranteed { .. })) => None,
     }
 }
 
diff --git a/compiler/rustc_attr/src/session_diagnostics.rs b/compiler/rustc_attr/src/session_diagnostics.rs
@@ -370,6 +370,13 @@ pub(crate) struct ExpectsFeatures {
     pub name: String,
 }
 
+#[derive(Diagnostic)]
+#[diag(attr_invalid_since)]
+pub(crate) struct InvalidSince {
+    #[primary_span]
+    pub span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(attr_soft_no_args)]
 pub(crate) struct SoftNoArgs {
diff --git a/library/core/src/ffi/mod.rs b/library/core/src/ffi/mod.rs
@@ -6,7 +6,7 @@
 //! match those defined by C, so that code that interacts with C will
 //! refer to the correct types.
 
-#![stable(feature = "", since = "1.30.0")]
+#![stable(feature = "core_ffi", since = "1.30.0")]
 #![allow(non_camel_case_types)]
 
 use crate::fmt;
diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs
@@ -1140,10 +1140,10 @@ pub fn read_to_string<R: Read>(mut reader: R) -> Result<String> {
 #[repr(transparent)]
 pub struct IoSliceMut<'a>(sys::io::IoSliceMut<'a>);
 
-#[stable(feature = "iovec-send-sync", since = "1.44.0")]
+#[stable(feature = "iovec_send_sync", since = "1.44.0")]
 unsafe impl<'a> Send for IoSliceMut<'a> {}
 
-#[stable(feature = "iovec-send-sync", since = "1.44.0")]
+#[stable(feature = "iovec_send_sync", since = "1.44.0")]
 unsafe impl<'a> Sync for IoSliceMut<'a> {}
 
 #[stable(feature = "iovec", since = "1.36.0")]
@@ -1283,10 +1283,10 @@ impl<'a> DerefMut for IoSliceMut<'a> {
 #[repr(transparent)]
 pub struct IoSlice<'a>(sys::io::IoSlice<'a>);
 
-#[stable(feature = "iovec-send-sync", since = "1.44.0")]
+#[stable(feature = "iovec_send_sync", since = "1.44.0")]
 unsafe impl<'a> Send for IoSlice<'a> {}
 
-#[stable(feature = "iovec-send-sync", since = "1.44.0")]
+#[stable(feature = "iovec_send_sync", since = "1.44.0")]
 unsafe impl<'a> Sync for IoSlice<'a> {}
 
 #[stable(feature = "iovec", since = "1.36.0")]
diff --git a/src/tools/clippy/tests/ui/missing_const_for_fn/auxiliary/helper.rs b/src/tools/clippy/tests/ui/missing_const_for_fn/auxiliary/helper.rs
@@ -1,8 +1,8 @@
 // This file provides a const function that is unstably const forever.
 
 #![feature(staged_api)]
-#![stable(feature = "1", since = "1.0.0")]
+#![stable(feature = "clippytest", since = "1.0.0")]
 
-#[stable(feature = "1", since = "1.0.0")]
+#[stable(feature = "clippytest", since = "1.0.0")]
 #[rustc_const_unstable(feature = "foo", issue = "none")]
 pub const fn unstably_const_fn() {}
diff --git a/tests/rustdoc/deprecated-future-staged-api.rs b/tests/rustdoc/deprecated-future-staged-api.rs
@@ -1,18 +1,18 @@
 #![feature(staged_api)]
-#![stable(feature = "deprecated-future-staged-api", since = "1.0.0")]
+#![stable(feature = "deprecated_future_staged_api", since = "1.0.0")]
 
 // @has deprecated_future_staged_api/index.html '//*[@class="stab deprecated"]' \
 //      'Deprecation planned'
 // @has deprecated_future_staged_api/struct.S1.html '//*[@class="stab deprecated"]' \
 //      'Deprecating in 99.99.99: effectively never'
 #[deprecated(since = "99.99.99", note = "effectively never")]
-#[stable(feature = "deprecated-future-staged-api", since = "1.0.0")]
+#[stable(feature = "deprecated_future_staged_api", since = "1.0.0")]
 pub struct S1;
 
 // @has deprecated_future_staged_api/index.html '//*[@class="stab deprecated"]' \
 //      'Deprecation planned'
 // @has deprecated_future_staged_api/struct.S2.html '//*[@class="stab deprecated"]' \
 //      'Deprecating in a future Rust version: literally never'
 #[deprecated(since = "TBD", note = "literally never")]
-#[stable(feature = "deprecated-future-staged-api", since = "1.0.0")]
+#[stable(feature = "deprecated_future_staged_api", since = "1.0.0")]
 pub struct S2;
diff --git a/tests/rustdoc/implementor-stable-version.rs b/tests/rustdoc/implementor-stable-version.rs
@@ -1,21 +1,21 @@
-#![stable(feature = "bar", since = "OLD 1.0")]
+#![stable(feature = "bar", since = "3.3.3")]
 #![crate_name = "foo"]
 
 #![feature(staged_api)]
 
-#[stable(feature = "bar", since = "OLD 1.0")]
+#[stable(feature = "bar", since = "3.3.3")]
 pub trait Bar {}
 
-#[stable(feature = "baz", since = "OLD 1.0")]
+#[stable(feature = "baz", since = "3.3.3")]
 pub trait Baz {}
 
-#[stable(feature = "baz", since = "OLD 1.0")]
+#[stable(feature = "baz", since = "3.3.3")]
 pub struct Foo;
 
-// @has foo/trait.Bar.html '//div[@id="implementors-list"]//span[@class="since"]' 'NEW 2.0'
-#[stable(feature = "foobar", since = "NEW 2.0")]
+// @has foo/trait.Bar.html '//div[@id="implementors-list"]//span[@class="since"]' '4.4.4'
+#[stable(feature = "foobar", since = "4.4.4")]
 impl Bar for Foo {}
 
-// @!has foo/trait.Baz.html '//div[@id="implementors-list"]//span[@class="since"]' 'OLD 1.0'
-#[stable(feature = "foobaz", since = "OLD 1.0")]
+// @!has foo/trait.Baz.html '//div[@id="implementors-list"]//span[@class="since"]' '3.3.3'
+#[stable(feature = "foobaz", since = "3.3.3")]
 impl Baz for Foo {}
diff --git a/tests/ui/attributes/const-stability-on-macro.rs b/tests/ui/attributes/const-stability-on-macro.rs
@@ -1,7 +1,7 @@
 #![feature(staged_api)]
 #![stable(feature = "rust1", since = "1.0.0")]
 
-#[rustc_const_stable(feature = "foo", since = "0")]
+#[rustc_const_stable(feature = "foo", since = "3.3.3")]
 //~^ ERROR macros cannot have const stability attributes
 macro_rules! foo {
     () => {};
diff --git a/tests/ui/attributes/const-stability-on-macro.stderr b/tests/ui/attributes/const-stability-on-macro.stderr
@@ -1,8 +1,8 @@
 error: macros cannot have const stability attributes
   --> $DIR/const-stability-on-macro.rs:4:1
    |
-LL | #[rustc_const_stable(feature = "foo", since = "0")]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid const stability attribute
+LL | #[rustc_const_stable(feature = "foo", since = "3.3.3")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid const stability attribute
 LL |
 LL | macro_rules! foo {
    | ---------------- const stability attribute affects this macro
diff --git a/tests/ui/const-generics/defaults/default-annotation.rs b/tests/ui/const-generics/defaults/default-annotation.rs
@@ -4,12 +4,12 @@
 // FIXME(const_generics_defaults): It seems like we aren't testing the right thing here,
 // I would assume that we want the attributes to apply to the const parameter defaults
 // themselves.
-#![stable(feature = "const_default_test", since="none")]
+#![stable(feature = "const_default_test", since = "3.3.3")]
 
-#[unstable(feature = "const_default_stable", issue="none")]
+#[unstable(feature = "const_default_stable", issue = "none")]
 pub struct ConstDefaultUnstable<const N: usize = 3>;
 
-#[stable(feature = "const_default_unstable", since="none")]
+#[stable(feature = "const_default_unstable", since = "3.3.3")]
 pub struct ConstDefaultStable<const N: usize = {
     3
 }>;
diff --git a/tests/ui/deprecation/staged-deprecation-in-future.rs b/tests/ui/deprecation/staged-deprecation-in-future.rs
@@ -2,14 +2,14 @@
 
 #![feature(staged_api)]
 
-#![stable(feature = "rustc_deprecation-in-future-test", since = "1.0.0")]
+#![stable(feature = "rustc_deprecation_in_future_test", since = "1.0.0")]
 
 #[deprecated(since = "99.99.99", note = "effectively never")]
-#[stable(feature = "rustc_deprecation-in-future-test", since = "1.0.0")]
+#[stable(feature = "rustc_deprecation_in_future_test", since = "1.0.0")]
 pub struct S1;
 
 #[deprecated(since = "TBD", note = "literally never")]
-#[stable(feature = "rustc_deprecation-in-future-test", since = "1.0.0")]
+#[stable(feature = "rustc_deprecation_in_future_test", since = "1.0.0")]
 pub struct S2;
 
 fn main() {
diff --git a/tests/ui/feature-gates/feature-gate-staged_api.rs b/tests/ui/feature-gates/feature-gate-staged_api.rs
@@ -1,11 +1,11 @@
-#![stable(feature = "a", since = "b")]
+#![stable(feature = "a", since = "3.3.3")]
 //~^ ERROR stability attributes may not be used outside of the standard library
 mod inner_private_module {
     // UnnameableTypeAlias isn't marked as reachable, so no stability annotation is required here
     pub type UnnameableTypeAlias = u8;
 }
 
-#[stable(feature = "a", since = "b")]
+#[stable(feature = "a", since = "3.3.3")]
 //~^ ERROR stability attributes may not be used outside of the standard library
 pub fn f() -> inner_private_module::UnnameableTypeAlias {
     0
diff --git a/tests/ui/feature-gates/feature-gate-staged_api.stderr b/tests/ui/feature-gates/feature-gate-staged_api.stderr
@@ -1,14 +1,14 @@
 error[E0734]: stability attributes may not be used outside of the standard library
   --> $DIR/feature-gate-staged_api.rs:8:1
    |
-LL | #[stable(feature = "a", since = "b")]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | #[stable(feature = "a", since = "3.3.3")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error[E0734]: stability attributes may not be used outside of the standard library
   --> $DIR/feature-gate-staged_api.rs:1:1
    |
-LL | #![stable(feature = "a", since = "b")]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | #![stable(feature = "a", since = "3.3.3")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 2 previous errors
 
diff --git a/tests/ui/reachable/reachable-unnameable-type-alias.rs b/tests/ui/reachable/reachable-unnameable-type-alias.rs
@@ -1,14 +1,14 @@
 // run-pass
 
 #![feature(staged_api)]
-#![stable(feature = "a", since = "b")]
+#![stable(feature = "a", since = "3.3.3")]
 
 mod inner_private_module {
     // UnnameableTypeAlias isn't marked as reachable, so no stability annotation is required here
     pub type UnnameableTypeAlias = u8;
 }
 
-#[stable(feature = "a", since = "b")]
+#[stable(feature = "a", since = "3.3.3")]
 pub fn f() -> inner_private_module::UnnameableTypeAlias {
     0
 }
diff --git a/tests/ui/repr/16-bit-repr-c-enum.rs b/tests/ui/repr/16-bit-repr-c-enum.rs
@@ -8,7 +8,7 @@
 #![feature(no_core, lang_items, intrinsics, staged_api, rustc_attrs)]
 #![no_core]
 #![crate_type = "lib"]
-#![stable(feature = "", since = "")]
+#![stable(feature = "intrinsics_for_test", since = "3.3.3")]
 #![allow(dead_code)]
 
 // Test that the repr(C) attribute doesn't break compilation
@@ -22,8 +22,8 @@ enum Foo {
 }
 
 extern "rust-intrinsic" {
-    #[stable(feature = "", since = "")]
-    #[rustc_const_stable(feature = "", since = "")]
+    #[stable(feature = "intrinsics_for_test", since = "3.3.3")]
+    #[rustc_const_stable(feature = "intrinsics_for_test", since = "3.3.3")]
     #[rustc_safe_intrinsic]
     fn size_of<T>() -> usize;
 }
diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/default-method-body-is-const-with-staged-api.rs b/tests/ui/rfcs/rfc-2632-const-trait-impl/default-method-body-is-const-with-staged-api.rs
@@ -7,7 +7,7 @@
 
 #![feature(staged_api)]
 #![feature(const_trait_impl)]
-#![stable(since = "1", feature = "foo")]
+#![stable(feature = "foo", since = "3.3.3")]
 
 #[const_trait]
 trait Tr {
diff --git a/tests/ui/stability-attribute/stability-attribute-issue-43027.rs b/tests/ui/stability-attribute/stability-attribute-issue-43027.rs
@@ -1,12 +1,12 @@
 // check-pass
 #![feature(staged_api)]
-#![stable(feature = "test", since = "0")]
+#![stable(feature = "test", since = "3.3.3")]
 
-#[stable(feature = "test", since = "0")]
+#[stable(feature = "test", since = "3.3.3")]
 pub struct A<T>(pub T);
 
-#[stable(feature = "test", since = "0")]
-pub struct B<T>(#[stable(feature = "test", since = "0")] pub T);
+#[stable(feature = "test", since = "3.3.3")]
+pub struct B<T>(#[stable(feature = "test", since = "3.3.3")] pub T);
 
 fn main() {
     // Make sure the field is used to fill the stability cache
diff --git a/tests/ui/stability-attribute/stability-attribute-sanity-4.rs b/tests/ui/stability-attribute/stability-attribute-sanity-4.rs
@@ -17,11 +17,11 @@ mod bogus_attribute_types_2 {
     #[stable = "a"] //~ ERROR malformed `stable` attribute
     fn f4() { }
 
-    #[stable(feature = "a", since = "b")]
+    #[stable(feature = "a", since = "3.3.3")]
     #[deprecated] //~ ERROR missing 'since'
     fn f5() { }
 
-    #[stable(feature = "a", since = "b")]
+    #[stable(feature = "a", since = "3.3.3")]
     #[deprecated = "a"] //~ ERROR missing 'since'
     fn f6() { }
 }
diff --git a/tests/ui/stability-attribute/stability-attribute-sanity.rs b/tests/ui/stability-attribute/stability-attribute-sanity.rs
diff --git a/tests/ui/stability-attribute/stability-attribute-sanity.stderr b/tests/ui/stability-attribute/stability-attribute-sanity.stderr
diff --git a/tests/ui/stability-attribute/stability-attribute-trait-impl.rs b/tests/ui/stability-attribute/stability-attribute-trait-impl.rs
diff --git a/tests/ui/stability-attribute/stability-attribute-trait-impl.stderr b/tests/ui/stability-attribute/stability-attribute-trait-impl.stderr
