Rollup merge of #42409 - bjorn3:patch-3, r=frewsxcv

Better docs

Working on more doc improvements

Edit: done for today

5 files changed, 118 insertions, 110 deletions

diff --git a/src/librustc/hir/def.rs b/src/librustc/hir/def.rs
index feded417ce1..c500d770cef 100644
--- a/src/librustc/hir/def.rs
+++ b/src/librustc/hir/def.rs
@@ -17,11 +17,11 @@ use hir;
 
 #[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
 pub enum CtorKind {
-    // Constructor function automatically created by a tuple struct/variant.
+    /// Constructor function automatically created by a tuple struct/variant.
     Fn,
-    // Constructor constant automatically created by a unit struct/variant.
+    /// Constructor constant automatically created by a unit struct/variant.
     Const,
-    // Unusable name in value namespace created by a struct variant.
+    /// Unusable name in value namespace created by a struct variant.
     Fictive,
 }
 
@@ -109,17 +109,21 @@ impl PathResolution {
     }
 }
 
-// Definition mapping
+/// Definition mapping
 pub type DefMap = NodeMap<PathResolution>;
-// This is the replacement export map. It maps a module to all of the exports
-// within.
+
+/// This is the replacement export map. It maps a module to all of the exports
+/// within.
 pub type ExportMap = NodeMap<Vec<Export>>;
 
 #[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable)]
 pub struct Export {
-    pub ident: ast::Ident, // The name of the target.
-    pub def: Def, // The definition of the target.
-    pub span: Span, // The span of the target definition.
+    /// The name of the target.
+    pub ident: ast::Ident,
+    /// The definition of the target.
+    pub def: Def,
+    /// The span of the target definition.
+    pub span: Span,
 }
 
 impl CtorKind {
@@ -160,6 +164,7 @@ impl Def {
         }
     }
 
