Auto merge of #35317 - TimNN:internal-deprecated, r=eddyb

bors · web-flow · commit 4c02363852e6 · 2016-08-05T04:38:41.000-07:00
Ignore deprecation for items deprecated by the same attribute Whenever a node would 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 r? @eddyb
diff --git 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<DefIndex>,
+}
+
+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<Option<&'tcx Stability>>,
-    depr_map: DefIdMap<Option<Deprecation>>,
+    depr_map: DefIdMap<Option<DeprecationEntry>>,
 
     /// Maps for each crate whether it is part of the staged API.
     staged_api: FnvHashMap<ast::CrateNum, bool>
@@ -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<Deprecation>,
+    parent_depr: Option<DeprecationEntry>,
     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);
@@ -351,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<Deprecation>) {
+             stab: &Option<&Stability>, _depr: &Option<DeprecationEntry>) {
         if !is_staged_api(self.tcx, id) {
             return;
         }
@@ -476,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<Deprecation>)) {
+                                           &Option<DeprecationEntry>)) {
     match item.node {
         hir::ItemExternCrate(_) => {
             // compiler-generated `extern crate` items have a dummy span.
@@ -515,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<Deprecation>)) {
+                                           &Option<DeprecationEntry>)) {
     let span;
     let id = match e.node {
         hir::ExprMethodCall(i, _, _) => {
@@ -579,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<Deprecation>)) {
+                                           &Option<DeprecationEntry>)) {
     // Paths in import prefixes may have no resolution.
     match tcx.expect_def_or_none(id) {
         Some(Def::PrimTy(..)) => {}
@@ -595,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<Deprecation>)) {
+                                                     &Option<DeprecationEntry>)) {
     match tcx.expect_def(item.node.id()) {
         Def::PrimTy(..) => {}
         def => {
@@ -607,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<Deprecation>)) {
+                                          &Option<DeprecationEntry>)) {
     debug!("check_pat(pat = {:?})", pat);
     if is_internal(tcx, pat.span) { return; }
 
@@ -638,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<Deprecation>)) {
+                                                     &Option<DeprecationEntry>)) {
     if is_internal(tcx, span) {
         debug!("maybe_do_stability_check: \
                 skipping span={:?} since it is internal", span);
@@ -647,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);
@@ -685,6 +720,10 @@ impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> {
     }
 
     pub fn lookup_deprecation(self, id: DefId) -> Option<Deprecation> {
+        self.lookup_deprecation_entry(id).map(|depr| depr.attr)
+    }
+
+    pub fn lookup_deprecation_entry(self, id: DefId) -> Option<DeprecationEntry> {
         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<Deprecation> {
+    fn lookup_deprecation_uncached(self, id: DefId) -> Option<DeprecationEntry> {
         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)
         }
     }
 }
diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
@@ -567,18 +567,36 @@ 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<attr::Deprecation>) {
+            stability: &Option<&attr::Stability>,
+            deprecation: &Option<stability::DeprecationEntry>) {
         // 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>) {
@@ -591,6 +609,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 +632,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 +665,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
@@ -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,
diff --git a/src/test/compile-fail/deprecation-lint-nested.rs 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<T: DeprecatedTrait>() {
+            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<T: DeprecatedTrait>() { //~ 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() {}
diff --git a/src/test/compile-fail/deprecation-lint.rs b/src/test/compile-fail/deprecation-lint.rs
