From fd48ca20d366cff016cb0f9cfd8e98c44a91da10 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Fri, 6 Sep 2019 21:05:37 +0100 Subject: [PATCH] Apply suggestions from code review Co-Authored-By: Mazdak Farrokhzad --- src/librustc/hir/lowering.rs | 2 +- src/librustc/mir/interpret/allocation.rs | 11 ++--- src/librustc/mir/interpret/mod.rs | 1 - src/librustc/mir/mod.rs | 27 +++++++------ src/librustc/session/config.rs | 4 +- src/librustc/traits/util.rs | 2 +- src/librustc/ty/context.rs | 6 +-- src/librustc/ty/print/pretty.rs | 51 +++++++++++------------- src/librustc/ty/query/on_disk_cache.rs | 12 ++---- 9 files changed, 55 insertions(+), 61 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index fd6445ea3c4..b50cfa00f09 100644 --- a/src/librustc/hir/lowering.rs +++ b/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`. diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index e81096c3066..755cda792ba 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/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 Allocation { } } - /// 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( diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 6f7e11877d5..23433c2e883 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/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>, diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index da2faeb4e1e..18a5142208d 100644 --- a/src/librustc/mir/mod.rs +++ b/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, - /// 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 - // 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. + // FIXME(tmandry): see above. pub __local_debuginfo_codegen_only_do_not_use: IndexVec>, } diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 27a33ced119..c74b2fee41d 100644 --- a/src/librustc/session/config.rs +++ b/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 { diff --git a/src/librustc/traits/util.rs b/src/librustc/traits/util.rs index 7f3c62ec994..3d36790c94b 100644 --- a/src/librustc/traits/util.rs +++ b/src/librustc/traits/util.rs @@ -571,7 +571,7 @@ impl<'tcx> TyCtxt<'tcx> { -> Vec> { 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) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 6867230f4ec..8e8472a5aac 100644 --- a/src/librustc/ty/context.rs +++ b/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>, } - /// 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] diff --git a/src/librustc/ty/print/pretty.rs b/src/librustc/ty/print/pretty.rs index 419a815074b..d99580116e4 100644 --- a/src/librustc/ty/print/pretty.rs +++ b/src/librustc/ty/print/pretty.rs @@ -1093,38 +1093,35 @@ impl Printer<'tcx> for FmtPrinter<'_, 'tcx, F> { } let key = self.tcx.def_key(def_id); - match key.disambiguated_data.data { - DefPathData::Impl => { - // Always use types for non-local impls, where types are always - // available, and filename/line-number is mostly uninteresting. - let use_types = - !def_id.is_local() || { - // Otherwise, use filename/line-number if forced. - let force_no_types = FORCE_IMPL_FILENAME_LINE.with(|f| f.get()); - !force_no_types - }; + if let DefPathData::Impl = key.disambiguated_data.data { + // Always use types for non-local impls, where types are always + // available, and filename/line-number is mostly uninteresting. + let use_types = + !def_id.is_local() || { + // Otherwise, use filename/line-number if forced. + let force_no_types = FORCE_IMPL_FILENAME_LINE.with(|f| f.get()); + !force_no_types + }; - if !use_types { - // If no type info is available, fall back to - // pretty-printing some span information. This should - // only occur very early in the compiler pipeline. - let parent_def_id = DefId { index: key.parent.unwrap(), ..def_id }; - let span = self.tcx.def_span(def_id); + if !use_types { + // If no type info is available, fall back to + // pretty printing some span information. This should + // only occur very early in the compiler pipeline. + let parent_def_id = DefId { index: key.parent.unwrap(), ..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 - // constructing a `DisambiguatedDefPathData`. - if !self.empty_path { - write!(self, "::")?; - } - write!(self, "", span)?; - self.empty_path = false; - - return Ok(self); + // HACK(eddyb) copy of `path_append` to avoid + // constructing a `DisambiguatedDefPathData`. + if !self.empty_path { + write!(self, "::")?; } + write!(self, "", span)?; + self.empty_path = false; + + return Ok(self); } - _ => {} } self.default_print_def_path(def_id, substs) diff --git a/src/librustc/ty/query/on_disk_cache.rs b/src/librustc/ty/query/on_disk_cache.rs index d24143ced5b..4cef6a09925 100644 --- a/src/librustc/ty/query/on_disk_cache.rs +++ b/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(decoder: &mut D, expected_tag: T) -> Result +fn decode_tagged(decoder: &mut D, expected_tag: T) -> Result where T: Decodable + Eq + ::std::fmt::Debug, V: Decodable, @@ -700,7 +698,6 @@ impl<'a, 'tcx> SpecializedDecoder for CacheDecoder<'a, 'tcx> { } impl<'a, 'tcx> SpecializedDecoder for CacheDecoder<'a, 'tcx> { - #[inline] fn specialized_decode(&mut self) -> Result { 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( + fn encode_tagged( &mut self, tag: T, value: &V @@ -960,7 +957,6 @@ where } impl<'a, 'tcx> SpecializedEncoder for CacheEncoder<'a, 'tcx, opaque::Encoder> { - #[inline] fn specialized_encode(&mut self, f: &Fingerprint) -> Result<(), Self::Error> { f.encode_opaque(&mut self.encoder) }