---

yaml
---
r: 96071
b: refs/heads/dist-snap
c: c2c7a24
h: refs/heads/master
i:
  96069: 70d4a07
  96067: 0e04ea8
  96063: f06627b
v: v3

Author: zkamsler

[refs]

@@ -6,7 +6,7 @@ refs/heads/try: c274a6888410ce3e357e014568b43310ed787d36
 refs/tags/release-0.1: 1f5c5126e96c79d22cb7862f75304136e204f105
 refs/heads/ndm: f3868061cd7988080c30d6d5bf352a5a5fe2460b
 refs/heads/try2: 147ecfdd8221e4a4d4e090486829a06da1e0ca3c
-refs/heads/dist-snap: 314d6f693f9442ab40879b58f34c6d0afb76f447
+refs/heads/dist-snap: c2c7a2497d2005f92834a2c2d6204cc7db783ec9
 refs/tags/release-0.2: c870d2dffb391e14efb05aa27898f1f6333a9596
 refs/tags/release-0.3: b5f0d0f648d9a6153664837026ba1be43d3e2503
 refs/heads/try3: 9387340aab40a73e8424c48fd42f0c521a4875c0

branches/dist-snap/src/librustc/driver/driver.rs

@@ -302,10 +302,11 @@ pub fn phase_3_run_analysis_passes(sess: Session,
 
     let reachable_map =
         time(time_passes, "reachability checking", (), |_|
-             reachable::find_reachable(ty_cx, method_map, &exported_items));
+             reachable::find_reachable(ty_cx, method_map, exp_map2,
+                                       &exported_items));
 
     time(time_passes, "lint checking", (), |_|
-         lint::check_crate(ty_cx, &exported_items, crate));
+         lint::check_crate(ty_cx, crate));
 
     CrateAnalysis {
         exp_map2: exp_map2,

branches/dist-snap/src/librustc/middle/lint.rs

@@ -34,7 +34,6 @@
 //! Context itself, span_lint should be used instead of add_lint.
 
 use driver::session;
-use middle::privacy;
 use middle::trans::adt; // for `adt::is_ffi_safe`
 use middle::ty;
 use middle::pat_util;
@@ -310,28 +309,21 @@ pub fn get_lint_dict() -> LintDict {
     return map;
 }
 
-struct Context<'self> {
+struct Context {
     // All known lint modes (string versions)
     dict: @LintDict,
     // Current levels of each lint warning
     cur: SmallIntMap<(level, LintSource)>,
     // context we're checking in (used to access fields like sess)
     tcx: ty::ctxt,
-    // Items exported by the crate; used by the missing_doc lint.
-    exported_items: &'self privacy::ExportedItems,
-    // The id of the current `ast::struct_def` being walked.
-    cur_struct_def_id: ast::NodeId,
-    // Whether some ancestor of the current node was marked
-    // #[doc(hidden)].
-    is_doc_hidden: bool,
 
     // When recursing into an attributed node of the ast which modifies lint
     // levels, this stack keeps track of the previous lint levels of whatever
     // was modified.
     lint_stack: ~[(lint, level, LintSource)],
 }
 
-impl<'self> Context<'self> {
+impl Context {
     fn get_level(&self, lint: lint) -> level {
         match self.cur.find(&(lint as uint)) {
           Some(&(lvl, _)) => lvl,
@@ -440,16 +432,9 @@ impl<'self> Context<'self> {
             true
         };
 
-        let old_is_doc_hidden = self.is_doc_hidden;
-        self.is_doc_hidden = self.is_doc_hidden ||
-            attrs.iter().any(|attr| ("doc" == attr.name() && match attr.meta_item_list()
-                                     { None => false,
-                                       Some(l) => attr::contains_name(l, "hidden") }));
-
         f(self);
 
         // rollback
-        self.is_doc_hidden = old_is_doc_hidden;
         do pushed.times {
             let (lint, lvl, src) = self.lint_stack.pop();
             self.set_level(lint, lvl, src);
@@ -877,81 +862,123 @@ fn check_unnecessary_allocation(cx: &Context, e: &ast::Expr) {
     }
 }
 
-fn check_missing_doc_attrs(cx: &Context,
-                           id: ast::NodeId,
-                           attrs: &[ast::Attribute],
-                           sp: Span,
-                           desc: &'static str) {
-    // If we're building a test harness, then warning about
-    // documentation is probably not really relevant right now.
-    if cx.tcx.sess.opts.test { return }
-
-    // `#[doc(hidden)]` disables missing_doc check.
-    if cx.is_doc_hidden { return }
-
-    // Only check publicly-visible items, using the result from the
-    // privacy pass.
-    if !cx.exported_items.contains(&id) { return }
-
-    if !attrs.iter().any(|a| a.node.is_sugared_doc) {
-        cx.span_lint(missing_doc, sp,
-                     format!("missing documentation for {}", desc));
-    }
-}
+struct MissingDocLintVisitor(ty::ctxt);
 
-fn check_missing_doc_item(cx: &mut Context, it: &ast::item) { // XXX doesn't need to be mut
-    let desc = match it.node {
-        ast::item_fn(*) => "a function",
-        ast::item_mod(*) => "a module",
-        ast::item_enum(*) => "an enum",
-        ast::item_struct(*) => "a struct",
-        ast::item_trait(*) => "a trait",
-        _ => return
-    };
-    check_missing_doc_attrs(cx, it.id, it.attrs, it.span, desc);
-}
+impl MissingDocLintVisitor {
+    fn check_attrs(&self, attrs: &[ast::Attribute], id: ast::NodeId,
+                   sp: Span, msg: ~str) {
+        if !attrs.iter().any(|a| a.node.is_sugared_doc) {
+            self.sess.add_lint(missing_doc, id, sp, msg);
+        }
+    }
 
-fn check_missing_doc_method(cx: &Context, m: &ast::method) {
-    let did = ast::DefId {
-        crate: ast::LOCAL_CRATE,
-        node: m.id
-    };
-    match cx.tcx.methods.find(&did) {
-        None => cx.tcx.sess.span_bug(m.span, "missing method descriptor?!"),
-        Some(md) => {
-            match md.container {
-                // Always check default methods defined on traits.
-                ty::TraitContainer(*) => {}
-                // For methods defined on impls, it depends on whether
-                // it is an implementation for a trait or is a plain
-                // impl.
-                ty::ImplContainer(cid) => {
-                    match ty::impl_trait_ref(cx.tcx, cid) {
-                        Some(*) => return, // impl for trait: don't doc
-                        None => {} // plain impl: doc according to privacy
-                    }
+    fn check_struct(&self, sdef: &ast::struct_def) {
+        for field in sdef.fields.iter() {
+            match field.node.kind {
+                ast::named_field(_, vis) if vis != ast::private => {
+                    self.check_attrs(field.node.attrs, field.node.id, field.span,
+                                     ~"missing documentation for a field");
                 }
+                ast::unnamed_field | ast::named_field(*) => {}
             }
         }
     }
-    check_missing_doc_attrs(cx, m.id, m.attrs, m.span, "a method");
-}
 
-fn check_missing_doc_ty_method(cx: &Context, tm: &ast::TypeMethod) {
-    check_missing_doc_attrs(cx, tm.id, tm.attrs, tm.span, "a type method");
+    fn doc_hidden(&self, attrs: &[ast::Attribute]) -> bool {
+        do attrs.iter().any |attr| {
+            "doc" == attr.name() &&
+                match attr.meta_item_list() {
+                    Some(l) => attr::contains_name(l, "hidden"),
+                    None    => false // not of the form #[doc(...)]
+                }
+        }
+    }
 }
 
-fn check_missing_doc_struct_field(cx: &Context, sf: &ast::struct_field) {
-    match sf.node.kind {
-        ast::named_field(_, vis) if vis != ast::private =>
-            check_missing_doc_attrs(cx, cx.cur_struct_def_id, sf.node.attrs,
-                                    sf.span, "a struct field"),
-        _ => {}
+impl Visitor<()> for MissingDocLintVisitor {
+    fn visit_ty_method(&mut self, m:&ast::TypeMethod, _: ()) {
+        if self.doc_hidden(m.attrs) { return }
+
+        // All ty_method objects are linted about because they're part of a
+        // trait (no visibility)
+        self.check_attrs(m.attrs, m.id, m.span,
+                         ~"missing documentation for a method");
+        visit::walk_ty_method(self, m, ());
     }
-}
 
-fn check_missing_doc_variant(cx: &Context, v: &ast::variant) {
-    check_missing_doc_attrs(cx, v.node.id, v.node.attrs, v.span, "a variant");
+    fn visit_fn(&mut self, fk: &visit::fn_kind, d: &ast::fn_decl,
+                b: &ast::Block, sp: Span, id: ast::NodeId, _: ()) {
+        // Only warn about explicitly public methods.
+        match *fk {
+            visit::fk_method(_, _, m) => {
+                if self.doc_hidden(m.attrs) {
+                    return;
+                }
+                // If we're in a trait implementation, no need to duplicate
+                // documentation
+                if m.vis == ast::public {
+                    self.check_attrs(m.attrs, id, sp,
+                                     ~"missing documentation for a method");
+                }
+            }
+            _ => {}
+        }
+        visit::walk_fn(self, fk, d, b, sp, id, ());
+    }
+
+    fn visit_item(&mut self, it: @ast::item, _: ()) {
+        // If we're building a test harness, then warning about documentation is
+        // probably not really relevant right now
+        if self.sess.opts.test { return }
+        if self.doc_hidden(it.attrs) { return }
+
+        match it.node {
+            ast::item_struct(sdef, _) if it.vis == ast::public => {
+                self.check_attrs(it.attrs, it.id, it.span,
+                                 ~"missing documentation for a struct");
+                self.check_struct(sdef);
+            }
+
+            // Skip implementations because they inherit documentation from the
+            // trait (which was already linted)
+            ast::item_impl(_, Some(*), _, _) => return,
+
+            ast::item_trait(*) if it.vis != ast::public => return,
+            ast::item_trait(*) => self.check_attrs(it.attrs, it.id, it.span,
+                                                   ~"missing documentation for a trait"),
+
+            ast::item_fn(*) if it.vis == ast::public => {
+                self.check_attrs(it.attrs, it.id, it.span,
+                                 ~"missing documentation for a function");
+            }
+
+            ast::item_mod(*) if it.vis == ast::public => {
+                self.check_attrs(it.attrs, it.id, it.span,
+                                 ~"missing documentation for a module");
+            }
+
+            ast::item_enum(ref edef, _) if it.vis == ast::public => {
+                self.check_attrs(it.attrs, it.id, it.span,
+                                 ~"missing documentation for an enum");
+                for variant in edef.variants.iter() {
+                    if variant.node.vis == ast::private { continue; }
+
+                    self.check_attrs(variant.node.attrs, variant.node.id,
+                                     variant.span,
+                                     ~"missing documentation for a variant");
+                    match variant.node.kind {
+                        ast::struct_variant_kind(sdef) => {
+                            self.check_struct(sdef);
+                        }
+                        _ => ()
+                    }
+                }
+            }
+
+            _ => {}
+        }
+        visit::walk_item(self, it, ());
+    }
 }
 
 /// Checks for use of items with #[deprecated], #[experimental] and
@@ -1027,14 +1054,13 @@ fn check_stability(cx: &Context, e: &ast::Expr) {
     cx.span_lint(lint, e.span, msg);
 }
 
-impl<'self> Visitor<()> for Context<'self> {
+impl Visitor<()> for Context {
     fn visit_item(&mut self, it: @ast::item, _: ()) {
         do self.with_lint_attrs(it.attrs) |cx| {
             check_item_ctypes(cx, it);
             check_item_non_camel_case_types(cx, it);
             check_item_non_uppercase_statics(cx, it);
             check_heap_item(cx, it);
-            check_missing_doc_item(cx, it);
 
             do cx.visit_ids |v| {
                 v.visit_item(it, ());
@@ -1077,8 +1103,6 @@ impl<'self> Visitor<()> for Context<'self> {
         match *fk {
             visit::fk_method(_, _, m) => {
                 do self.with_lint_attrs(m.attrs) |cx| {
-                    check_missing_doc_method(cx, m);
-
                     do cx.visit_ids |v| {
                         v.visit_fn(fk, decl, body, span, id, ());
                     }
@@ -1088,45 +1112,9 @@ impl<'self> Visitor<()> for Context<'self> {
             _ => recurse(self),
         }
     }
-
-    fn visit_ty_method(&mut self, t: &ast::TypeMethod, _: ()) {
-        do self.with_lint_attrs(t.attrs) |cx| {
-            check_missing_doc_ty_method(cx, t);
-
-            visit::walk_ty_method(cx, t, ());
-        }
-    }
-
-    fn visit_struct_def(&mut self,
-                        s: @ast::struct_def,
-                        i: ast::Ident,
-                        g: &ast::Generics,
-                        id: ast::NodeId,
-                        _: ()) {
-        let old_id = self.cur_struct_def_id;
-        self.cur_struct_def_id = id;
-        visit::walk_struct_def(self, s, i, g, id, ());
-        self.cur_struct_def_id = old_id;
-    }
-
-    fn visit_struct_field(&mut self, s: @ast::struct_field, _: ()) {
-        do self.with_lint_attrs(s.node.attrs) |cx| {
-            check_missing_doc_struct_field(cx, s);
-
-            visit::walk_struct_field(cx, s, ());
-        }
-    }
-
-    fn visit_variant(&mut self, v: &ast::variant, g: &ast::Generics, _: ()) {
-        do self.with_lint_attrs(v.node.attrs) |cx| {
-            check_missing_doc_variant(cx, v);
-
-            visit::walk_variant(cx, v, g, ());
-        }
-    }
 }
 
-impl<'self> ast_util::IdVisitingOperation for Context<'self> {
+impl ast_util::IdVisitingOperation for Context {
     fn visit_id(&self, id: ast::NodeId) {
         match self.tcx.sess.lints.pop(&id) {
             None => {}
@@ -1139,16 +1127,17 @@ impl<'self> ast_util::IdVisitingOperation for Context<'self> {
     }
 }
 
-pub fn check_crate(tcx: ty::ctxt,
-                   exported_items: &privacy::ExportedItems,
-                   crate: &ast::Crate) {
+pub fn check_crate(tcx: ty::ctxt, crate: &ast::Crate) {
+    // This visitor contains more state than is currently maintained in Context,
+    // and there's no reason for the Context to keep track of this information
+    // really
+    let mut dox = MissingDocLintVisitor(tcx);
+    visit::walk_crate(&mut dox, crate, ());
+
     let mut cx = Context {
         dict: @get_lint_dict(),
         cur: SmallIntMap::new(),
         tcx: tcx,
-        exported_items: exported_items,
-        cur_struct_def_id: -1,
-        is_doc_hidden: false,
         lint_stack: ~[],
     };