Apply suggestions from code review

Co-Authored-By: Mazdak Farrokhzad <twingoow@gmail.com>

src/librustc/hir/lowering.rs

@@ -126,7 +126,7 @@ pub struct LoweringContext<'a> {
     /// lifetime definitions in the corresponding impl or function generics.
     lifetimes_to_define: Vec<(Span, ParamName)>,
 
-    /// `true` ifs in-band lifetimes are being collected. This is used to
+    /// `true` if in-band lifetimes are being collected. This is used to
     /// indicate whether or not we're in a place where new lifetimes will result
     /// in in-band lifetime definitions, such a function or an impl header,
     /// including implicit lifetimes from `impl_header_lifetime_elision`.

src/librustc/mir/interpret/allocation.rs

@@ -4,13 +4,14 @@ use super::{
     Pointer, InterpResult, AllocId, ScalarMaybeUndef, write_target_uint, read_target_uint, Scalar,
 };
 
-use crate::ty::layout::{Size, Align};
-use syntax::ast::Mutability;
-use std::iter;
 use crate::mir;
-use std::ops::{Range, Deref, DerefMut};
+use crate::ty::layout::{Size, Align};
+
 use rustc_data_structures::sorted_map::SortedMap;
 use rustc_target::abi::HasDataLayout;
+use syntax::ast::Mutability;
+use std::iter;
+use std::ops::{Range, Deref, DerefMut};
 use std::borrow::Cow;
 
 // NOTE: When adding new fields, make sure to adjust the `Snapshot` impl in
@@ -765,7 +766,7 @@ impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
         }
     }
 
-    /// Apply a relocation copy.
+    /// Applies a relocation copy.
     /// The affected range, as defined in the parameters to `prepare_relocation_copy` is expected
     /// to be clear of relocations.
     pub fn mark_relocation_range(

src/librustc/mir/interpret/mod.rs

@@ -349,7 +349,6 @@ pub enum GlobalAlloc<'tcx> {
     Memory(&'tcx Allocation),
 }
 
-#[derive(Clone)]
 pub struct AllocMap<'tcx> {
     /// Maps `AllocId`s to their corresponding allocations.
     alloc_map: FxHashMap<AllocId, GlobalAlloc<'tcx>>,

src/librustc/mir/mod.rs

@@ -605,8 +605,6 @@ pub enum LocalKind {
 
 #[derive(Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
 pub struct VarBindingForm<'tcx> {
-    /// The `HirId` of the variable.
-    pub var_id: hir::HirId,
     /// Is variable bound via `x`, `mut x`, `ref x`, or `ref mut x`?
     pub binding_mode: ty::BindingMode,
     /// If an explicit type was provided for this variable binding,
@@ -656,7 +654,6 @@ pub enum ImplicitSelfKind {
 CloneTypeFoldableAndLiftImpls! { BindingForm<'tcx>, }
 
 impl_stable_hash_for!(struct self::VarBindingForm<'tcx> {
-    var_id,
     binding_mode,
     opt_ty_info,
     opt_match_place,
@@ -877,7 +874,9 @@ impl<'tcx> LocalDecl<'tcx> {
         match self.is_user_variable {
             Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
                 binding_mode: ty::BindingMode::BindByValue(_),
-                ..
+                opt_ty_info: _,
+                opt_match_place: _,
+                pat_span: _,
             }))) => true,
 
             Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(ImplicitSelfKind::Imm))) => true,
@@ -893,7 +892,9 @@ impl<'tcx> LocalDecl<'tcx> {
         match self.is_user_variable {
             Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
                 binding_mode: ty::BindingMode::BindByValue(_),
-                ..
+                opt_ty_info: _,
+                opt_match_place: _,
+                pat_span: _,
             }))) => true,
 
             Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(_))) => true,
