Rollup merge of #66299 - rossmacarthur:fix-41260-avoid-issue-0, r=varkor

support issue = "none" in unstable attributes

This works towards fixing #41260.

This PR allows the use of `issue = "none"` in unstable attributes and makes changes to internally store the issue number as an `Option<NonZeroU32>`. For example:

```rust
#[unstable(feature = "unstable_test_feature", issue = "none")]
fn unstable_issue_none() {}
```

It was not made optional because feedback seen here #60860 suggested that people might forget the issue field if it was optional.

I could not remove the current uses of `issue = "0"` (of which there are a lot) because the stage 0 compiler expects the old syntax. Once this is available in the stage 0 compiler we can replace all uses of `"0"` with `"none"` and no longer allow `"0"`. This is my first time contributing, so I'm not sure what the protocol is with two-part things like this, so some guidance would be appreciated.

r? @varkor

Author: JohnTitor

src/librustc/middle/stability.rs

@@ -22,8 +22,9 @@ use syntax::attr::{self, Stability, Deprecation, RustcDeprecation};
 use crate::ty::{self, TyCtxt};
 use crate::util::nodemap::{FxHashSet, FxHashMap};
 
-use std::mem::replace;
 use std::cmp::Ordering;
+use std::mem::replace;
+use std::num::NonZeroU32;
 
 #[derive(PartialEq, Clone, Copy, Debug)]
 pub enum StabilityLevel {
@@ -441,7 +442,7 @@ impl<'tcx> Index<'tcx> {
                 let stability = tcx.intern_stability(Stability {
                     level: attr::StabilityLevel::Unstable {
                         reason: Some(Symbol::intern(reason)),
-                        issue: 27812,
+                        issue: NonZeroU32::new(27812),
                         is_soft: false,
                     },
                     feature: sym::rustc_private,
@@ -488,7 +489,7 @@ pub fn report_unstable(
     sess: &Session,
     feature: Symbol,
     reason: Option<Symbol>,
-    issue: u32,
+    issue: Option<NonZeroU32>,
     is_soft: bool,
     span: Span,
     soft_handler: impl FnOnce(&'static lint::Lint, Span, &str),
@@ -520,7 +521,7 @@ pub fn report_unstable(
             soft_handler(lint::builtin::SOFT_UNSTABLE, span, &msg)
         } else {
             emit_feature_err(
-                &sess.parse_sess, feature, span, GateIssue::Library(Some(issue)), &msg
+                &sess.parse_sess, feature, span, GateIssue::Library(issue), &msg
             );
         }
     }
@@ -637,7 +638,7 @@ pub enum EvalResult {
     Deny {
         feature: Symbol,
         reason: Option<Symbol>,
-        issue: u32,
+        issue: Option<NonZeroU32>,
         is_soft: bool,
     },
     /// The item does not have the `#[stable]` or `#[unstable]` marker assigned.
@@ -758,7 +759,7 @@ impl<'tcx> TyCtxt<'tcx> {
                 // the `-Z force-unstable-if-unmarked` flag present (we're
                 // compiling a compiler crate), then let this missing feature
                 // annotation slide.
-                if feature == sym::rustc_private && issue == 27812 {
+                if feature == sym::rustc_private && issue == NonZeroU32::new(27812) {
                     if self.sess.opts.debugging_opts.force_unstable_if_unmarked {
                         return EvalResult::Allow;
                     }

src/librustc/session/mod.rs

@@ -41,6 +41,7 @@ use std::cell::{self, RefCell};
 use std::env;
 use std::fmt;
 use std::io::Write;
+use std::num::NonZeroU32;
 use std::path::PathBuf;
 use std::time::Duration;
 use std::sync::Arc;
@@ -183,7 +184,7 @@ enum DiagnosticBuilderMethod {
 pub enum DiagnosticMessageId {
     ErrorId(u16), // EXXXX error code as integer
     LintId(lint::LintId),
-    StabilityId(u32), // issue number
+    StabilityId(Option<NonZeroU32>), // issue number
 }
 
 impl From<&'static lint::Lint> for DiagnosticMessageId {

src/librustdoc/clean/mod.rs

@@ -39,6 +39,7 @@ use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::default::Default;
 use std::{mem, slice, vec};
+use std::num::NonZeroU32;
 use std::iter::FromIterator;
 use std::rc::Rc;
 use std::cell::RefCell;
@@ -4399,7 +4400,7 @@ pub struct Stability {
     pub since: String,
     pub deprecation: Option<Deprecation>,
     pub unstable_reason: Option<String>,
-    pub issue: Option<u32>,
+    pub issue: Option<NonZeroU32>,
 }
 
 #[derive(Clone, Debug)]
@@ -4428,7 +4429,7 @@ impl Clean<Stability> for attr::Stability {
                 _ => None,
             },
             issue: match self.level {
-                attr::Unstable {issue, ..} => Some(issue),
+                attr::Unstable {issue, ..} => issue,
                 _ => None,
             }
         }

src/libsyntax/attr/builtin.rs

@@ -6,6 +6,7 @@ use crate::print::pprust;
 use crate::sess::ParseSess;
 
 use errors::{Applicability, Handler};
+use std::num::NonZeroU32;
 use syntax_pos::hygiene::Transparency;
 use syntax_pos::{symbol::Symbol, symbol::sym, Span};
 
@@ -157,7 +158,7 @@ pub struct Stability {
 #[derive(RustcEncodable, RustcDecodable, PartialEq, PartialOrd, Copy, Clone, Debug, Eq, Hash)]
 pub enum StabilityLevel {
     // Reason for the current stability level and the relevant rust-lang issue
-    Unstable { reason: Option<Symbol>, issue: u32, is_soft: bool },
+    Unstable { reason: Option<Symbol>, issue: Option<NonZeroU32>, is_soft: bool },
     Stable { since: Symbol },
 }
 
@@ -394,18 +395,28 @@ fn find_stability_generic<'a, I>(sess: &ParseSess,
 
                     match (feature, reason, issue) {
                         (Some(feature), reason, Some(issue)) => {
+                            let issue = match &*issue.as_str() {
+                                // FIXME(rossmacarthur): remove "0" because "none" should be used
+                                // See #41260
+                                "none" | "0" => None,
+                                issue => {
+                                    if let Ok(num) = issue.parse() {
+                                        NonZeroU32::new(num)
+                                    } else {
+                                        span_err!(
+                                            diagnostic,
+                                            attr.span,
+                                            E0545,
+                                            "incorrect 'issue'"
+                                        );
+                                        continue
+                                    }
+                                }
+                            };
                             stab = Some(Stability {
                                 level: Unstable {
                                     reason,
-                                    issue: {
-                                        if let Ok(issue) = issue.as_str().parse() {
-                                            issue
-                                        } else {
-                                            span_err!(diagnostic, attr.span, E0545,
-                                                      "incorrect 'issue'");
-                                            continue
-                                        }
-                                    },
+                                    issue,
                                     is_soft,
                                 },
                                 feature,

src/libsyntax/feature_gate/active.rs

@@ -207,10 +207,10 @@ declare_features! (
     /// Allows using `#![needs_allocator]`, an implementation detail of `#[global_allocator]`.
     (active, allocator_internals, "1.20.0", None, None),
 
-    // no-tracking-issue-end
-
     /// Added for testing E0705; perma-unstable.
-    (active, test_2018_feature, "1.31.0", Some(0), Some(Edition::Edition2018)),
+    (active, test_2018_feature, "1.31.0", None, Some(Edition::Edition2018)),
+
+    // no-tracking-issue-end
 
     // -------------------------------------------------------------------------
     // feature-group-end: internal feature gates

src/libsyntax/feature_gate/check.rs

@@ -18,6 +18,7 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan};
 use log::debug;
 
 use std::env;
+use std::num::NonZeroU32;
 
 #[derive(Copy, Clone, Debug)]
 pub enum Stability {
@@ -55,25 +56,28 @@ pub fn check_attribute(attr: &ast::Attribute, parse_sess: &ParseSess, features:
     PostExpansionVisitor { parse_sess, features }.visit_attribute(attr)
 }
 
-fn find_lang_feature_issue(feature: Symbol) -> Option<u32> {
+fn find_lang_feature_issue(feature: Symbol) -> Option<NonZeroU32> {
     if let Some(info) = ACTIVE_FEATURES.iter().find(|t| t.name == feature) {
         // FIXME (#28244): enforce that active features have issue numbers
-        // assert!(info.issue.is_some())
-        info.issue
+        // assert!(info.issue().is_some())
+        info.issue()
     } else {
         // search in Accepted, Removed, or Stable Removed features
-        let found = ACCEPTED_FEATURES.iter().chain(REMOVED_FEATURES).chain(STABLE_REMOVED_FEATURES)
+        let found = ACCEPTED_FEATURES
+            .iter()
+            .chain(REMOVED_FEATURES)
+            .chain(STABLE_REMOVED_FEATURES)
             .find(|t| t.name == feature);
         match found {
-            Some(&Feature { issue, .. }) => issue,
-            None => panic!("Feature `{}` is not declared anywhere", feature),
+            Some(found) => found.issue(),
+            None => panic!("feature `{}` is not declared anywhere", feature),
         }
     }
 }
 
 pub enum GateIssue {
     Language,
-    Library(Option<u32>)
+    Library(Option<NonZeroU32>)
 }
 
 #[derive(Debug, Copy, Clone, PartialEq)]
@@ -126,14 +130,11 @@ fn leveled_feature_err<'a, S: Into<MultiSpan>>(
         GateStrength::Soft => diag.struct_span_warn(span, explain),
     };
 
-    match issue {
-        None | Some(0) => {}  // We still accept `0` as a stand-in for backwards compatibility
-        Some(n) => {
-            err.note(&format!(
-                "for more information, see https://github.com/rust-lang/rust/issues/{}",
-                n,
-            ));
-        }
+    if let Some(n) = issue {
+        err.note(&format!(
+            "for more information, see https://github.com/rust-lang/rust/issues/{}",
+            n,
+        ));
     }
 
     // #23973: do not suggest `#![feature(...)]` if we are in beta/stable

src/libsyntax/feature_gate/mod.rs

@@ -18,8 +18,9 @@ mod active;
 mod builtin_attrs;
 mod check;
 
-use std::fmt;
 use crate::{edition::Edition, symbol::Symbol};
+use std::fmt;
+use std::num::NonZeroU32;
 use syntax_pos::Span;
 
 #[derive(Clone, Copy)]
@@ -46,11 +47,17 @@ pub struct Feature {
     state: State,
     name: Symbol,
     since: &'static str,
-    issue: Option<u32>,
+    issue: Option<u32>,  // FIXME: once #58732 is done make this an Option<NonZeroU32>
     edition: Option<Edition>,
     description: &'static str,
 }
 
+impl Feature {
+    fn issue(&self) -> Option<NonZeroU32> {
+        self.issue.and_then(|i| NonZeroU32::new(i))
+    }
+}
+
 pub use active::{Features, INCOMPLETE_FEATURES};
 pub use builtin_attrs::{
     AttributeGate, AttributeType, GatedCfg,

src/test/ui/feature-gate/unstable-attribute-allow-issue-none.rs

@@ -0,0 +1,13 @@
+// Check that an issue value can be explicitly set to "none" instead of "0"
+#![crate_type = "lib"]
+#![feature(staged_api)]
+#![stable(feature = "stable_test_feature", since = "1.0.0")]
+
+#[unstable(feature = "unstable_test_feature", issue = "0")]
+fn unstable_issue_0() {}
+
+#[unstable(feature = "unstable_test_feature", issue = "none")]
+fn unstable_issue_none() {}
+
+#[unstable(feature = "unstable_test_feature", issue = "something")] //~ ERROR incorrect 'issue'
+fn unstable_issue_not_allowed() {}

src/test/ui/feature-gate/unstable-attribute-allow-issue-none.stderr

@@ -0,0 +1,8 @@
+error[E0545]: incorrect 'issue'
+  --> $DIR/unstable-attribute-allow-issue-none.rs:12:1
+   |
+LL | #[unstable(feature = "unstable_test_feature", issue = "something")]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+