From 023274483e5b61202c623352452cf17fcf455a6d Mon Sep 17 00:00:00 2001 From: kennytm Date: Sat, 3 Mar 2018 17:02:01 +0800 Subject: [PATCH] Changed `check_stability` to take an `Option` instead of `NodeId`. This clarifies the intent of whether to emit deprecated lint or not. --- src/librustc/middle/stability.rs | 56 +++++++++++++---------- src/librustc_typeck/astconv.rs | 4 +- src/librustc_typeck/check/_match.rs | 4 +- src/librustc_typeck/check/method/mod.rs | 4 +- src/librustc_typeck/check/method/probe.rs | 2 +- src/librustc_typeck/check/mod.rs | 6 +-- 6 files changed, 43 insertions(+), 33 deletions(-) diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 9193dc086fdb..29c8ac046b81 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -527,17 +527,21 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { /// Evaluates the stability of an item. /// - /// Returns `None` if the item is stable, or unstable but the corresponding `#![feature]` has - /// been provided. Returns the tuple `Some((feature, reason, issue))` of the offending unstable - /// feature otherwise. - pub fn eval_stability(self, def_id: DefId, id: NodeId, span: Span) -> EvalResult { + /// Returns `EvalResult::Allow` if the item is stable, or unstable but the corresponding + /// `#![feature]` has been provided. Returns `EvalResult::Deny` which describes the offending + /// unstable feature otherwise. + /// + /// 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, span: Span) -> EvalResult { if span.allows_unstable() { debug!("stability: \ skipping span={:?} since it is internal", span); return EvalResult::Allow; } - let lint_deprecated = |def_id: DefId, note: Option| { + let lint_deprecated = |def_id: DefId, id: NodeId, note: Option| { let path = self.item_path_str(def_id); let msg = if let Some(note) = note { @@ -547,22 +551,21 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { }; self.lint_node(lint::builtin::DEPRECATED, id, span, &msg); + if id == ast::DUMMY_NODE_ID { + span_bug!(span, "emitted a deprecated lint with dummy node id: {:?}", def_id); + } }; // Deprecated attributes apply in-crate and cross-crate. - if let Some(depr_entry) = self.lookup_deprecation_entry(def_id) { - let skip = if id == ast::DUMMY_NODE_ID { - true - } else { + 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(id)); - self.lookup_deprecation_entry(parent_def_id).map_or(false, |parent_depr| { - parent_depr.same_origin(&depr_entry) - }) + let skip = self.lookup_deprecation_entry(parent_def_id) + .map_or(false, |parent_depr| parent_depr.same_origin(&depr_entry)); + if !skip { + lint_deprecated(def_id, id, depr_entry.attr.note); + } }; - - if !skip { - lint_deprecated(def_id, depr_entry.attr.note); - } } let is_staged_api = self.lookup_stability(DefId { @@ -579,8 +582,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { if let Some(&Stability{rustc_depr: Some(attr::RustcDeprecation { reason, .. }), ..}) = stability { - if id != ast::DUMMY_NODE_ID { - lint_deprecated(def_id, Some(reason)); + if let Some(id) = id { + lint_deprecated(def_id, id, Some(reason)); } } @@ -629,7 +632,14 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } } - pub fn check_stability(self, def_id: DefId, id: NodeId, span: Span) { + /// 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, 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, span: Span) { match self.eval_stability(def_id, id, span) { EvalResult::Allow => {} EvalResult::Deny { feature, reason, issue } => { @@ -687,7 +697,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { None => return, }; let def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX }; - self.tcx.check_stability(def_id, item.id, item.span); + self.tcx.check_stability(def_id, Some(item.id), item.span); } // For implementations of traits, check the stability of each item @@ -700,8 +710,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { let trait_item_def_id = self.tcx.associated_items(trait_did) .find(|item| item.name == impl_item.name).map(|item| item.def_id); if let Some(def_id) = trait_item_def_id { - // Pass `DUMMY_NODE_ID` to skip deprecation warnings. - self.tcx.check_stability(def_id, ast::DUMMY_NODE_ID, impl_item.span); + // Pass `None` to skip deprecation warnings. + self.tcx.check_stability(def_id, None, impl_item.span); } } } @@ -737,7 +747,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { match path.def { Def::Local(..) | Def::Upvar(..) | Def::PrimTy(..) | Def::SelfTy(..) | Def::Err => {} - _ => self.tcx.check_stability(path.def.def_id(), id, path.span) + _ => self.tcx.check_stability(path.def.def_id(), Some(id), path.span) } intravisit::walk_path(self, path) } diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 827ca79334cb..385154152b37 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -530,7 +530,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { let msg = format!("associated type `{}` is private", binding.item_name); tcx.sess.span_err(binding.span, &msg); } - tcx.check_stability(assoc_ty.def_id, ref_id, binding.span); + tcx.check_stability(assoc_ty.def_id, Some(ref_id), binding.span); Ok(candidate.map_bound(|trait_ref| { ty::ProjectionPredicate { @@ -868,7 +868,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { let msg = format!("{} `{}` is private", def.kind_name(), assoc_name); tcx.sess.span_err(span, &msg); } - tcx.check_stability(item.def_id, ref_id, span); + tcx.check_stability(item.def_id, Some(ref_id), span); (ty, def) } diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 7965806af5d0..00c3b2278098 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -861,7 +861,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); let field_ty = self.field_ty(subpat.span, &variant.fields[i], substs); self.check_pat_walk(&subpat, field_ty, def_bm, true); - self.tcx.check_stability(variant.fields[i].did, pat.id, subpat.span); + self.tcx.check_stability(variant.fields[i].did, Some(pat.id), subpat.span); } } else { let subpats_ending = if subpats.len() == 1 { "" } else { "s" }; @@ -923,7 +923,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); vacant.insert(span); field_map.get(&field.name) .map(|f| { - self.tcx.check_stability(f.did, pat_id, span); + self.tcx.check_stability(f.did, Some(pat_id), span); self.field_ty(span, f, substs) }) diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 1664f46464d1..54f41e65d06a 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -171,7 +171,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { .unwrap().insert(import_def_id); } - self.tcx.check_stability(pick.item.def_id, call_expr.id, span); + self.tcx.check_stability(pick.item.def_id, Some(call_expr.id), span); let result = self.confirm_method(span, self_expr, @@ -371,7 +371,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } let def = pick.item.def(); - self.tcx.check_stability(def.def_id(), expr_id, span); + self.tcx.check_stability(def.def_id(), Some(expr_id), span); Ok(def) } diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 738e1ee87c72..ede7703b6196 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -1008,7 +1008,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, '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, ast::DUMMY_NODE_ID, self.span) + self.tcx.eval_stability(p.item.def_id, None, self.span) { uc.push((p, feature)); return false; diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index f86fe1fb7564..9e302c0ffb39 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3078,7 +3078,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.apply_adjustments(base, adjustments); autoderef.finalize(); - self.tcx.check_stability(field.did, expr.id, expr.span); + self.tcx.check_stability(field.did, Some(expr.id), expr.span); return field_ty; } @@ -3219,7 +3219,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if let Some(field) = fields.iter().find(|f| f.name.to_ident() == ident) { let field_ty = self.field_ty(expr.span, field, substs); if field.vis.is_accessible_from(def_scope, self.tcx) { - self.tcx.check_stability(field.did, expr.id, expr.span); + self.tcx.check_stability(field.did, Some(expr.id), expr.span); Some(field_ty) } else { private_candidate = Some((base_def.did, field_ty)); @@ -3364,7 +3364,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // struct-like enums (yet...), but it's definitely not // a bug to have construct one. if adt_kind != ty::AdtKind::Enum { - tcx.check_stability(v_field.did, expr_id, field.span); + tcx.check_stability(v_field.did, Some(expr_id), field.span); } self.field_ty(field.span, v_field, substs)