@@ -2830,7 +2831,7 @@ impl Location {
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable, HashStable)]
 pub enum UnsafetyViolationKind {
     General,
-    /// Permitted in const fns and regular fns.
+    /// Permitted both in `const fn`s and regular `fn`s.
     GeneralAndConstFn,
     ExternStatic(hir::HirId),
     BorrowPacked(hir::HirId),
@@ -2848,7 +2849,7 @@ pub struct UnsafetyViolation {
 pub struct UnsafetyCheckResult {
     /// Violations that are propagated *upwards* from this function.
     pub violations: Lrc<[UnsafetyViolation]>,
-    /// Unsafe blocks in this function, along with whether they are used. This is
+    /// `unsafe` blocks in this function, along with whether they are used. This is
     /// used for the "unused_unsafe" lint.
     pub unsafe_blocks: Lrc<[(hir::HirId, bool)]>,
 }
@@ -2875,12 +2876,14 @@ pub struct GeneratorLayout<'tcx> {
     /// layout.
     pub storage_conflicts: BitMatrix<GeneratorSavedLocal, GeneratorSavedLocal>,
 
-    /// Names and scopes of all the stored generator locals.
+    /// The names and scopes of all the stored generator locals.
+    ///
+    /// N.B., this is *strictly* a temporary hack for codegen
+    /// debuginfo generation, and will be removed at some point.
+    /// Do **NOT** use it for anything else, local information should not be
+    /// in the MIR, please rely on local crate HIR or other side-channels.
     //
-    // NOTE(tmandry) This is *strictly* a temporary hack for codegen
+    // FIXME(tmandry): see above.
-    // debuginfo generation, and will be removed at some point.
-    // Do **NOT** use it for anything else, local information should not be
-    // in the MIR, please rely on local crate HIR or other side-channels.
     pub __local_debuginfo_codegen_only_do_not_use: IndexVec<GeneratorSavedLocal, LocalDecl<'tcx>>,
 }
 

src/librustc/session/config.rs

@@ -711,11 +711,11 @@ impl Passes {
 }
 
 /// Defines all `CodegenOptions`/`DebuggingOptions` fields and parsers all at once. The goal of this
-/// macro is to define an interface that can be programmatically used by the option parser in order
+/// macro is to define an interface that can be programmatically used by the option parser
 /// to initialize the struct without hardcoding field names all over the place.
 ///
 /// The goal is to invoke this macro once with the correct fields, and then this macro generates all
-/// necessary code. The main gotcha of this macro is the cgsetters module which is a bunch of
+/// necessary code. The main gotcha of this macro is the `cgsetters` module which is a bunch of
 /// generated code to parse an option into its respective field in the struct. There are a few
 /// hand-written parsers for parsing specific types of values in this module.
 macro_rules! options {

src/librustc/traits/util.rs

@@ -571,7 +571,7 @@ impl<'tcx> TyCtxt<'tcx> {
                           -> Vec<ty::PolyTraitRef<'tcx>>
     {
         if source_trait_ref.def_id() == target_trait_def_id {
-            return vec![source_trait_ref]; // Shorcut the most common case.
+            return vec![source_trait_ref]; // Shortcut the most common case.
         }
 
         supertraits(self, source_trait_ref)

src/librustc/ty/context.rs

@@ -1,5 +1,3 @@
-// ignore-tidy-filelength
-
 //! Type context book-keeping.
 
 use crate::arena::Arena;
@@ -1753,7 +1751,7 @@ pub mod tls {
         pub task_deps: Option<&'a Lock<TaskDeps>>,
     }
 
-    /// Sets Rayon's thread local variablem, which is preserved for Rayon jobs
+    /// Sets Rayon's thread-local variable, which is preserved for Rayon jobs
     /// to `value` during the call to `f`. It is restored to its previous value after.
     /// This is used to set the pointer to the new `ImplicitCtxt`.
     #[cfg(parallel_compiler)]
@@ -1762,7 +1760,7 @@ pub mod tls {
         rayon_core::tlv::with(value, f)
     }
 
-    /// Gets Rayon's thread local variable, which is preserved for Rayon jobs.
+    /// Gets Rayon's thread-local variable, which is preserved for Rayon jobs.
     /// This is used to get the pointer to the current `ImplicitCtxt`.
     #[cfg(parallel_compiler)]
     #[inline]

src/librustc/ty/print/pretty.rs

@@ -1093,38 +1093,35 @@ impl<F: fmt::Write> Printer<'tcx> for FmtPrinter<'_, 'tcx, F> {
         }
 
         let key = self.tcx.def_key(def_id);
-        match key.disambiguated_data.data {
+        if let DefPathData::Impl = key.disambiguated_data.data {
-            DefPathData::Impl => {
+            // Always use types for non-local impls, where types are always
-                // Always use types for non-local impls, where types are always
+            // available, and filename/line-number is mostly uninteresting.
-                // available, and filename/line-number is mostly uninteresting.
+            let use_types =
-                let use_types =
+                !def_id.is_local() || {
-                    !def_id.is_local() || {
+                    // Otherwise, use filename/line-number if forced.
-                        // Otherwise, use filename/line-number if forced.
+                    let force_no_types = FORCE_IMPL_FILENAME_LINE.with(|f| f.get());
-                        let force_no_types = FORCE_IMPL_FILENAME_LINE.with(|f| f.get());
+                    !force_no_types
-                        !force_no_types
+                };
-                    };
 
-                if !use_types {
+            if !use_types {
-                    // If no type info is available, fall back to
+                // If no type info is available, fall back to
-                    // pretty-printing some span information. This should
+                // pretty printing some span information. This should
-                    // only occur very early in the compiler pipeline.
+                // only occur very early in the compiler pipeline.
-                    let parent_def_id = DefId { index: key.parent.unwrap(), ..def_id };
+                let parent_def_id = DefId { index: key.parent.unwrap(), ..def_id };
-                    let span = self.tcx.def_span(def_id);
+                let span = self.tcx.def_span(def_id);
 
-                    self = self.print_def_path(parent_def_id, &[])?;
+                self = self.print_def_path(parent_def_id, &[])?;
 
-                    // HACK(eddyb) copy of `path_append` to avoid
+                // HACK(eddyb) copy of `path_append` to avoid
-                    // constructing a `DisambiguatedDefPathData`.
+                // constructing a `DisambiguatedDefPathData`.
-                    if !self.empty_path {
+                if !self.empty_path {
-                        write!(self, "::")?;
+                    write!(self, "::")?;
-                    }
-                    write!(self, "<impl at {:?}>", span)?;
-                    self.empty_path = false;
-
-                    return Ok(self);
                 }
+                write!(self, "<impl at {:?}>", span)?;
+                self.empty_path = false;
+
+                return Ok(self);
             }
-            _ => {}
         }
 
         self.default_print_def_path(def_id, substs)

src/librustc/ty/query/on_disk_cache.rs

@@ -429,7 +429,7 @@ impl<'sess> OnDiskCache<'sess> {
 
 //- DECODING -------------------------------------------------------------------
 
-/// A decoder that can read fro the incr. comp. cache. It is similar to the onem
+/// A decoder that can read fro the incr. comp. cache. It is similar to the one
 /// we use for crate metadata decoding in that it can rebase spans and eventually
 /// will also handle things that contain `Ty` instances.
 struct CacheDecoder<'a, 'tcx> {
@@ -460,19 +460,17 @@ impl<'a, 'tcx> CacheDecoder<'a, 'tcx> {
     }
 }
 
-pub trait DecoderWithPosition: Decoder {
+trait DecoderWithPosition: Decoder {
     fn position(&self) -> usize;
 }
 
 impl<'a> DecoderWithPosition for opaque::Decoder<'a> {
-    #[inline]
     fn position(&self) -> usize {
         self.position()
     }
 }
 
 impl<'a, 'tcx> DecoderWithPosition for CacheDecoder<'a, 'tcx> {
-    #[inline]
     fn position(&self) -> usize {
         self.opaque.position()
     }
@@ -480,7 +478,7 @@ impl<'a, 'tcx> DecoderWithPosition for CacheDecoder<'a, 'tcx> {
 
 // Decodes something that was encoded with `encode_tagged()` and verify that the
 // tag matches and the correct amount of bytes was read.
-pub fn decode_tagged<D, T, V>(decoder: &mut D, expected_tag: T) -> Result<V, D::Error>
+fn decode_tagged<D, T, V>(decoder: &mut D, expected_tag: T) -> Result<V, D::Error>
 where
     T: Decodable + Eq + ::std::fmt::Debug,
     V: Decodable,
@@ -700,7 +698,6 @@ impl<'a, 'tcx> SpecializedDecoder<NodeId> for CacheDecoder<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> SpecializedDecoder<Fingerprint> for CacheDecoder<'a, 'tcx> {
-    #[inline]
     fn specialized_decode(&mut self) -> Result<Fingerprint, Self::Error> {
         Fingerprint::decode_opaque(&mut self.opaque)
     }
@@ -754,7 +751,7 @@ where
     /// encode the specified tag, then the given value, then the number of
     /// bytes taken up by tag and value. On decoding, we can then verify that
     /// we get the expected tag and read the expected number of bytes.
-    pub fn encode_tagged<T: Encodable, V: Encodable>(
+    fn encode_tagged<T: Encodable, V: Encodable>(
         &mut self,
         tag: T,
         value: &V
@@ -960,7 +957,6 @@ where
 }
 
 impl<'a, 'tcx> SpecializedEncoder<Fingerprint> for CacheEncoder<'a, 'tcx, opaque::Encoder> {
-    #[inline]
     fn specialized_encode(&mut self, f: &Fingerprint) -> Result<(), Self::Error> {
         f.encode_opaque(&mut self.encoder)
     }