Auto merge of #56578 - alexreg:cosmetic-1, r=alexreg

Various minor/cosmetic improvements to code

r? @Centril 😄

13 files changed, 166 insertions, 171 deletions

diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs
index 543df844cb8..ac9680b4570 100644
--- a/src/librustdoc/clean/auto_trait.rs
+++ b/src/librustdoc/clean/auto_trait.rs
@@ -255,7 +255,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> AutoTraitFinder<'a, 'tcx, 'rcx, 'cstore> {
     // handle_lifetimes determines what *needs be* true in order for an impl to hold.
     // lexical_region_resolve, along with much of the rest of the compiler, is concerned
     // with determining if a given set up constraints/predicates *are* met, given some
-    // starting conditions (e.g. user-provided code). For this reason, it's easier
+    // starting conditions (e.g., user-provided code). For this reason, it's easier
     // to perform the calculations we need on our own, rather than trying to make
     // existing inference/solver code do what we want.
     fn handle_lifetimes<'cx>(
@@ -274,7 +274,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> AutoTraitFinder<'a, 'tcx, 'rcx, 'cstore> {
         // Flattening is done in two parts. First, we insert all of the constraints
         // into a map. Each RegionTarget (either a RegionVid or a Region) maps
         // to its smaller and larger regions. Note that 'larger' regions correspond
-        // to sub-regions in Rust code (e.g. in 'a: 'b, 'a is the larger region).
+        // to sub-regions in Rust code (e.g., in 'a: 'b, 'a is the larger region).
         for constraint in regions.constraints.keys() {
             match constraint {
                 &Constraint::VarSubVar(r1, r2) => {
@@ -524,7 +524,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> AutoTraitFinder<'a, 'tcx, 'rcx, 'cstore> {
     // display on the docs page. Cleaning the Predicates produces sub-optimal WherePredicate's,
     // so we fix them up:
     //
-    // * Multiple bounds for the same type are coalesced into one: e.g. 'T: Copy', 'T: Debug'
+    // * Multiple bounds for the same type are coalesced into one: e.g., 'T: Copy', 'T: Debug'
     // becomes 'T: Copy + Debug'
     // * Fn bounds are handled specially - instead of leaving it as 'T: Fn(), <T as Fn::Output> =
     // K', we use the dedicated syntax 'T: Fn() -> K'
@@ -545,7 +545,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> AutoTraitFinder<'a, 'tcx, 'rcx, 'cstore> {
         );
 
         // The `Sized` trait must be handled specially, since we only only display it when
-        // it is *not* required (i.e. '?Sized')
+        // it is *not* required (i.e., '?Sized')
         let sized_trait = self.cx
             .tcx
             .require_lang_item(lang_items::SizedTraitLangItem);
@@ -629,7 +629,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> AutoTraitFinder<'a, 'tcx, 'rcx, 'cstore> {
                         let is_fn = match &mut b {
                             &mut GenericBound::TraitBound(ref mut p, _) => {
                                 // Insert regions into the for_generics hash map first, to ensure
-                                // that we don't end up with duplicate bounds (e.g. for<'b, 'b>)
+                                // that we don't end up with duplicate bounds (e.g., for<'b, 'b>)
                                 for_generics.extend(p.generic_params.clone());
                                 p.generic_params = for_generics.into_iter().collect();
                                 self.is_fn_ty(&tcx, &p.trait_)
@@ -737,7 +737,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> AutoTraitFinder<'a, 'tcx, 'rcx, 'cstore> {
                                         hir::TraitBoundModifier::None,
                                     ));
 
-                                    // Remove any existing 'plain' bound (e.g. 'T: Iterator`) so
+                                    // Remove any existing 'plain' bound (e.g., 'T: Iterator`) so
                                     // that we don't see a
                                     // duplicate bound like `T: Iterator + Iterator<Item=u8>`
                                     // on the docs page.
@@ -837,7 +837,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> AutoTraitFinder<'a, 'tcx, 'rcx, 'cstore> {
     // auto-trait impls always render in exactly the same way.
     //
     // Using the Debug implementation for sorting prevents us from needing to
-    // write quite a bit of almost entirely useless code (e.g. how should two
+    // write quite a bit of almost entirely useless code (e.g., how should two
     // Types be sorted relative to each other). It also allows us to solve the
     // problem for both WherePredicates and GenericBounds at the same time. This
     // approach is probably somewhat slower, but the small number of items
diff --git a/src/librustdoc/clean/cfg.rs b/src/librustdoc/clean/cfg.rs
index f90f1e54da2..847786d123e 100644
--- a/src/librustdoc/clean/cfg.rs
+++ b/src/librustdoc/clean/cfg.rs
@@ -31,13 +31,13 @@ pub enum Cfg {
     True,
     /// Denies all configurations.
     False,
-    /// A generic configuration option, e.g. `test` or `target_os = "linux"`.
+    /// A generic configuration option, e.g., `test` or `target_os = "linux"`.
     Cfg(Symbol, Option<Symbol>),
-    /// Negate a configuration requirement, i.e. `not(x)`.
+    /// Negate a configuration requirement, i.e., `not(x)`.
     Not(Box<Cfg>),
-    /// Union of a list of configuration requirements, i.e. `any(...)`.
+    /// Union of a list of configuration requirements, i.e., `any(...)`.
     Any(Vec<Cfg>),
-    /// Intersection of a list of configuration requirements, i.e. `all(...)`.
+    /// Intersection of a list of configuration requirements, i.e., `all(...)`.
     All(Vec<Cfg>),
 }
 
@@ -61,7 +61,7 @@ impl Cfg {
 
     /// Parses a `MetaItem` into a `Cfg`.
     ///
-    /// The `MetaItem` should be the content of the `#[cfg(...)]`, e.g. `unix` or
+    /// The `MetaItem` should be the content of the `#[cfg(...)]`, e.g., `unix` or
     /// `target_os = "redox"`.
     ///
     /// If the content is not properly formatted, it will return an error indicating what and where
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index ce7de0e3128..64f66d55fc6 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -11,39 +11,39 @@
 //! This module contains the "cleaned" pieces of the AST, and the functions
 //! that clean them.
 
-pub use self::Type::*;
-pub use self::Mutability::*;
-pub use self::ItemEnum::*;
-pub use self::SelfTy::*;
-pub use self::FunctionRetTy::*;
-pub use self::Visibility::{Public, Inherited};
+pub mod inline;
+pub mod cfg;
+mod simplify;
+mod auto_trait;
+mod blanket_impl;
+pub mod def_ctor;
 
+use rustc_data_structures::indexed_vec::{IndexVec, Idx};
+use rustc_data_structures::sync::Lrc;
 use rustc_target::spec::abi::Abi;
-use syntax::ast::{self, AttrStyle, Ident};
-use syntax::attr;
-use syntax::ext::base::MacroKind;
-use syntax::source_map::{dummy_spanned, Spanned};
-use syntax::ptr::P;
-use syntax::symbol::keywords::{self, Keyword};
-use syntax::symbol::InternedString;
-use syntax_pos::{self, DUMMY_SP, Pos, FileName};
-
+use rustc_typeck::hir_ty_to_ty;
+use rustc::infer::region_constraints::{RegionConstraintData, Constraint};
 use rustc::mir::interpret::ConstValue;
 use rustc::middle::resolve_lifetime as rl;
-use rustc::ty::fold::TypeFolder;
 use rustc::middle::lang_items;
+use rustc::middle::stability;
 use rustc::mir::interpret::GlobalId;
 use rustc::hir::{self, GenericArg, HirVec};
 use rustc::hir::def::{self, Def, CtorKind};
 use rustc::hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE};
 use rustc::ty::subst::Substs;
 use rustc::ty::{self, TyCtxt, Region, RegionVid, Ty, AdtKind};
+use rustc::ty::fold::TypeFolder;
 use rustc::ty::layout::VariantIdx;
-use rustc::middle::stability;
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
-use rustc_typeck::hir_ty_to_ty;
-use rustc::infer::region_constraints::{RegionConstraintData, Constraint};
-use rustc_data_structures::indexed_vec::{IndexVec, Idx};
+use syntax::ast::{self, AttrStyle, Ident};
+use syntax::attr;
+use syntax::ext::base::MacroKind;
+use syntax::source_map::{dummy_spanned, Spanned};
+use syntax::ptr::P;
+use syntax::symbol::keywords::{self, Keyword};
+use syntax::symbol::InternedString;
+use syntax_pos::{self, DUMMY_SP, Pos, FileName};
 
 use std::collections::hash_map::Entry;
 use std::fmt;
@@ -51,7 +51,6 @@ use std::hash::{Hash, Hasher};
 use std::default::Default;
 use std::{mem, slice, vec};
 use std::iter::{FromIterator, once};
-use rustc_data_structures::sync::Lrc;
 use std::rc::Rc;
 use std::str::FromStr;
 use std::cell::RefCell;
@@ -66,17 +65,17 @@ use visit_ast;
 use html::render::{cache, ExternalLocation};
 use html::item_type::ItemType;
 
-pub mod inline;
-pub mod cfg;
-mod simplify;
-mod auto_trait;
-mod blanket_impl;
-pub mod def_ctor;
-
 use self::cfg::Cfg;
 use self::auto_trait::AutoTraitFinder;
 use self::blanket_impl::BlanketImplFinder;
 
+pub use self::Type::*;
+pub use self::Mutability::*;
+pub use self::ItemEnum::*;
+pub use self::SelfTy::*;
+pub use self::FunctionRetTy::*;
+pub use self::Visibility::{Public, Inherited};
+
 thread_local!(pub static MAX_DEF_ID: RefCell<FxHashMap<CrateNum, DefId>> = Default::default());
 
 const FN_OUTPUT_NAME: &'static str = "Output";
@@ -1621,7 +1620,7 @@ impl<'a, 'tcx> Clean<Generics> for (&'a ty::Generics,
         }
 
         // It would be nice to collect all of the bounds on a type and recombine
-        // them if possible, to avoid e.g. `where T: Foo, T: Bar, T: Sized, T: 'a`
+        // them if possible, to avoid e.g., `where T: Foo, T: Bar, T: Sized, T: 'a`
         // and instead see `where T: Foo + Bar + Sized + 'a`
 
         Generics {
@@ -3899,7 +3898,7 @@ impl Clean<Deprecation> for attr::Deprecation {
     }
 }
 
-/// An equality constraint on an associated type, e.g. `A=Bar` in `Foo<A=Bar>`
+/// An equality constraint on an associated type, e.g., `A=Bar` in `Foo<A=Bar>`
 #[derive(Clone, PartialEq, Eq, RustcDecodable, RustcEncodable, Debug, Hash)]
 pub struct TypeBinding {
     pub name: String,
diff --git a/src/librustdoc/clean/simplify.rs b/src/librustdoc/clean/simplify.rs
index 635608d140d..81608b380d0 100644
--- a/src/librustdoc/clean/simplify.rs
+++ b/src/librustdoc/clean/simplify.rs
@@ -12,7 +12,7 @@
 //! more canonical form.
 //!
 //! Currently all cross-crate-inlined function use `rustc::ty` to reconstruct
-//! the AST (e.g. see all of `clean::inline`), but this is not always a
+//! the AST (e.g., see all of `clean::inline`), but this is not always a
 //! non-lossy transformation. The current format of storage for where clauses
 //! for functions and such is simply a list of predicates. One example of this
 //! is that the AST predicate of: `where T: Trait<Foo=Bar>` is encoded as:
diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs
index fa5d015e5b7..6ce02c313ee 100644
--- a/src/librustdoc/html/layout.rs
+++ b/src/librustdoc/html/layout.rs
@@ -130,7 +130,7 @@ pub fn render<T: fmt::Display, S: fmt::Display>(
             <div class=\"infos\">\
                 <h2>Search Tricks</h2>\
                 <p>\
-                    Prefix searches with a type followed by a colon (e.g. \
+                    Prefix searches with a type followed by a colon (e.g., \
                     <code>fn:</code>) to restrict the search to a given type.\
                 </p>\
                 <p>\
@@ -140,11 +140,11 @@ pub fn render<T: fmt::Display, S: fmt::Display>(
                     and <code>const</code>.\
                 </p>\
                 <p>\
-                    Search functions by type signature (e.g. \
+                    Search functions by type signature (e.g., \
                     <code>vec -> usize</code> or <code>* -> vec</code>)\
                 </p>\
                 <p>\
-                    Search multiple things at once by splitting your query with comma (e.g. \
+                    Search multiple things at once by splitting your query with comma (e.g., \
                     <code>str,u8</code> or <code>String,struct:Vec,test</code>)\
                 </p>\
             </div>\
diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs
index b316cddf0c8..8d7942a1466 100644
--- a/src/librustdoc/html/render.rs
+++ b/src/librustdoc/html/render.rs
@@ -2071,7 +2071,7 @@ impl Context {
     fn item<F>(&mut self, item: clean::Item, all: &mut AllTypes, mut f: F) -> Result<(), Error>
         where F: FnMut(&mut Context, clean::Item),
     {
-        // Stripped modules survive the rustdoc passes (i.e. `strip-private`)
+        // Stripped modules survive the rustdoc passes (i.e., `strip-private`)
         // if they contain impls for public types. These modules can also
         // contain items such as publicly re-exported structures.
         //
diff --git a/src/librustdoc/html/toc.rs b/src/librustdoc/html/toc.rs
index 88ada5c7f7f..45bd6990fab 100644
--- a/src/librustdoc/html/toc.rs
+++ b/src/librustdoc/html/toc.rs
@@ -52,7 +52,7 @@ pub struct TocEntry {
 pub struct TocBuilder {
     top_level: Toc,
     /// The current hierarchy of parent headings, the levels are
-    /// strictly increasing (i.e. chain[0].level < chain[1].level <
+    /// strictly increasing (i.e., chain[0].level < chain[1].level <
     /// ...) with each entry being the most recent occurrence of a
     /// heading with that level (it doesn't include the most recent
     /// occurrences of every level, just, if it *is* in `chain` then
@@ -76,7 +76,7 @@ impl TocBuilder {
     }
 
     /// Collapse the chain until the first heading more important than
-    /// `level` (i.e. lower level)
+    /// `level` (i.e., lower level)
     ///
     /// Example:
     ///
@@ -91,7 +91,7 @@ impl TocBuilder {
     /// ### H
     /// ```
     ///
-    /// If we are considering H (i.e. level 3), then A and B are in
+    /// If we are considering H (i.e., level 3), then A and B are in
     /// self.top_level, D is in C.children, and C, E, F, G are in
     /// self.chain.
     ///
@@ -102,7 +102,7 @@ impl TocBuilder {
     ///
     /// This leaves us looking at E, which does have a smaller level,
     /// and, by construction, it's the most recent thing with smaller
-    /// level, i.e. it's the immediate parent of H.
+    /// level, i.e., it's the immediate parent of H.
     fn fold_until(&mut self, level: u32) {
         let mut this = None;
         loop {
@@ -133,7 +133,7 @@ impl TocBuilder {
         assert!(level >= 1);
 
         // collapse all previous sections into their parents until we
-        // get to relevant heading (i.e. the first one with a smaller
+        // get to relevant heading (i.e., the first one with a smaller
         // level than us)
         self.fold_until(level);
 
@@ -150,7 +150,7 @@ impl TocBuilder {
                     (entry.level, &entry.children)
                 }
             };
-            // fill in any missing zeros, e.g. for
+            // fill in any missing zeros, e.g., for
             // # Foo (1)
             // ### Bar (1.0.1)
             for _ in toc_level..level - 1 {
diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs
index b21f005c06b..b0045e41f50 100644
--- a/src/librustdoc/lib.rs
+++ b/src/librustdoc/lib.rs
@@ -276,7 +276,7 @@ fn opts() -> Vec<RustcOptGroup> {
         unstable("resource-suffix", |o| {
             o.optopt("",
                      "resource-suffix",
-                     "suffix to add to CSS and JavaScript files, e.g. \"light.css\" will become \
+                     "suffix to add to CSS and JavaScript files, e.g., \"light.css\" will become \
                       \"light-suffix.css\"",
                      "PATH")
         }),
diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs
index 8008f8848d4..e0e0be717b2 100644
--- a/src/librustdoc/markdown.rs
+++ b/src/librustdoc/markdown.rs
@@ -46,8 +46,8 @@ fn extract_leading_metadata<'a>(s: &'a str) -> (Vec<&'a str>, &'a str) {
     (metadata, "")
 }
 
-/// Render `input` (e.g. "foo.md") into an HTML file in `output`
-/// (e.g. output = "bar" => "bar/foo.html").
+/// Render `input` (e.g., "foo.md") into an HTML file in `output`
+/// (e.g., output = "bar" => "bar/foo.html").
 pub fn render(input: PathBuf, options: RenderOptions, diag: &errors::Handler) -> isize {
     let mut output = options.output;
     output.push(input.file_stem().unwrap());
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index cbbcf92c6aa..426d3f3eeea 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -8,8 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use clean::*;
-
 use rustc::lint as lint;
 use rustc::hir;
 use rustc::hir::def::Def;
@@ -26,6 +24,7 @@ use core::DocContext;
 use fold::DocFolder;
 use html::markdown::markdown_links;
 
+use clean::*;
 use passes::{look_for_tests, Pass};
 
 pub const COLLECT_INTRA_DOC_LINKS: Pass =
@@ -44,13 +43,13 @@ pub fn collect_intra_doc_links(krate: Crate, cx: &DocContext) -> Crate {
 
 #[derive(Debug)]
 enum PathKind {
-    /// can be either value or type, not a macro
+    /// Either a value or type, but not a macro
     Unknown,
-    /// macro
+    /// Macro
     Macro,
-    /// values, functions, consts, statics, everything in the value namespace
+    /// Values, functions, consts, statics (everything in the value namespace)
     Value,
-    /// types, traits, everything in the type namespace
+    /// Types, traits (everything in the type namespace)
     Type,
 }
 
@@ -71,7 +70,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> {
 
     /// Resolve a given string as a path, along with whether or not it is
     /// in the value namespace. Also returns an optional URL fragment in the case
-    /// of variants and methods
+    /// of variants and methods.
     fn resolve(&self,
                path_str: &str,
                is_val: bool,
@@ -82,9 +81,9 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> {
         let cx = self.cx;
 
         // In case we're in a module, try to resolve the relative
-        // path
+        // path.
         if let Some(id) = parent_id.or(self.mod_ids.last().cloned()) {
-            // FIXME: `with_scope` requires the NodeId of a module
+            // FIXME: `with_scope` requires the `NodeId` of a module.
             let result = cx.resolver.borrow_mut()
                                     .with_scope(id,
                 |resolver| {
@@ -94,12 +93,12 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> {
 
             if let Ok(result) = result {
                 // In case this is a trait item, skip the
-                // early return and try looking for the trait
+                // early return and try looking for the trait.
                 let value = match result.def {
                     Def::Method(_) | Def::AssociatedConst(_) => true,
                     Def::AssociatedTy(_) => false,
                     Def::Variant(_) => return handle_variant(cx, result.def),
-                    // not a trait item, just return what we found
+                    // Not a trait item; just return what we found.
                     _ => return Ok((result.def, None))
                 };
 
@@ -111,13 +110,13 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> {
             } else {
                 // If resolution failed, it may still be a method
                 // because methods are not handled by the resolver
-                // If so, bail when we're not looking for a value
+                // If so, bail when we're not looking for a value.
                 if !is_val {
                     return Err(())
                 }
             }
 
-            // Try looking for methods and associated items
+            // Try looking for methods and associated items.
             let mut split = path_str.rsplitn(2, "::");
             let item_name = if let Some(first) = split.next() {
                 first
@@ -137,7 +136,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> {
                 }
             }
 
-            // FIXME: `with_scope` requires the NodeId of a module
+            // FIXME: `with_scope` requires the `NodeId` of a module.
             let ty = cx.resolver.borrow_mut()
                                 .with_scope(id,
                 |resolver| {
@@ -227,10 +226,10 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
             None
         };
 
-        // FIXME: get the resolver to work with non-local resolve scopes
+        // FIXME: get the resolver to work with non-local resolve scopes.
         let parent_node = self.cx.as_local_node_id(item.def_id).and_then(|node_id| {
             // FIXME: this fails hard for impls in non-module scope, but is necessary for the
-            // current resolve() implementation
+            // current `resolve()` implementation.
             match self.cx.tcx.hir().get_module_parent_node(node_id) {
                 id if id != node_id => Some(id),
                 _ => None,
@@ -252,7 +251,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
                 } else {
                     match parent_node.or(self.mod_ids.last().cloned()) {
                         Some(parent) if parent != NodeId::from_u32(0) => {
-                            //FIXME: can we pull the parent module's name from elsewhere?
+                            // FIXME: can we pull the parent module's name from elsewhere?
                             Some(self.cx.tcx.hir().name(parent).to_string())
                         }
                         _ => None,
@@ -262,7 +261,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
             ImplItem(Impl { ref for_, .. }) => {
                 for_.def_id().map(|did| self.cx.tcx.item_name(did).to_string())
             }
-            // we don't display docs on `extern crate` items anyway, so don't process them
+            // we don't display docs on `extern crate` items anyway, so don't process them.
             ExternCrateItem(..) => return self.fold_item_recur(item),
             ImportItem(Import::Simple(ref name, ..)) => Some(name.clone()),
             MacroItem(..) => None,
@@ -283,7 +282,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
         }
 
         for (ori_link, link_range) in markdown_links(&dox) {
-            // bail early for real links
+            // Bail early for real links.
             if ori_link.contains('/') {
                 continue;
             }
@@ -327,9 +326,9 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
                             def
                         } else {
                             resolution_failure(cx, &item.attrs, path_str, &dox, link_range);
-                            // this could just be a normal link or a broken link
+                            // This could just be a normal link or a broken link
                             // we could potentially check if something is
-                            // "intra-doc-link-like" and warn in that case
+                            // "intra-doc-link-like" and warn in that case.
                             continue;
                         }
                     }
@@ -338,12 +337,12 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
                             def
                         } else {
                             resolution_failure(cx, &item.attrs, path_str, &dox, link_range);
-                            // this could just be a normal link
+                            // This could just be a normal link.
                             continue;
                         }
                     }
                     PathKind::Unknown => {
-                        // try everything!
+                        // Try everything!
                         if let Some(macro_def) = macro_resolve(cx, path_str) {
                             if let Ok(type_def) =
                                 self.resolve(path_str, false, &current_item, parent_node)
@@ -371,8 +370,8 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
                         {
                             // It is imperative we search for not-a-value first
                             // Otherwise we will find struct ctors for when we are looking
-                            // for structs, and the link won't work.
-                            // if there is something in both namespaces
+                            // for structs, and the link won't work if there is something in
+                            // both namespaces.
                             if let Ok(value_def) =
                                 self.resolve(path_str, true, &current_item, parent_node)
                             {
@@ -432,7 +431,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
     }
 }
 
-/// Resolve a string as a macro
+/// Resolve a string as a macro.
 fn macro_resolve(cx: &DocContext, path_str: &str) -> Option<Def> {
     use syntax::ext::base::{MacroKind, SyntaxExtension};
     let segment = ast::PathSegment::from_ident(Ident::from_str(path_str));
@@ -482,19 +481,19 @@ fn resolution_failure(
         let mut diag;
         if dox.lines().count() == code_dox.lines().count() {
             let line_offset = dox[..link_range.start].lines().count();
-            // The span starts in the `///`, so we don't have to account for the leading whitespace
+            // The span starts in the `///`, so we don't have to account for the leading whitespace.
             let code_dox_len = if line_offset <= 1 {
                 doc_comment_padding
             } else {
-                // The first `///`
+                // The first `///`.
                 doc_comment_padding +
-                    // Each subsequent leading whitespace and `///`
+                    // Each subsequent leading whitespace and `///`.
                     code_dox.lines().skip(1).take(line_offset - 1).fold(0, |sum, line| {
                         sum + doc_comment_padding + line.len() - line.trim_start().len()
                     })
             };
 
-            // Extract the specific span
+            // Extract the specific span.
             let sp = sp.from_inner_byte_pos(
                 link_range.start + code_dox_len,
                 link_range.end + code_dox_len,
@@ -514,7 +513,7 @@ fn resolution_failure(
             let last_new_line_offset = dox[..link_range.start].rfind('\n').map_or(0, |n| n + 1);
             let line = dox[last_new_line_offset..].lines().next().unwrap_or("");
 
-            // Print the line containing the `link_range` and manually mark it with '^'s
+            // Print the line containing the `link_range` and manually mark it with '^'s.
             diag.note(&format!(
                 "the link appears in this line:\n\n{line}\n\
                  {indicator: <before$}{indicator:^<found$}",
@@ -555,13 +554,13 @@ fn ambiguity_error(cx: &DocContext, attrs: &Attributes,
 }
 
 /// Given a def, returns its name and disambiguator
-/// for a value namespace
+/// for a value namespace.
 ///
-/// Returns None for things which cannot be ambiguous since
-/// they exist in both namespaces (structs and modules)
+/// Returns `None` for things which cannot be ambiguous since
+/// they exist in both namespaces (structs and modules).
 fn value_ns_kind(def: Def, path_str: &str) -> Option<(&'static str, String)> {
     match def {
-        // structs, variants, and mods exist in both namespaces. skip them
+        // Structs, variants, and mods exist in both namespaces; skip them.
         Def::StructCtor(..) | Def::Mod(..) | Def::Variant(..) |
         Def::VariantCtor(..) | Def::SelfCtor(..)
             => None,
@@ -578,10 +577,10 @@ fn value_ns_kind(def: Def, path_str: &str) -> Option<(&'static str, String)> {
 }
 
 /// Given a def, returns its name, the article to be used, and a disambiguator
-/// for the type namespace
+/// for the type namespace.
 fn type_ns_kind(def: Def, path_str: &str) -> (&'static str, &'static str, String) {
     let (kind, article) = match def {
-        // we can still have non-tuple structs
+        // We can still have non-tuple structs.
         Def::Struct(..) => ("struct", "a"),
         Def::Enum(..) => ("enum", "an"),
         Def::Trait(..) => ("trait", "a"),
@@ -591,7 +590,7 @@ fn type_ns_kind(def: Def, path_str: &str) -> (&'static str, &'static str, String
     (kind, article, format!("{}@{}", kind, path_str))
 }
 
-/// Given an enum variant's def, return the def of its enum and the associated fragment
+/// Given an enum variant's def, return the def of its enum and the associated fragment.
 fn handle_variant(cx: &DocContext, def: Def) -> Result<(Def, Option<String>), ()> {
     use rustc::ty::DefIdTree;
 
diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs
index 74583196818..be9327ced26 100644
--- a/src/librustdoc/test.rs
+++ b/src/librustdoc/test.rs
@@ -8,38 +8,38 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::env;
-use std::ffi::OsString;
-use std::io::prelude::*;
-use std::io;
-use std::path::PathBuf;
-use std::panic::{self, AssertUnwindSafe};
-use std::process::Command;
-use std::str;
+use errors;
+use errors::emitter::ColorConfig;
 use rustc_data_structures::sync::Lrc;
-use std::sync::{Arc, Mutex};
-
-use testing;
 use rustc_lint;
+use rustc_driver::{self, driver, target_features, Compilation};
+use rustc_driver::driver::phase_2_configure_and_expand;
+use rustc_metadata::cstore::CStore;
+use rustc_metadata::dynamic_lib::DynamicLibrary;
+use rustc_resolve::MakeGlobMap;
 use rustc::hir;
 use rustc::hir::intravisit;
 use rustc::session::{self, CompileIncomplete, config};
 use rustc::session::config::{OutputType, OutputTypes, Externs, CodegenOptions};
 use rustc::session::search_paths::{SearchPaths, PathKind};
-use rustc_metadata::dynamic_lib::DynamicLibrary;
-use tempfile::Builder as TempFileBuilder;
-use rustc_driver::{self, driver, target_features, Compilation};
-use rustc_driver::driver::phase_2_configure_and_expand;
-use rustc_metadata::cstore::CStore;
-use rustc_resolve::MakeGlobMap;
 use syntax::ast;
 use syntax::source_map::SourceMap;
 use syntax::edition::Edition;
 use syntax::feature_gate::UnstableFeatures;
 use syntax::with_globals;
 use syntax_pos::{BytePos, DUMMY_SP, Pos, Span, FileName};
-use errors;
-use errors::emitter::ColorConfig;
+use tempfile::Builder as TempFileBuilder;
+use testing;
+
+use std::env;
+use std::ffi::OsString;
+use std::io::prelude::*;
+use std::io;
+use std::path::PathBuf;
+use std::panic::{self, AssertUnwindSafe};
+use std::process::Command;
+use std::str;
+use std::sync::{Arc, Mutex};
 
 use clean::Attributes;
 use config::Options;
@@ -153,7 +153,7 @@ pub fn run(mut options: Options) -> isize {
     })
 }
 
-// Look for #![doc(test(no_crate_inject))], used by crates in the std facade
+// Look for `#![doc(test(no_crate_inject))]`, used by crates in the std facade.
 fn scrape_test_config(krate: &::rustc::hir::Crate) -> TestOptions {
     use syntax::print::pprust;
 
@@ -192,12 +192,11 @@ fn run_test(test: &str, cratename: &str, filename: &FileName, line: usize,
             should_panic: bool, no_run: bool, as_test_harness: bool,
             compile_fail: bool, mut error_codes: Vec<String>, opts: &TestOptions,
             maybe_sysroot: Option<PathBuf>, linker: Option<PathBuf>, edition: Edition) {
-    // the test harness wants its own `main` & top level functions, so
-    // never wrap the test in `fn main() { ... }`
+    // The test harness wants its own `main` and top-level functions, so
+    // never wrap the test in `fn main() { ... }`.
     let (test, line_offset) = make_test(test, Some(cratename), as_test_harness, opts);
     // FIXME(#44940): if doctests ever support path remapping, then this filename
-    // needs to be the result of SourceMap::span_to_unmapped_path
-
+    // needs to be the result of `SourceMap::span_to_unmapped_path`.
     let path = match filename {
         FileName::Real(path) => path.clone(),
         _ => PathBuf::from(r"doctest.rs"),
@@ -408,8 +407,8 @@ pub fn make_test(s: &str,
         let filename = FileName::anon_source_code(s);
         let source = crates + &everything_else;
 
-        // any errors in parsing should also appear when the doctest is compiled for real, so just
-        // send all the errors that libsyntax emits directly into a Sink instead of stderr
+        // Any errors in parsing should also appear when the doctest is compiled for real, so just
+        // send all the errors that libsyntax emits directly into a `Sink` instead of stderr.
         let cm = Lrc::new(SourceMap::new(FilePathMapping::empty()));
         let emitter = EmitterWriter::new(box io::sink(), None, false, false);
         let handler = Handler::with_emitter(false, false, box emitter);
@@ -537,10 +536,10 @@ pub struct Collector {
     // The name of the test displayed to the user, separated by `::`.
     //
     // In tests from Rust source, this is the path to the item
-    // e.g. `["std", "vec", "Vec", "push"]`.
+    // e.g., `["std", "vec", "Vec", "push"]`.
     //
     // In tests from a markdown file, this is the titles of all headers (h1~h6)
-    // of the sections that contain the code block, e.g. if the markdown file is
+    // of the sections that contain the code block, e.g., if the markdown file is
     // written as:
     //
     // ``````markdown
@@ -689,7 +688,7 @@ impl Tester for Collector {
 
     fn register_header(&mut self, name: &str, level: u32) {
         if self.use_headers {
-            // we use these headings as test names, so it's good if
+            // We use these headings as test names, so it's good if
             // they're valid identifiers.
             let name = name.chars().enumerate().map(|(i, c)| {
                     if (i == 0 && c.is_xid_start()) ||
@@ -703,7 +702,7 @@ impl Tester for Collector {
             // Here we try to efficiently assemble the header titles into the
             // test name in the form of `h1::h2::h3::h4::h5::h6`.
             //
-            // Suppose originally `self.names` contains `[h1, h2, h3]`...
+            // Suppose that originally `self.names` contains `[h1, h2, h3]`...
             let level = level as usize;
             if level <= self.names.len() {
                 // ... Consider `level == 2`. All headers in the lower levels
@@ -752,8 +751,8 @@ impl<'a, 'hir> HirCollector<'a, 'hir> {
 
         attrs.collapse_doc_comments();
         attrs.unindent_doc_comments();
-        // the collapse-docs pass won't combine sugared/raw doc attributes, or included files with
-        // anything else, this will combine them for us
+        // The collapse-docs pass won't combine sugared/raw doc attributes, or included files with
+        // anything else, this will combine them for us.
         if let Some(doc) = attrs.collapsed_doc_value() {
             self.collector.set_position(attrs.span.unwrap_or(DUMMY_SP));
             let res = markdown::find_testable_code(&doc, self.collector, self.codes);
@@ -847,8 +846,8 @@ assert_eq!(2+2, 4);
 
     #[test]
     fn make_test_crate_name_no_use() {
-        //if you give a crate name but *don't* use it within the test, it won't bother inserting
-        //the `extern crate` statement
+        // If you give a crate name but *don't* use it within the test, it won't bother inserting
+        // the `extern crate` statement.
         let opts = TestOptions::default();
         let input =
 "assert_eq!(2+2, 4);";
@@ -863,8 +862,8 @@ assert_eq!(2+2, 4);
 
     #[test]
     fn make_test_crate_name() {
-        //if you give a crate name and use it within the test, it will insert an `extern crate`
-        //statement before `fn main`
+        // If you give a crate name and use it within the test, it will insert an `extern crate`
+        // statement before `fn main`.
         let opts = TestOptions::default();
         let input =
 "use asdf::qwop;
@@ -882,8 +881,8 @@ assert_eq!(2+2, 4);
 
     #[test]
     fn make_test_no_crate_inject() {
-        //even if you do use the crate within the test, setting `opts.no_crate_inject` will skip
-        //adding it anyway
+        // Even if you do use the crate within the test, setting `opts.no_crate_inject` will skip
+        // adding it anyway.
         let opts = TestOptions {
             no_crate_inject: true,
             display_warnings: false,
@@ -904,8 +903,9 @@ assert_eq!(2+2, 4);
 
     #[test]
     fn make_test_ignore_std() {
-        //even if you include a crate name, and use it in the doctest, we still won't include an
-        //`extern crate` statement if the crate is "std" - that's included already by the compiler!
+        // Even if you include a crate name, and use it in the doctest, we still won't include an
+        // `extern crate` statement if the crate is "std" -- that's included already by the
+        // compiler!
         let opts = TestOptions::default();
         let input =
 "use std::*;
@@ -922,8 +922,8 @@ assert_eq!(2+2, 4);
 
     #[test]
     fn make_test_manual_extern_crate() {
-        //when you manually include an `extern crate` statement in your doctest, make_test assumes
-        //you've included one for your own crate too
+        // When you manually include an `extern crate` statement in your doctest, `make_test`
+        // assumes you've included one for your own crate too.
         let opts = TestOptions::default();
         let input =
 "extern crate asdf;
@@ -960,8 +960,8 @@ assert_eq!(2+2, 4);
 
     #[test]
     fn make_test_opts_attrs() {
-        //if you supplied some doctest attributes with #![doc(test(attr(...)))], it will use those
-        //instead of the stock #![allow(unused)]
+        // If you supplied some doctest attributes with `#![doc(test(attr(...)))]`, it will use
+        // those instead of the stock `#![allow(unused)]`.
         let mut opts = TestOptions::default();
         opts.attrs.push("feature(sick_rad)".to_string());
         let input =
@@ -977,7 +977,7 @@ assert_eq!(2+2, 4);
         let output = make_test(input, Some("asdf"), false, &opts);
         assert_eq!(output, (expected, 3));
 
-        //adding more will also bump the returned line offset
+        // Adding more will also bump the returned line offset.
         opts.attrs.push("feature(hella_dope)".to_string());
         let expected =
 "#![feature(sick_rad)]
@@ -993,8 +993,8 @@ assert_eq!(2+2, 4);
 
     #[test]
     fn make_test_crate_attrs() {
-        //including inner attributes in your doctest will apply them to the whole "crate", pasting
-        //them outside the generated main function
+        // Including inner attributes in your doctest will apply them to the whole "crate", pasting
+        // them outside the generated main function.
         let opts = TestOptions::default();
         let input =
 "#![feature(sick_rad)]
@@ -1011,7 +1011,7 @@ assert_eq!(2+2, 4);
 
     #[test]
     fn make_test_with_main() {
-        //including your own `fn main` wrapper lets the test use it verbatim
+        // Including your own `fn main` wrapper lets the test use it verbatim.
         let opts = TestOptions::default();
         let input =
 "fn main() {
@@ -1028,7 +1028,7 @@ fn main() {
 
     #[test]
     fn make_test_fake_main() {
-        //...but putting it in a comment will still provide a wrapper
+        // ... but putting it in a comment will still provide a wrapper.
         let opts = TestOptions::default();
         let input =
 "//Ceci n'est pas une `fn main`
@@ -1045,7 +1045,7 @@ assert_eq!(2+2, 4);
 
     #[test]
     fn make_test_dont_insert_main() {
-        //even with that, if you set `dont_insert_main`, it won't create the `fn main` wrapper
+        // Even with that, if you set `dont_insert_main`, it won't create the `fn main` wrapper.
         let opts = TestOptions::default();
         let input =
 "//Ceci n'est pas une `fn main`
@@ -1060,7 +1060,7 @@ assert_eq!(2+2, 4);".to_string();
 
     #[test]
     fn make_test_display_warnings() {
-        //if the user is asking to display doctest warnings, suppress the default allow(unused)
+        // If the user is asking to display doctest warnings, suppress the default `allow(unused)`.
         let mut opts = TestOptions::default();
         opts.display_warnings = true;
         let input =
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
index fc7e8d72d69..004be1cfe39 100644
--- a/src/librustdoc/visit_ast.rs
+++ b/src/librustdoc/visit_ast.rs
@@ -8,36 +8,33 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-//! Rust AST Visitor. Extracts useful information and massages it into a form
-//! usable for clean
-
-use std::mem;
+//! The Rust AST Visitor. Extracts useful information and massages it into a form
+//! usable for `clean`.
 
+use rustc::hir::{self, Node};
+use rustc::hir::def::Def;
+use rustc::hir::def_id::{DefId, LOCAL_CRATE};
+use rustc::middle::privacy::AccessLevel;
+use rustc::util::nodemap::{FxHashSet, FxHashMap};
 use syntax::ast;
 use syntax::attr;
 use syntax::ext::base::MacroKind;
 use syntax::source_map::Spanned;
 use syntax_pos::{self, Span};
 
-use rustc::hir::Node;
-use rustc::hir::def::Def;
-use rustc::hir::def_id::{DefId, LOCAL_CRATE};
-use rustc::middle::privacy::AccessLevel;
-use rustc::util::nodemap::{FxHashSet, FxHashMap};
-
-use rustc::hir;
+use std::mem;
 
 use core;
 use clean::{self, AttributesExt, NestedAttributesExt, def_id_to_path};
 use doctree::*;
 
-// looks to me like the first two of these are actually
+// Looks to me like the first two of these are actually
 // output parameters, maybe only mutated once; perhaps
 // better simply to have the visit method return a tuple
 // containing them?
 
-// also, is there some reason that this doesn't use the 'visit'
-// framework from syntax?
+// Also, is there some reason that this doesn't use the 'visit'
+// framework from syntax?.
 
 pub struct RustdocVisitor<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> {
     pub module: Module,
@@ -45,7 +42,7 @@ pub struct RustdocVisitor<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> {
     pub cx: &'a core::DocContext<'a, 'tcx, 'rcx, 'cstore>,
     view_item_stack: FxHashSet<ast::NodeId>,
     inlining: bool,
-    /// Is the current module and all of its parents public?
+    /// Are the current module and all of its parents public?
     inside_public_path: bool,
     exact_paths: Option<FxHashMap<DefId, Vec<String>>>,
 }
@@ -69,8 +66,8 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> {
     }
 
     fn store_path(&mut self, did: DefId) {
-        // We can't use the entry api, as that keeps the mutable borrow of self active
-        // when we try to use cx
+        // We can't use the entry API, as that keeps the mutable borrow of `self` active
+        // when we try to use `cx`.
         let exact_paths = self.exact_paths.as_mut().unwrap();
         if exact_paths.get(&did).is_none() {
             let path = def_id_to_path(self.cx, did, self.cx.crate_name.clone());
@@ -98,7 +95,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> {
                                               ast::CRATE_NODE_ID,
                                               &krate.module,
                                               None);
-        // attach the crate's exported macros to the top-level module:
+        // Attach the crate's exported macros to the top-level module:
         let macro_exports: Vec<_> =
             krate.exported_macros.iter().map(|def| self.visit_local_macro(def, None)).collect();
         self.module.macros.extend(macro_exports);
@@ -303,14 +300,14 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> {
         let def_did = def.def_id();
 
         let use_attrs = tcx.hir().attrs(id);
-        // Don't inline doc(hidden) imports so they can be stripped at a later stage.
+        // Don't inline `doc(hidden)` imports so they can be stripped at a later stage.
         let is_no_inline = use_attrs.lists("doc").has_word("no_inline") ||
                            use_attrs.lists("doc").has_word("hidden");
 
         // For cross-crate impl inlining we need to know whether items are
-        // reachable in documentation - a previously nonreachable item can be
+        // reachable in documentation -- a previously nonreachable item can be
         // made reachable by cross-crate inlining which we're checking here.
-        // (this is done here because we need to know this upfront)
+        // (this is done here because we need to know this upfront).
         if !def_did.is_local() && !is_no_inline {
             let attrs = clean::inline::load_attrs(self.cx, def_did);
             let self_is_hidden = attrs.lists("doc").has_word("hidden");
@@ -342,7 +339,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> {
         let is_private = !self.cx.renderinfo.borrow().access_levels.is_public(def_did);
         let is_hidden = inherits_doc_hidden(self.cx, def_node_id);
 
-        // Only inline if requested or if the item would otherwise be stripped
+        // Only inline if requested or if the item would otherwise be stripped.
         if (!please_inline && !is_private && !is_hidden) || is_no_inline {
             return false
         }
@@ -366,7 +363,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> {
                 true
             }
             Node::ForeignItem(it) if !glob => {
-                // generate a fresh `extern {}` block if we want to inline a foreign item.
+                // Generate a fresh `extern {}` block if we want to inline a foreign item.
                 om.foreigns.push(hir::ForeignMod {
                     abi: tcx.hir().get_foreign_abi(it.id),
                     items: vec![hir::ForeignItem {
@@ -427,7 +424,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> {
             hir::ItemKind::Use(ref path, kind) => {
                 let is_glob = kind == hir::UseKind::Glob;
 
-                // struct and variant constructors always show up alongside their definitions, we've
+                // Struct and variant constructors always show up alongside their definitions, we've
                 // already processed them so just discard these.
                 match path.def {
                     Def::StructCtor(..) | Def::VariantCtor(..) | Def::SelfCtor(..) => return,
@@ -596,7 +593,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> RustdocVisitor<'a, 'tcx, 'rcx, 'cstore> {
         }
     }
 
-    // convert each exported_macro into a doc item
+    // Convert each `exported_macro` into a doc item.
     fn visit_local_macro(
         &self,
         def: &hir::MacroDef,
diff --git a/src/librustdoc/visit_lib.rs b/src/librustdoc/visit_lib.rs
index 5d1f42c83f4..f64334a8219 100644
--- a/src/librustdoc/visit_lib.rs
+++ b/src/librustdoc/visit_lib.rs
@@ -21,7 +21,7 @@ use clean::{AttributesExt, NestedAttributesExt};
 // FIXME: this may not be exhaustive, but is sufficient for rustdocs current uses
 
 /// Similar to `librustc_privacy::EmbargoVisitor`, but also takes
-/// specific rustdoc annotations into account (i.e. `doc(hidden)`)
+/// specific rustdoc annotations into account (i.e., `doc(hidden)`)
 pub struct LibEmbargoVisitor<'a, 'tcx: 'a, 'rcx: 'a, 'cstore: 'rcx> {
     cx: &'a ::core::DocContext<'a, 'tcx, 'rcx, 'cstore>,
     // Accessibility levels for reachable nodes