+    /// A human readable kind name
     pub fn kind_name(&self) -> &'static str {
         match *self {
             Def::Fn(..) => "function",
diff --git a/src/librustc/hir/def_id.rs b/src/librustc/hir/def_id.rs
index 47604b961ae..ce2baa73897 100644
--- a/src/librustc/hir/def_id.rs
+++ b/src/librustc/hir/def_id.rs
@@ -187,6 +187,7 @@ impl fmt::Debug for DefId {
 
 
 impl DefId {
+    /// Make a local `DefId` with the given index.
     pub fn local(index: DefIndex) -> DefId {
         DefId { krate: LOCAL_CRATE, index: index }
     }
diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
index 4c436fb640f..a6ab67e0469 100644
--- a/src/librustc/hir/lowering.rs
+++ b/src/librustc/hir/lowering.rs
@@ -8,37 +8,37 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// Lowers the AST to the HIR.
-//
-// Since the AST and HIR are fairly similar, this is mostly a simple procedure,
-// much like a fold. Where lowering involves a bit more work things get more
-// interesting and there are some invariants you should know about. These mostly
-// concern spans and ids.
-//
-// Spans are assigned to AST nodes during parsing and then are modified during
-// expansion to indicate the origin of a node and the process it went through
-// being expanded. Ids are assigned to AST nodes just before lowering.
-//
-// For the simpler lowering steps, ids and spans should be preserved. Unlike
-// expansion we do not preserve the process of lowering in the spans, so spans
-// should not be modified here. When creating a new node (as opposed to
-// 'folding' an existing one), then you create a new id using `next_id()`.
-//
-// You must ensure that ids are unique. That means that you should only use the
-// id from an AST node in a single HIR node (you can assume that AST node ids
-// are unique). Every new node must have a unique id. Avoid cloning HIR nodes.
-// If you do, you must then set the new node's id to a fresh one.
-//
-// Spans are used for error messages and for tools to map semantics back to
-// source code. It is therefore not as important with spans as ids to be strict
-// about use (you can't break the compiler by screwing up a span). Obviously, a
-// HIR node can only have a single span. But multiple nodes can have the same
-// span and spans don't need to be kept in order, etc. Where code is preserved
-// by lowering, it should have the same span as in the AST. Where HIR nodes are
-// new it is probably best to give a span for the whole AST node being lowered.
-// All nodes should have real spans, don't use dummy spans. Tools are likely to
-// get confused if the spans from leaf AST nodes occur in multiple places
-// in the HIR, especially for multiple identifiers.
+//! Lowers the AST to the HIR.
+//!
+//! Since the AST and HIR are fairly similar, this is mostly a simple procedure,
+//! much like a fold. Where lowering involves a bit more work things get more
+//! interesting and there are some invariants you should know about. These mostly
+//! concern spans and ids.
+//!
+//! Spans are assigned to AST nodes during parsing and then are modified during
+//! expansion to indicate the origin of a node and the process it went through
+//! being expanded. Ids are assigned to AST nodes just before lowering.
+//!
+//! For the simpler lowering steps, ids and spans should be preserved. Unlike
+//! expansion we do not preserve the process of lowering in the spans, so spans
+//! should not be modified here. When creating a new node (as opposed to
+//! 'folding' an existing one), then you create a new id using `next_id()`.
+//!
+//! You must ensure that ids are unique. That means that you should only use the
+//! id from an AST node in a single HIR node (you can assume that AST node ids
+//! are unique). Every new node must have a unique id. Avoid cloning HIR nodes.
+//! If you do, you must then set the new node's id to a fresh one.
+//!
+//! Spans are used for error messages and for tools to map semantics back to
+//! source code. It is therefore not as important with spans as ids to be strict
+//! about use (you can't break the compiler by screwing up a span). Obviously, a
+//! HIR node can only have a single span. But multiple nodes can have the same
+//! span and spans don't need to be kept in order, etc. Where code is preserved
+//! by lowering, it should have the same span as in the AST. Where HIR nodes are
+//! new it is probably best to give a span for the whole AST node being lowered.
+//! All nodes should have real spans, don't use dummy spans. Tools are likely to
+//! get confused if the spans from leaf AST nodes occur in multiple places
+//! in the HIR, especially for multiple identifiers.
 
 use hir;
 use hir::map::{Definitions, DefKey, REGULAR_SPACE};
@@ -70,8 +70,10 @@ const HIR_ID_COUNTER_LOCKED: u32 = 0xFFFFFFFF;
 
 pub struct LoweringContext<'a> {
     crate_root: Option<&'static str>,
+
     // Use to assign ids to hir nodes that do not directly correspond to an ast node
     sess: &'a Session,
+
     // As we walk the AST we must keep track of the current 'parent' def id (in
     // the form of a DefIndex) so that if we create a new node which introduces
     // a definition, then we can properly create the def id.
@@ -102,14 +104,14 @@ pub struct LoweringContext<'a> {
 }
 
 pub trait Resolver {
-    // Resolve a hir path generated by the lowerer when expanding `for`, `if let`, etc.
+    /// Resolve a hir path generated by the lowerer when expanding `for`, `if let`, etc.
     fn resolve_hir_path(&mut self, path: &mut hir::Path, is_value: bool);
 
-    // Obtain the resolution for a node id
+    /// Obtain the resolution for a node id
     fn get_resolution(&mut self, id: NodeId) -> Option<PathResolution>;
 
-    // We must keep the set of definitions up to date as we add nodes that weren't in the AST.
-    // This should only return `None` during testing.
+    /// We must keep the set of definitions up to date as we add nodes that weren't in the AST.
+    /// This should only return `None` during testing.
     fn definitions(&mut self) -> &mut Definitions;
 }
 
diff --git a/src/librustc/infer/region_inference/graphviz.rs b/src/librustc/infer/region_inference/graphviz.rs
index cce253c1a1a..81a8984e753 100644
--- a/src/librustc/infer/region_inference/graphviz.rs
+++ b/src/librustc/infer/region_inference/graphviz.rs
@@ -133,7 +133,6 @@ enum Node {
     Region(ty::RegionKind),
 }
 
-// type Edge = Constraint;
 #[derive(Clone, PartialEq, Eq, Debug, Copy)]
 enum Edge<'tcx> {
     Constraint(Constraint<'tcx>),
diff --git a/src/librustc/infer/region_inference/mod.rs b/src/librustc/infer/region_inference/mod.rs
index acc1a397b45..f62470fab72 100644
--- a/src/librustc/infer/region_inference/mod.rs
+++ b/src/librustc/infer/region_inference/mod.rs
@@ -35,31 +35,31 @@ use std::u32;
 
 mod graphviz;
 
-// A constraint that influences the inference process.
+/// A constraint that influences the inference process.
 #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
 pub enum Constraint<'tcx> {
-    // One region variable is subregion of another
+    /// One region variable is subregion of another
     ConstrainVarSubVar(RegionVid, RegionVid),
 
-    // Concrete region is subregion of region variable
+    /// Concrete region is subregion of region variable
     ConstrainRegSubVar(Region<'tcx>, RegionVid),
 
-    // Region variable is subregion of concrete region. This does not
-    // directly affect inference, but instead is checked after
-    // inference is complete.
+    /// Region variable is subregion of concrete region. This does not
+    /// directly affect inference, but instead is checked after
+    /// inference is complete.
     ConstrainVarSubReg(RegionVid, Region<'tcx>),
 
-    // A constraint where neither side is a variable. This does not
-    // directly affect inference, but instead is checked after
-    // inference is complete.
+    /// A constraint where neither side is a variable. This does not
+    /// directly affect inference, but instead is checked after
+    /// inference is complete.
     ConstrainRegSubReg(Region<'tcx>, Region<'tcx>),
 }
 
-// VerifyGenericBound(T, _, R, RS): The parameter type `T` (or
-// associated type) must outlive the region `R`. `T` is known to
-// outlive `RS`. Therefore verify that `R <= RS[i]` for some
-// `i`. Inference variables may be involved (but this verification
-// step doesn't influence inference).
+/// VerifyGenericBound(T, _, R, RS): The parameter type `T` (or
+/// associated type) must outlive the region `R`. `T` is known to
+/// outlive `RS`. Therefore verify that `R <= RS[i]` for some
+/// `i`. Inference variables may be involved (but this verification
+/// step doesn't influence inference).
 #[derive(Debug)]
 pub struct Verify<'tcx> {
     kind: GenericKind<'tcx>,
@@ -74,29 +74,29 @@ pub enum GenericKind<'tcx> {
     Projection(ty::ProjectionTy<'tcx>),
 }
 
-// When we introduce a verification step, we wish to test that a
-// particular region (let's call it `'min`) meets some bound.
-// The bound is described the by the following grammar:
+/// When we introduce a verification step, we wish to test that a
+/// particular region (let's call it `'min`) meets some bound.
+/// The bound is described the by the following grammar:
 #[derive(Debug)]
 pub enum VerifyBound<'tcx> {
-    // B = exists {R} --> some 'r in {R} must outlive 'min
-    //
-    // Put another way, the subject value is known to outlive all
-    // regions in {R}, so if any of those outlives 'min, then the
-    // bound is met.
+    /// B = exists {R} --> some 'r in {R} must outlive 'min
+    ///
+    /// Put another way, the subject value is known to outlive all
+    /// regions in {R}, so if any of those outlives 'min, then the
+    /// bound is met.
     AnyRegion(Vec<Region<'tcx>>),
 
-    // B = forall {R} --> all 'r in {R} must outlive 'min
-    //
-    // Put another way, the subject value is known to outlive some
-    // region in {R}, so if all of those outlives 'min, then the bound
-    // is met.
+    /// B = forall {R} --> all 'r in {R} must outlive 'min
+    ///
+    /// Put another way, the subject value is known to outlive some
+    /// region in {R}, so if all of those outlives 'min, then the bound
+    /// is met.
     AllRegions(Vec<Region<'tcx>>),
 
-    // B = exists {B} --> 'min must meet some bound b in {B}
+    /// B = exists {B} --> 'min must meet some bound b in {B}
     AnyBound(Vec<VerifyBound<'tcx>>),
 
-    // B = forall {B} --> 'min must meet all bounds b in {B}
+    /// B = forall {B} --> 'min must meet all bounds b in {B}
     AllBounds(Vec<VerifyBound<'tcx>>),
 }
 
@@ -183,35 +183,35 @@ pub struct RegionVarBindings<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     tcx: TyCtxt<'a, 'gcx, 'tcx>,
     var_origins: RefCell<Vec<RegionVariableOrigin>>,
 
-    // Constraints of the form `A <= B` introduced by the region
-    // checker.  Here at least one of `A` and `B` must be a region
-    // variable.
+    /// Constraints of the form `A <= B` introduced by the region
+    /// checker.  Here at least one of `A` and `B` must be a region
+    /// variable.
     constraints: RefCell<FxHashMap<Constraint<'tcx>, SubregionOrigin<'tcx>>>,
 
-    // A "verify" is something that we need to verify after inference is
-    // done, but which does not directly affect inference in any way.
-    //
-    // An example is a `A <= B` where neither `A` nor `B` are
-    // inference variables.
+    /// A "verify" is something that we need to verify after inference is
+    /// done, but which does not directly affect inference in any way.
+    ///
+    /// An example is a `A <= B` where neither `A` nor `B` are
+    /// inference variables.
     verifys: RefCell<Vec<Verify<'tcx>>>,
 
-    // A "given" is a relationship that is known to hold. In particular,
-    // we often know from closure fn signatures that a particular free
-    // region must be a subregion of a region variable:
-    //
-    //    foo.iter().filter(<'a> |x: &'a &'b T| ...)
-    //
-    // In situations like this, `'b` is in fact a region variable
-    // introduced by the call to `iter()`, and `'a` is a bound region
-    // on the closure (as indicated by the `<'a>` prefix). If we are
-    // naive, we wind up inferring that `'b` must be `'static`,
-    // because we require that it be greater than `'a` and we do not
-    // know what `'a` is precisely.
-    //
-    // This hashmap is used to avoid that naive scenario. Basically we
-    // record the fact that `'a <= 'b` is implied by the fn signature,
-    // and then ignore the constraint when solving equations. This is
-    // a bit of a hack but seems to work.
+    /// A "given" is a relationship that is known to hold. In particular,
+    /// we often know from closure fn signatures that a particular free
+    /// region must be a subregion of a region variable:
+    ///
+    ///    foo.iter().filter(<'a> |x: &'a &'b T| ...)
+    ///
+    /// In situations like this, `'b` is in fact a region variable
+    /// introduced by the call to `iter()`, and `'a` is a bound region
+    /// on the closure (as indicated by the `<'a>` prefix). If we are
+    /// naive, we wind up inferring that `'b` must be `'static`,
+    /// because we require that it be greater than `'a` and we do not
+    /// know what `'a` is precisely.
+    ///
+    /// This hashmap is used to avoid that naive scenario. Basically we
+    /// record the fact that `'a <= 'b` is implied by the fn signature,
+    /// and then ignore the constraint when solving equations. This is
+    /// a bit of a hack but seems to work.
     givens: RefCell<FxHashSet<(Region<'tcx>, ty::RegionVid)>>,
 
     lubs: RefCell<CombineMap<'tcx>>,
@@ -219,20 +219,21 @@ pub struct RegionVarBindings<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     skolemization_count: Cell<u32>,
     bound_count: Cell<u32>,
 
-    // The undo log records actions that might later be undone.
-    //
-    // Note: when the undo_log is empty, we are not actively
-    // snapshotting. When the `start_snapshot()` method is called, we
-    // push an OpenSnapshot entry onto the list to indicate that we
-    // are now actively snapshotting. The reason for this is that
-    // otherwise we end up adding entries for things like the lower
-    // bound on a variable and so forth, which can never be rolled
-    // back.
+    /// The undo log records actions that might later be undone.
+    ///
+    /// Note: when the undo_log is empty, we are not actively
+    /// snapshotting. When the `start_snapshot()` method is called, we
+    /// push an OpenSnapshot entry onto the list to indicate that we
+    /// are now actively snapshotting. The reason for this is that
+    /// otherwise we end up adding entries for things like the lower
+    /// bound on a variable and so forth, which can never be rolled
+    /// back.
     undo_log: RefCell<Vec<UndoLogEntry<'tcx>>>,
+
     unification_table: RefCell<UnificationTable<ty::RegionVid>>,
 
-    // This contains the results of inference.  It begins as an empty
-    // option and only acquires a value after inference is complete.
+    /// This contains the results of inference.  It begins as an empty
+    /// option and only acquires a value after inference is complete.
     values: RefCell<Option<Vec<VarValue<'tcx>>>>,
 }