From 75e2624a51f5e3a4b2da47f59b95b15caae64031 Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Thu, 4 Aug 2016 15:06:26 +0200 Subject: [PATCH 1/5] track current_item in Deprecated lint pass --- src/librustc_lint/builtin.rs | 57 ++++++++++++++++++++++++++++++++++-- src/librustc_lint/lib.rs | 2 +- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 7547e28625c18..ac33fb610c566 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -567,10 +567,21 @@ declare_lint! { } /// Checks for use of items with `#[deprecated]` or `#[rustc_deprecated]` attributes -#[derive(Copy, Clone)] -pub struct Deprecated; +#[derive(Clone)] +pub struct Deprecated { + /// Tracks the `NodeId` of the current item. + /// + /// This is required since not all node ids are present in the hir map. + current_item: ast::NodeId, +} impl Deprecated { + pub fn new() -> Deprecated { + Deprecated { + current_item: ast::CRATE_NODE_ID, + } + } + fn lint(&self, cx: &LateContext, _id: DefId, span: Span, stability: &Option<&attr::Stability>, deprecation: &Option) { // Deprecated attributes apply in-crate and cross-crate. @@ -591,6 +602,19 @@ impl Deprecated { cx.span_lint(lint, span, &msg); } } + + fn push_item(&mut self, item_id: ast::NodeId) { + self.current_item = item_id; + } + + fn item_post(&mut self, cx: &LateContext, item_id: ast::NodeId) { + assert_eq!(self.current_item, item_id); + self.current_item = cx.tcx.map.get_parent(item_id); + } + + fn parent_def(&self, cx: &LateContext) -> DefId { + cx.tcx.map.local_def_id(self.current_item) + } } impl LintPass for Deprecated { @@ -601,11 +625,16 @@ impl LintPass for Deprecated { impl LateLintPass for Deprecated { fn check_item(&mut self, cx: &LateContext, item: &hir::Item) { + self.push_item(item.id); stability::check_item(cx.tcx, item, false, &mut |id, sp, stab, depr| self.lint(cx, id, sp, &stab, &depr)); } + fn check_item_post(&mut self, cx: &LateContext, item: &hir::Item) { + self.item_post(cx, item.id); + } + fn check_expr(&mut self, cx: &LateContext, e: &hir::Expr) { stability::check_expr(cx.tcx, e, &mut |id, sp, stab, depr| @@ -629,6 +658,30 @@ impl LateLintPass for Deprecated { &mut |id, sp, stab, depr| self.lint(cx, id, sp, &stab, &depr)); } + + fn check_impl_item(&mut self, _: &LateContext, item: &hir::ImplItem) { + self.push_item(item.id); + } + + fn check_impl_item_post(&mut self, cx: &LateContext, item: &hir::ImplItem) { + self.item_post(cx, item.id); + } + + fn check_trait_item(&mut self, _: &LateContext, item: &hir::TraitItem) { + self.push_item(item.id); + } + + fn check_trait_item_post(&mut self, cx: &LateContext, item: &hir::TraitItem) { + self.item_post(cx, item.id); + } + + fn check_foreign_item(&mut self, _: &LateContext, item: &hir::ForeignItem) { + self.push_item(item.id); + } + + fn check_foreign_item_post(&mut self, cx: &LateContext, item: &hir::ForeignItem) { + self.item_post(cx, item.id); + } } declare_lint! { diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 7b0ee91b69ed0..43376dfd8c2a0 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -124,7 +124,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { UnusedAllocation, MissingCopyImplementations, UnstableFeatures, - Deprecated, UnconditionalRecursion, InvalidNoMangleItems, PluginAsLibrary, @@ -133,6 +132,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { ); add_builtin_with_new!(sess, + Deprecated, TypeLimits, MissingDoc, MissingDebugImplementations, From b4c6a39ccf051c9a05956d25e8b075659ae60d1c Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Thu, 4 Aug 2016 15:18:36 +0200 Subject: [PATCH 2/5] change depr_map to use DeprecationEntry --- src/librustc/middle/stability.rs | 57 +++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 36268a9de960f..209b44e67a1b8 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -19,7 +19,7 @@ use session::Session; use lint; use middle::cstore::LOCAL_CRATE; use hir::def::Def; -use hir::def_id::{CRATE_DEF_INDEX, DefId}; +use hir::def_id::{CRATE_DEF_INDEX, DefId, DefIndex}; use ty::{self, TyCtxt}; use middle::privacy::AccessLevels; use syntax::parse::token::InternedString; @@ -61,12 +61,46 @@ enum AnnotationKind { Container, } +/// An entry in the `depr_map`. +#[derive(Clone)] +pub struct DeprecationEntry { + /// The metadata of the attribute associated with this entry. + pub attr: Deprecation, + /// The def id where the attr was originally attached. `None` for non-local + /// `DefId`'s. + origin: Option, +} + +impl DeprecationEntry { + fn local(attr: Deprecation, id: DefId) -> DeprecationEntry { + assert!(id.is_local()); + DeprecationEntry { + attr: attr, + origin: Some(id.index), + } + } + + fn external(attr: Deprecation) -> DeprecationEntry { + DeprecationEntry { + attr: attr, + origin: None, + } + } + + pub fn same_origin(&self, other: &DeprecationEntry) -> bool { + match (self.origin, other.origin) { + (Some(o1), Some(o2)) => o1 == o2, + _ => false + } + } +} + /// A stability index, giving the stability level for items and methods. pub struct Index<'tcx> { /// This is mostly a cache, except the stabilities of local items /// are filled by the annotator. stab_map: DefIdMap>, - depr_map: DefIdMap>, + depr_map: DefIdMap>, /// Maps for each crate whether it is part of the staged API. staged_api: FnvHashMap @@ -77,7 +111,7 @@ struct Annotator<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, index: &'a mut Index<'tcx>, parent_stab: Option<&'tcx Stability>, - parent_depr: Option, + parent_depr: Option, access_levels: &'a AccessLevels, in_trait_impl: bool, } @@ -184,14 +218,15 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> { // `Deprecation` is just two pointers, no need to intern it let def_id = self.tcx.map.local_def_id(id); - self.index.depr_map.insert(def_id, Some(depr.clone())); + let depr_entry = Some(DeprecationEntry::local(depr, def_id)); + self.index.depr_map.insert(def_id, depr_entry.clone()); - let orig_parent_depr = replace(&mut self.parent_depr, Some(depr)); + let orig_parent_depr = replace(&mut self.parent_depr, depr_entry); visit_children(self); self.parent_depr = orig_parent_depr; - } else if let Some(depr) = self.parent_depr.clone() { + } else if let parent_depr @ Some(_) = self.parent_depr.clone() { let def_id = self.tcx.map.local_def_id(id); - self.index.depr_map.insert(def_id, Some(depr)); + self.index.depr_map.insert(def_id, parent_depr); visit_children(self); } else { visit_children(self); @@ -685,6 +720,10 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { } pub fn lookup_deprecation(self, id: DefId) -> Option { + self.lookup_deprecation_entry(id).map(|depr| depr.attr) + } + + pub fn lookup_deprecation_entry(self, id: DefId) -> Option { if let Some(depr) = self.stability.borrow().depr_map.get(&id) { return depr.clone(); } @@ -703,12 +742,12 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { } } - fn lookup_deprecation_uncached(self, id: DefId) -> Option { + fn lookup_deprecation_uncached(self, id: DefId) -> Option { debug!("lookup(id={:?})", id); if id.is_local() { None // The stability cache is filled partially lazily } else { - self.sess.cstore.deprecation(id) + self.sess.cstore.deprecation(id).map(DeprecationEntry::external) } } } From c17501fea4f9b73134446fbc0700e5d1203909ac Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Thu, 4 Aug 2016 20:13:40 +0200 Subject: [PATCH 3/5] ignore deprecation for items deprecated by the same attribute Whenever a node whould be reported as deprecated: - check if the parent item is also deprecated - if it is and both were deprecated by the same attribute - skip the deprecation warning fixes #35128 closes #16490 --- src/librustc/middle/stability.rs | 16 ++++++++-------- src/librustc_lint/builtin.rs | 13 ++++++++++--- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 209b44e67a1b8..cbbc2c4f98f5e 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -386,7 +386,7 @@ struct Checker<'a, 'tcx: 'a> { impl<'a, 'tcx> Checker<'a, 'tcx> { fn check(&mut self, id: DefId, span: Span, - stab: &Option<&Stability>, _depr: &Option) { + stab: &Option<&Stability>, _depr: &Option) { if !is_staged_api(self.tcx, id) { return; } @@ -511,7 +511,7 @@ pub fn check_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, warn_about_defns: bool, cb: &mut FnMut(DefId, Span, &Option<&Stability>, - &Option)) { + &Option)) { match item.node { hir::ItemExternCrate(_) => { // compiler-generated `extern crate` items have a dummy span. @@ -550,7 +550,7 @@ pub fn check_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, pub fn check_expr<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, e: &hir::Expr, cb: &mut FnMut(DefId, Span, &Option<&Stability>, - &Option)) { + &Option)) { let span; let id = match e.node { hir::ExprMethodCall(i, _, _) => { @@ -614,7 +614,7 @@ pub fn check_path<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, path: &hir::Path, id: ast::NodeId, cb: &mut FnMut(DefId, Span, &Option<&Stability>, - &Option)) { + &Option)) { // Paths in import prefixes may have no resolution. match tcx.expect_def_or_none(id) { Some(Def::PrimTy(..)) => {} @@ -630,7 +630,7 @@ pub fn check_path_list_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &hir::PathListItem, cb: &mut FnMut(DefId, Span, &Option<&Stability>, - &Option)) { + &Option)) { match tcx.expect_def(item.node.id()) { Def::PrimTy(..) => {} def => { @@ -642,7 +642,7 @@ pub fn check_path_list_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, pub fn check_pat<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, pat: &hir::Pat, cb: &mut FnMut(DefId, Span, &Option<&Stability>, - &Option)) { + &Option)) { debug!("check_pat(pat = {:?})", pat); if is_internal(tcx, pat.span) { return; } @@ -673,7 +673,7 @@ fn maybe_do_stability_check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId, span: Span, cb: &mut FnMut(DefId, Span, &Option<&Stability>, - &Option)) { + &Option)) { if is_internal(tcx, span) { debug!("maybe_do_stability_check: \ skipping span={:?} since it is internal", span); @@ -682,7 +682,7 @@ fn maybe_do_stability_check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let (stability, deprecation) = if is_staged_api(tcx, id) { (tcx.lookup_stability(id), None) } else { - (None, tcx.lookup_deprecation(id)) + (None, tcx.lookup_deprecation_entry(id)) }; debug!("maybe_do_stability_check: \ inspecting id={:?} span={:?} of stability={:?}", id, span, stability); diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index ac33fb610c566..49dad2d0f6d92 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -583,13 +583,20 @@ impl Deprecated { } fn lint(&self, cx: &LateContext, _id: DefId, span: Span, - stability: &Option<&attr::Stability>, deprecation: &Option) { + stability: &Option<&attr::Stability>, + deprecation: &Option) { // Deprecated attributes apply in-crate and cross-crate. if let Some(&attr::Stability{rustc_depr: Some(attr::RustcDeprecation{ref reason, ..}), ..}) = *stability { output(cx, DEPRECATED, span, Some(&reason)) - } else if let Some(attr::Deprecation{ref note, ..}) = *deprecation { - output(cx, DEPRECATED, span, note.as_ref().map(|x| &**x)) + } else if let Some(ref depr_entry) = *deprecation { + if let Some(parent_depr) = cx.tcx.lookup_deprecation_entry(self.parent_def(cx)) { + if parent_depr.same_origin(depr_entry) { + return; + } + } + + output(cx, DEPRECATED, span, depr_entry.attr.note.as_ref().map(|x| &**x)) } fn output(cx: &LateContext, lint: &'static Lint, span: Span, note: Option<&str>) { From 98fe30b58bca5ce861eae69e85a28d98f0608505 Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Thu, 4 Aug 2016 16:56:20 +0200 Subject: [PATCH 4/5] fix existing tests --- src/test/compile-fail/deprecation-lint.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/compile-fail/deprecation-lint.rs b/src/test/compile-fail/deprecation-lint.rs index 5fc8f684a66fe..edee24206cd33 100644 --- a/src/test/compile-fail/deprecation-lint.rs +++ b/src/test/compile-fail/deprecation-lint.rs @@ -266,14 +266,14 @@ mod this_crate { #[deprecated(since = "1.0.0", note = "text")] fn test_fn_body() { fn fn_in_body() {} - fn_in_body(); //~ ERROR use of deprecated item: text + fn_in_body(); } impl MethodTester { #[deprecated(since = "1.0.0", note = "text")] fn test_method_body(&self) { fn fn_in_body() {} - fn_in_body(); //~ ERROR use of deprecated item: text + fn_in_body(); } } From 627b1e8ec72dfda6944ff247d50d470f8d1d672b Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Thu, 4 Aug 2016 18:54:04 +0200 Subject: [PATCH 5/5] add test for nested deprecated --- .../compile-fail/deprecation-lint-nested.rs | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 src/test/compile-fail/deprecation-lint-nested.rs diff --git a/src/test/compile-fail/deprecation-lint-nested.rs b/src/test/compile-fail/deprecation-lint-nested.rs new file mode 100644 index 0000000000000..eedbba59c6f48 --- /dev/null +++ b/src/test/compile-fail/deprecation-lint-nested.rs @@ -0,0 +1,81 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![deny(deprecated)] +#![allow(warnings)] + +#[deprecated] +fn issue_35128() { + format_args!("foo"); +} + +#[deprecated] +fn issue_35128_minimal() { + static FOO: &'static str = "foo"; + let _ = FOO; +} + +#[deprecated] +mod silent { + type DeprecatedType = u8; + struct DeprecatedStruct; + fn deprecated_fn() {} + trait DeprecatedTrait {} + static DEPRECATED_STATIC: u8 = 0; + const DEPRECATED_CONST: u8 = 1; + + struct Foo(DeprecatedType); + + impl DeprecatedTrait for Foo {} + + impl Foo { + fn bar() { + deprecated_fn(); + } + } + + fn foo() -> u8 { + DEPRECATED_STATIC + + DEPRECATED_CONST + } +} + +#[deprecated] +mod loud { + #[deprecated] + type DeprecatedType = u8; + #[deprecated] + struct DeprecatedStruct; + #[deprecated] + fn deprecated_fn() {} + #[deprecated] + trait DeprecatedTrait {} + #[deprecated] + static DEPRECATED_STATIC: u8 = 0; + #[deprecated] + const DEPRECATED_CONST: u8 = 1; + + struct Foo(DeprecatedType); //~ ERROR use of deprecated item + + impl DeprecatedTrait for Foo {} //~ ERROR use of deprecated item + + impl Foo { + fn bar() { //~ ERROR use of deprecated item + deprecated_fn(); //~ ERROR use of deprecated item + } + } + + fn foo() -> u8 { + DEPRECATED_STATIC + //~ ERROR use of deprecated item + DEPRECATED_CONST //~ ERROR use of deprecated item + } +} + +fn main() {}