WIP: stability annotations on generic parameters

src/librustc/middle/privacy.rs

@@ -12,7 +12,7 @@ use std::hash::Hash;
 // Accessibility levels, sorted in ascending order
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, HashStable)]
 pub enum AccessLevel {
-    /// Superset of `AccessLevel::Reachable` used to mark impl Trait items.
+    /// Superset of `AccessLevel::Reachable` used to mark `impl Trait` items.
     ReachableFromImplTrait,
     /// Exported items + items participating in various kinds of public interfaces,
     /// but not directly nameable. For example, if function `fn f() -> T {...}` is

src/librustc/middle/stability.rs

@@ -281,14 +281,21 @@ impl<'tcx> TyCtxt<'tcx> {
     /// If `id` is `Some(_)`, this function will also check if the item at `def_id` has been
     /// deprecated. If the item is indeed deprecated, we will emit a deprecation lint attached to
     /// `id`.
-    pub fn eval_stability(self, def_id: DefId, id: Option<HirId>, span: Span) -> EvalResult {
+    pub fn eval_stability(
+        self,
+        def_id: DefId,
+        id: Option<HirId>,
+        span: Span,
+        inherit_dep: bool,
+    ) -> EvalResult {
         // Deprecated attributes apply in-crate and cross-crate.
         if let Some(id) = id {
             if let Some(depr_entry) = self.lookup_deprecation_entry(def_id) {
                 let parent_def_id = self.hir().local_def_id(self.hir().get_parent_item(id));
-                let skip = self
-                    .lookup_deprecation_entry(parent_def_id)
-                    .map_or(false, |parent_depr| parent_depr.same_origin(&depr_entry));
+                let skip = !inherit_dep
+                    || self
+                        .lookup_deprecation_entry(parent_def_id)
+                        .map_or(false, |parent_depr| parent_depr.same_origin(&depr_entry));
 
                 if !skip {
                     let (message, lint) =
@@ -386,18 +393,36 @@ impl<'tcx> TyCtxt<'tcx> {
     /// Additionally, this function will also check if the item is deprecated. If so, and `id` is
     /// not `None`, a deprecated lint attached to `id` will be emitted.
     pub fn check_stability(self, def_id: DefId, id: Option<HirId>, span: Span) {
+        self.check_stability_internal(def_id, id, span, true, |span, def_id| {
+            // The API could be uncallable for other reasons, for example when a private module
+            // was referenced.
+            self.sess.delay_span_bug(span, &format!("encountered unmarked API: {:?}", def_id));
+        })
+    }
+
+    /// Checks if an item is stable or error out.
+    ///
+    /// If the item defined by `def_id` is unstable and the corresponding `#![feature]` does not
+    /// exist, emits an error.
+    ///
+    /// Additionally when `inherit_dep` is `true`, this function will also check if the item is deprecated. If so, and `id` is
+    /// not `None`, a deprecated lint attached to `id` will be emitted.
+    pub fn check_stability_internal(
+        self,
+        def_id: DefId,
+        id: Option<HirId>,
+        span: Span,
+        inherit_dep: bool,
+        unmarked: impl FnOnce(Span, DefId) -> (),
+    ) {
         let soft_handler =
             |lint, span, msg: &_| self.lint_hir(lint, id.unwrap_or(hir::CRATE_HIR_ID), span, msg);
-        match self.eval_stability(def_id, id, span) {
+        match self.eval_stability(def_id, id, span, inherit_dep) {
             EvalResult::Allow => {}
             EvalResult::Deny { feature, reason, issue, is_soft } => {
                 report_unstable(self.sess, feature, reason, issue, is_soft, span, soft_handler)
             }
-            EvalResult::Unmarked => {
-                // The API could be uncallable for other reasons, for example when a private module
-                // was referenced.
-                self.sess.delay_span_bug(span, &format!("encountered unmarked API: {:?}", def_id));
-            }
+            EvalResult::Unmarked => unmarked(span, def_id),
         }
     }
 

src/librustc_metadata/rmeta/encoder.rs

@@ -1579,9 +1579,13 @@ impl EncodeContext<'tcx> {
                         EntryKind::TypeParam,
                         default.is_some(),
                     );
+                    if default.is_some() {
+                        self.encode_stability(def_id);
+                    }
                 }
                 GenericParamKind::Const { .. } => {
                     self.encode_info_for_generic_param(def_id, EntryKind::ConstParam, true);
+                    // FIXME(const_generics:defaults)
                 }
             }
         }

src/librustc_passes/reachable.rs

@@ -303,7 +303,7 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> {
             Node::Expr(&hir::Expr { kind: hir::ExprKind::Closure(.., body, _, _), .. }) => {
                 self.visit_nested_body(body);
             }
-            // Nothing to recurse on for these
+            // Nothing to recurse on for these.
             Node::ForeignItem(_)
             | Node::Variant(_)
             | Node::Ctor(..)

src/librustc_passes/stability.rs

@@ -26,17 +26,31 @@ use std::cmp::Ordering;
 use std::mem::replace;
 use std::num::NonZeroU32;
 
+/// *** Adding stability attributes to a new kind of node ***
+///
+/// 1.  In `<Annotator as Visitor>`, visit the node and call `self.annotate` to record the new
+///     stability attribute. This will also do some validity checking for the attributes.
+/// 2.  If stability attributes should be required, visit the node in
+///     `<MissingStabilityAnnotations as Visitor>` and call `self.check_missing_stability`.
+/// 3.  If using `check_missing_stability`, you'll also need to make sure the node is reachable. In
+///     `ReachableContext::propagate_node` in `src/librustc_passes/reachable.rs`, make sure the node
+///     is handled. Then, in `<EmbargoVisitor as Visitor>` in `src/librustc_privacy`, visit the node
+///     and call `self.reach`.
+/// 4.  Finally, we need to make sure that stability information is recorded in crate metadata. In
+///     `src/librustc_metadata/rmeta/encoder.rs`, make sure that `self.encode_stability(def_id)` is
+///     called in the relevant `encode_info_for_[your node]` method.
+
 #[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,
-    // Annotation itself is useless, but it can be propagated to children
-    Container,
+    // Annotation is optional. Stability can be propagated to children.
+    Optional,
 }
 
-// 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<'tcx>,
@@ -54,6 +68,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
         attrs: &[Attribute],
         item_sp: Span,
         kind: AnnotationKind,
+        inherit_deprecation: bool,
         visit_children: F,
     ) where
         F: FnOnce(&mut Self),
@@ -65,7 +80,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
                 self.tcx.sess.span_err(
                     item_sp,
                     "`#[deprecated]` cannot be used in staged API; \
-                                                 use `#[rustc_deprecated]` instead",
+                     use `#[rustc_deprecated]` instead",
                 );
             }
             let (stab, const_stab) =
@@ -77,19 +92,22 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
             if let Some(mut stab) = stab {
                 // Error if prohibited, or can't inherit anything from a container.
                 if kind == AnnotationKind::Prohibited
-                    || (kind == AnnotationKind::Container
+                    || (kind == AnnotationKind::Optional
                         && stab.level.is_stable()
                         && stab.rustc_depr.is_none())
                 {
-                    self.tcx.sess.span_err(item_sp, "This stability annotation is useless");
+                    self.tcx.sess.span_err(item_sp, "item does not require a stability annotation");
                 }
 
                 debug!("annotate: found {:?}", stab);
                 // If parent is deprecated and we're not, inherit this by merging
                 // deprecated_since and its reason.
                 if let Some(parent_stab) = self.parent_stab {
-                    if parent_stab.rustc_depr.is_some() && stab.rustc_depr.is_none() {
-                        stab.rustc_depr = parent_stab.rustc_depr
+                    if inherit_deprecation
+                        && parent_stab.rustc_depr.is_some()
+                        && stab.rustc_depr.is_none()
+                    {
+                        stab.rustc_depr = parent_stab.rustc_depr.clone()
                     }
                 }
 
@@ -102,7 +120,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
                     &attr::Stable { since: stab_since },
                 ) = (&stab.rustc_depr, &stab.level)
                 {
-                    // Explicit version of iter::order::lt to handle parse errors properly
+                    // Explicit version of `iter::order::lt` to handle parse errors properly.
                     for (dep_v, stab_v) in
                         dep_since.as_str().split('.').zip(stab_since.as_str().split('.'))
                     {
@@ -111,8 +129,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
                                 Ordering::Less => {
                                     self.tcx.sess.span_err(
                                         item_sp,
-                                        "An API can't be stabilized \
-                                                                     after it is deprecated",
+                                        "an API cannot be stabilized after it has been deprecated",
                                     );
                                     break;
                                 }
@@ -124,8 +141,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
                             // and this makes us not do anything else interesting.
                             self.tcx.sess.span_err(
                                 item_sp,
-                                "Invalid stability or deprecation \
-                                                             version found",
+                                "item has an invalid stability or deprecation version",
                             );
                             break;
                         }
@@ -140,7 +156,10 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
             } else {
                 debug!("annotate: not found, parent = {:?}", self.parent_stab);
                 if let Some(stab) = self.parent_stab {
-                    if stab.level.is_unstable() {
+                    // Instability is inherited from the parent: if something is unstable,
+                    // everything inside it should also be considered unstable for the same
+                    // reason.
+                    if inherit_deprecation && stab.level.is_unstable() {
                         self.index.stab_map.insert(hir_id, stab);
                     }
                 }
@@ -172,14 +191,16 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
             // Propagate unstability.  This can happen even for non-staged-api crates in case
             // -Zforce-unstable-if-unmarked is set.
             if let Some(stab) = self.parent_stab {
-                if stab.level.is_unstable() {
+                if inherit_deprecation && stab.level.is_unstable() {
                     self.index.stab_map.insert(hir_id, stab);
                 }
             }
 
             if let Some(depr) = attr::find_deprecation(&self.tcx.sess.parse_sess, attrs, item_sp) {
                 if kind == AnnotationKind::Prohibited {
-                    self.tcx.sess.span_err(item_sp, "This deprecation annotation is useless");
+                    self.tcx
+                        .sess
+                        .span_err(item_sp, "item does not require a deprecation annotation");
                 }
 
                 // `Deprecation` is just two pointers, no need to intern it
@@ -219,61 +240,89 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
             // optional. They inherit stability from their parents when unannotated.
             hir::ItemKind::Impl { of_trait: None, .. } | hir::ItemKind::ForeignMod(..) => {
                 self.in_trait_impl = false;
-                kind = AnnotationKind::Container;
+                kind = AnnotationKind::Optional;
             }
             hir::ItemKind::Impl { of_trait: Some(_), .. } => {
                 self.in_trait_impl = true;
             }
             hir::ItemKind::Struct(ref sd, _) => {
                 if let Some(ctor_hir_id) = sd.ctor_hir_id() {
-                    self.annotate(ctor_hir_id, &i.attrs, i.span, AnnotationKind::Required, |_| {})
+                    self.annotate(
+                        ctor_hir_id,
+                        &i.attrs,
+                        i.span,
+                        AnnotationKind::Required,
+                        true,
+                        |_| {},
+                    )
                 }
             }
             _ => {}
         }
 
-        self.annotate(i.hir_id, &i.attrs, i.span, kind, |v| intravisit::walk_item(v, i));
+        self.annotate(i.hir_id, &i.attrs, i.span, kind, true, |v| intravisit::walk_item(v, i));
         self.in_trait_impl = orig_in_trait_impl;
     }
 
     fn visit_trait_item(&mut self, ti: &'tcx hir::TraitItem<'tcx>) {
-        self.annotate(ti.hir_id, &ti.attrs, ti.span, AnnotationKind::Required, |v| {
+        self.annotate(ti.hir_id, &ti.attrs, ti.span, AnnotationKind::Required, true, |v| {
             intravisit::walk_trait_item(v, ti);
         });
     }
 
     fn visit_impl_item(&mut self, ii: &'tcx hir::ImplItem<'tcx>) {
         let kind =
             if self.in_trait_impl { AnnotationKind::Prohibited } else { AnnotationKind::Required };
-        self.annotate(ii.hir_id, &ii.attrs, ii.span, kind, |v| {
+        self.annotate(ii.hir_id, &ii.attrs, ii.span, kind, true, |v| {
             intravisit::walk_impl_item(v, ii);
         });
     }
 
     fn visit_variant(&mut self, var: &'tcx Variant<'tcx>, g: &'tcx Generics<'tcx>, item_id: HirId) {
-        self.annotate(var.id, &var.attrs, var.span, AnnotationKind::Required, |v| {
+        self.annotate(var.id, &var.attrs, var.span, AnnotationKind::Required, true, |v| {
             if let Some(ctor_hir_id) = var.data.ctor_hir_id() {
-                v.annotate(ctor_hir_id, &var.attrs, var.span, AnnotationKind::Required, |_| {});
+                v.annotate(
+                    ctor_hir_id,
+                    &var.attrs,
+                    var.span,
+                    AnnotationKind::Required,
+                    true,
+                    |_| {},
+                );
             }
 
             intravisit::walk_variant(v, var, g, item_id)
         })
     }
 
     fn visit_struct_field(&mut self, s: &'tcx StructField<'tcx>) {
-        self.annotate(s.hir_id, &s.attrs, s.span, AnnotationKind::Required, |v| {
+        self.annotate(s.hir_id, &s.attrs, s.span, AnnotationKind::Required, true, |v| {
             intravisit::walk_struct_field(v, s);
         });
     }
 
     fn visit_foreign_item(&mut self, i: &'tcx hir::ForeignItem<'tcx>) {
-        self.annotate(i.hir_id, &i.attrs, i.span, AnnotationKind::Required, |v| {
+        self.annotate(i.hir_id, &i.attrs, i.span, AnnotationKind::Required, true, |v| {
             intravisit::walk_foreign_item(v, i);
         });
     }
 
     fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef<'tcx>) {
-        self.annotate(md.hir_id, &md.attrs, md.span, AnnotationKind::Required, |_| {});
+        self.annotate(md.hir_id, &md.attrs, md.span, AnnotationKind::Required, true, |_| {});
+    }
+
+    fn visit_generic_param(&mut self, p: &'tcx hir::GenericParam<'tcx>) {
+        let kind = match &p.kind {
+            // FIXME(const_generics:defaults)
+            hir::GenericParamKind::Type { default, .. } if default.is_some() => {
+                AnnotationKind::Optional
+            }
+            _ => AnnotationKind::Prohibited,
+        };
+
+        self.annotate(p.hir_id, &p.attrs, p.span, kind, false, |v| {
+            intravisit::walk_generic_param(v, p);
+        });
     }
 }
 
@@ -345,6 +394,10 @@ impl<'a, 'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'a, 'tcx> {
     fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef<'tcx>) {
         self.check_missing_stability(md.hir_id, md.span, "macro");
     }
+
+    // Note that we don't need to `check_missing_stability` for default generic parameters,
+    // as we assume that any default generic parameters without attributes are automatically
+    // stable (assuming they have not inherited instability from their parent).
 }
 
 fn new_index(tcx: TyCtxt<'tcx>) -> Index<'tcx> {
@@ -407,6 +460,7 @@ fn new_index(tcx: TyCtxt<'tcx>) -> Index<'tcx> {
             &krate.attrs,
             krate.span,
             AnnotationKind::Required,
+            true,
             |v| intravisit::walk_crate(v, krate),
         );
     }

src/librustc_typeck/astconv.rs

@@ -2,6 +2,7 @@
 //! Conversion from AST representation of types to the `ty.rs` representation.
 //! The main routine here is `ast_ty_to_ty()`; each use is parameterized by an
 //! instance of `AstConv`.
+// ignore-tidy-filelength
 
 // ignore-tidy-filelength
 
@@ -697,7 +698,17 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                 (GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => {
                     self.ast_region_to_region(&lt, Some(param)).into()
                 }
-                (GenericParamDefKind::Type { .. }, GenericArg::Type(ty)) => {
+                (GenericParamDefKind::Type { has_default, .. }, GenericArg::Type(ty)) => {
+                    if *has_default {
+                        tcx.check_stability_internal(
+                            param.def_id,
+                            Some(arg.id()),
+                            arg.span(),
+                            false,
+                            |_, _| (),
+                        );
+                    }
+
                     self.ast_ty_to_ty(&ty).into()
                 }
                 (GenericParamDefKind::Const, GenericArg::Const(ct)) => {

src/librustc_typeck/check/method/probe.rs

@@ -1252,7 +1252,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         if let Some(uc) = unstable_candidates {
             applicable_candidates.retain(|&(p, _)| {
                 if let stability::EvalResult::Deny { feature, .. } =
-                    self.tcx.eval_stability(p.item.def_id, None, self.span)
+                    self.tcx.eval_stability(p.item.def_id, None, self.span, false)
                 {
                     uc.push((p, feature));
                     return false;