From 1404d0ff8e4ac69f5a6b72db6d9348333875bc4a Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Fri, 6 Sep 2019 03:58:55 +0100 Subject: [PATCH] Aggregation of cosmetic changes made during work on REPL PRs: librustc_mir --- src/librustc_mir/borrow_check/borrow_set.rs | 7 +- .../borrow_check/conflict_errors.rs | 29 +- .../borrow_check/error_reporting.rs | 56 ++-- src/librustc_mir/borrow_check/mod.rs | 277 ++++++++---------- src/librustc_mir/borrow_check/move_errors.rs | 4 +- .../borrow_check/mutability_errors.rs | 10 +- .../borrow_check/nll/explain_borrow/mod.rs | 2 +- .../borrow_check/nll/invalidation.rs | 9 +- .../nll/type_check/liveness/local_use_map.rs | 4 +- .../borrow_check/nll/type_check/mod.rs | 16 +- src/librustc_mir/borrow_check/path_utils.rs | 4 +- .../borrow_check/places_conflict.rs | 4 +- src/librustc_mir/build/block.rs | 18 +- src/librustc_mir/build/expr/as_rvalue.rs | 94 +++--- src/librustc_mir/build/expr/into.rs | 32 +- src/librustc_mir/build/expr/stmt.rs | 4 +- src/librustc_mir/build/matches/mod.rs | 80 ++--- src/librustc_mir/build/mod.rs | 15 +- src/librustc_mir/build/scope.rs | 70 ++--- src/librustc_mir/const_eval.rs | 82 +++--- .../dataflow/drop_flag_effects.rs | 13 +- .../dataflow/impls/borrowed_locals.rs | 17 +- src/librustc_mir/dataflow/impls/borrows.rs | 12 +- src/librustc_mir/dataflow/impls/mod.rs | 28 +- .../dataflow/impls/storage_liveness.rs | 31 +- src/librustc_mir/dataflow/mod.rs | 7 +- src/librustc_mir/dataflow/move_paths/mod.rs | 30 +- src/librustc_mir/hair/cx/block.rs | 2 +- src/librustc_mir/hair/mod.rs | 30 +- src/librustc_mir/interpret/cast.rs | 8 +- src/librustc_mir/interpret/eval_context.rs | 61 ++-- src/librustc_mir/interpret/intern.rs | 74 ++--- .../interpret/intrinsics/type_name.rs | 14 +- src/librustc_mir/interpret/machine.rs | 30 +- src/librustc_mir/interpret/memory.rs | 86 +++--- src/librustc_mir/interpret/operand.rs | 107 +++---- src/librustc_mir/interpret/place.rs | 109 ++++--- src/librustc_mir/interpret/step.rs | 18 +- src/librustc_mir/interpret/terminator.rs | 11 +- src/librustc_mir/interpret/traits.rs | 10 +- src/librustc_mir/transform/add_retag.rs | 2 +- src/librustc_mir/transform/generator.rs | 35 ++- src/librustc_mir/transform/inline.rs | 2 +- src/librustc_mir/transform/mod.rs | 16 +- src/librustc_mir/transform/qualify_consts.rs | 4 +- .../transform/qualify_min_const_fn.rs | 16 +- .../transform/remove_noop_landing_pads.rs | 4 +- src/librustc_mir/transform/rustc_peek.rs | 2 +- src/librustc_mir/transform/simplify.rs | 9 +- src/librustc_mir/util/pretty.rs | 24 +- 50 files changed, 830 insertions(+), 799 deletions(-) diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index c9e6e7f70a2b4..18f2fab059475 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -3,6 +3,7 @@ use crate::borrow_check::nll::ToRegionVid; use crate::borrow_check::path_utils::allow_two_phase_borrow; use crate::dataflow::indexes::BorrowIndex; use crate::dataflow::move_paths::MoveData; + use rustc::mir::traversal; use rustc::mir::visit::{PlaceContext, Visitor, NonUseContext, MutatingUseContext}; use rustc::mir::{self, Location, Body, Local}; @@ -168,7 +169,7 @@ struct GatherBorrows<'a, 'tcx> { activation_map: FxHashMap>, local_map: FxHashMap>, - /// When we encounter a 2-phase borrow statement, it will always + /// When we encounter a two-phase borrow statement, it will always /// be assigning into a temporary TEMP: /// /// TEMP = &foo @@ -228,7 +229,7 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> { } // We found a use of some temporary TMP - // check whether we (earlier) saw a 2-phase borrow like + // check whether we (earlier) saw a two-phase borrow like // // TMP = &mut place if let Some(&borrow_index) = self.pending_activations.get(temp) { @@ -309,7 +310,7 @@ impl<'a, 'tcx> GatherBorrows<'a, 'tcx> { return; } - // When we encounter a 2-phase borrow statement, it will always + // When we encounter a two-phase borrow statement, it will always // be assigning into a temporary TEMP: // // TEMP = &foo diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index bfc7050ebf71c..007fa592ed103 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -26,7 +26,7 @@ use crate::util::borrowck_errors; #[derive(Debug)] struct MoveSite { - /// Index of the "move out" that we found. The `MoveData` can + /// The index of the "move out" that we found. The `MoveData` can /// then tell us where the move occurred. moi: MoveOutIndex, @@ -35,7 +35,7 @@ struct MoveSite { traversed_back_edge: bool } -/// Which case a StorageDeadOrDrop is for. +/// Which case a `StorageDeadOrDrop` is for. #[derive(Copy, Clone, PartialEq, Eq, Debug)] enum StorageDeadOrDrop<'tcx> { LocalStorageDead, @@ -1130,8 +1130,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { format!("`{}` is borrowed here", place_desc), ) } else { - let root_place = self.prefixes(borrow.borrowed_place.as_ref(), - PrefixSet::All) + let root_place = self.prefixes(borrow.borrowed_place.as_ref(), PrefixSet::All) .last() .unwrap(); let local = if let PlaceRef { @@ -1223,7 +1222,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { &format!("function requires argument type to outlive `{}`", fr_name), ); } - _ => bug!("report_escaping_closure_capture called with unexpected constraint \ + _ => bug!("`report_escaping_closure_capture` called with unexpected constraint \ category: `{:?}`", category), } err @@ -1245,7 +1244,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { match tables.node_type(mir_hir_id).sty { ty::Closure(..) => "closure", ty::Generator(..) => "generator", - _ => bug!("Closure body doesn't have a closure or generator type"), + _ => bug!("closure body doesn't have a closure or generator type"), } } else { "function" @@ -1302,7 +1301,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { 'dfs: while let Some((location, is_back_edge)) = stack.pop() { debug!( - "report_use_of_moved_or_uninitialized: (current_location={:?}, back_edge={})", + "report_use_of_moved_or_uninitialized: current_location={:?} back_edge={}", location, is_back_edge ); @@ -1310,23 +1309,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { continue; } - // check for moves + // Check for moves. let stmt_kind = body[location.block] .statements .get(location.statement_index) .map(|s| &s.kind); if let Some(StatementKind::StorageDead(..)) = stmt_kind { - // this analysis only tries to find moves explicitly + // This analysis only tries to find moves explicitly // written by the user, so we ignore the move-outs - // created by `StorageDead` and at the beginning + // created by StorageDead and at the beginning // of a function. } else { - // If we are found a use of a.b.c which was in error, then we want to look for - // moves not only of a.b.c but also a.b and a. + // If we are found a use of `a.b.c` which was in error, then we want to look for + // moves not only of `a.b.c` but also `a.b` and `a`. // // Note that the moves data already includes "parent" paths, so we don't have to - // worry about the other case: that is, if there is a move of a.b.c, it is already - // marked as a move of a.b and a as well, so we will generate the correct errors + // worry about the other case: that is, if there is a move of `a.b.c`, it is already + // marked as a move of `a.b` and `a` as well, so we will generate the correct errors // there. let mut mpis = vec![mpi]; let move_paths = &self.move_data.move_paths; @@ -1362,7 +1361,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - // check for inits + // Check for inits. let mut any_match = false; drop_flag_effects::for_location_inits( self.infcx.tcx, diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 9f25e98052ec4..f79add1103bed 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -119,13 +119,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - /// End-user visible description of `place` if one can be found. If the + /// Gets an end-user visible description of `place` if one can be found. If the /// place is a temporary for instance, None will be returned. pub(super) fn describe_place(&self, place_ref: PlaceRef<'cx, 'tcx>) -> Option { self.describe_place_with_options(place_ref, IncludingDowncast(false)) } - /// End-user visible description of `place` if one can be found. If the + /// Gets an end-user visible description of `place` if one can be found. If the /// place is a temporary for instance, None will be returned. /// `IncludingDowncast` parameter makes the function return `Err` if `ProjectionElem` is /// `Downcast` and `IncludingDowncast` is true @@ -195,7 +195,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } else { if autoderef { - // FIXME turn this recursion into iteration + // FIXME: turn this recursion into iteration. self.append_place_to_string( PlaceRef { base, @@ -219,7 +219,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { &including_downcast, )?; } else { - // FIXME deduplicate this and the _ => body below + // FIXME: deduplicate this and the `_ =>` body below. buf.push_str(&"*"); self.append_place_to_string( PlaceRef { @@ -307,11 +307,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } buf.push_str("]"); } - ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } => { + ProjectionElem::ConstantIndex { .. } | + ProjectionElem::Subslice { .. } => { autoderef = true; // Since it isn't possible to borrow an element on a particular index and // then use another while the borrow is held, don't output indices details - // to avoid confusing the end-user + // to avoid confusing the end-user. self.append_place_to_string( PlaceRef { base, @@ -343,7 +344,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - /// End-user visible description of the `field`nth field of `base` + /// Gets an end-user visible description of the `field`nth field of `base`. fn describe_field(&self, place: PlaceRef<'cx, 'tcx>, field: Field) -> String { // FIXME Place2 Make this work iteratively match place { @@ -375,9 +376,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ProjectionElem::Field(_, field_type) => { self.describe_field_from_ty(&field_type, field, None) } - ProjectionElem::Index(..) - | ProjectionElem::ConstantIndex { .. } - | ProjectionElem::Subslice { .. } => { + ProjectionElem::Index(..) | + ProjectionElem::ConstantIndex { .. } | + ProjectionElem::Subslice { .. } => { self.describe_field(PlaceRef { base, projection: &proj.base, @@ -387,7 +388,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - /// End-user visible description of the `field_index`nth field of `ty` + /// Gets an end-user visible description of the `field_index`nth field of `ty`. fn describe_field_from_ty( &self, ty: Ty<'_>, @@ -395,7 +396,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { variant_index: Option ) -> String { if ty.is_box() { - // If the type is a box, the field is described from the boxed type + // If the type is a box, the field is described from the boxed type. self.describe_field_from_ty(&ty.boxed_ty(), field, variant_index) } else { match ty.sty { @@ -411,12 +412,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { .to_string() }, ty::Tuple(_) => field.index().to_string(), - ty::Ref(_, ty, _) | ty::RawPtr(ty::TypeAndMut { ty, .. }) => { + ty::Ref(_, ty, _) | + ty::RawPtr(ty::TypeAndMut { ty, .. }) => { self.describe_field_from_ty(&ty, field, variant_index) } - ty::Array(ty, _) | ty::Slice(ty) => + ty::Array(ty, _) | + ty::Slice(ty) => self.describe_field_from_ty(&ty, field, variant_index), - ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => { + ty::Closure(def_id, _) | + ty::Generator(def_id, _, _) => { // `tcx.upvars(def_id)` returns an `Option`, which is `None` in case // the closure comes from another crate. But in that case we wouldn't // be borrowck'ing it, so we can just unwrap: @@ -491,7 +495,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // we'll use this to check whether it was originally from an overloaded // operator. match self.move_data.rev_lookup.find(deref_base) { - LookupResult::Exact(mpi) | LookupResult::Parent(Some(mpi)) => { + LookupResult::Exact(mpi) | + LookupResult::Parent(Some(mpi)) => { debug!("borrowed_content_source: mpi={:?}", mpi); for i in &self.move_data.init_path_map[mpi] { @@ -557,8 +562,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // this by hooking into the pretty printer and telling it to label the // lifetimes without names with the value `'0`. match ty.sty { - ty::Ref(ty::RegionKind::ReLateBound(_, br), _, _) - | ty::Ref( + ty::Ref(ty::RegionKind::ReLateBound(_, br), _, _) | + ty::Ref( ty::RegionKind::RePlaceholder(ty::PlaceholderRegion { name: br, .. }), _, _, @@ -579,8 +584,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let region = match ty.sty { ty::Ref(region, _, _) => { match region { - ty::RegionKind::ReLateBound(_, br) - | ty::RegionKind::RePlaceholder(ty::PlaceholderRegion { name: br, .. }) => { + ty::RegionKind::ReLateBound(_, br) | + ty::RegionKind::RePlaceholder(ty::PlaceholderRegion { name: br, .. }) => { printer.region_highlight_mode.highlighting_bound_region(*br, counter) } _ => {} @@ -618,14 +623,15 @@ impl UseSpans { match self { UseSpans::ClosureUse { args_span: span, .. - } - | UseSpans::OtherUse(span) => span, + } | + UseSpans::OtherUse(span) => span, } } pub(super) fn var_or_use(self) -> Span { match self { - UseSpans::ClosureUse { var_span: span, .. } | UseSpans::OtherUse(span) => span, + UseSpans::ClosureUse { var_span: span, .. } | + UseSpans::OtherUse(span) => span, } } @@ -726,8 +732,8 @@ impl BorrowedContentSource<'tcx> { BorrowedContentSource::DerefMutableRef => Some("mutable reference"), // Overloaded deref and index operators should be evaluated into a // temporary. So we don't need a description here. - BorrowedContentSource::OverloadedDeref(_) - | BorrowedContentSource::OverloadedIndex(_) => None + BorrowedContentSource::OverloadedDeref(_) | + BorrowedContentSource::OverloadedIndex(_) => None } } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 9ad7cbc478bb0..277846b6463cb 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1,6 +1,7 @@ //! This query borrow-checks the MIR to (further) ensure it is not broken. use crate::borrow_check::nll::region_infer::RegionInferenceContext; + use rustc::hir::{self, HirId}; use rustc::hir::Node; use rustc::hir::def_id::DefId; @@ -66,14 +67,14 @@ mod used_muts; pub(crate) mod nll; -// FIXME(eddyb) perhaps move this somewhere more centrally. +// FIXME(eddyb): perhaps move this somewhere more central. #[derive(Debug)] crate struct Upvar { name: Name, var_hir_id: HirId, - /// If true, the capture is behind a reference. + /// `true` if the capture is behind a reference. by_ref: bool, mutability: Mutability, @@ -281,7 +282,7 @@ fn do_mir_borrowck<'a, 'tcx>( if let Some(errors) = move_errors { mbcx.report_move_errors(errors); } - mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer + mbcx.analyze_results(&mut state); // entry point for `DataflowResultsConsumer` // Convert any reservation warnings into lints. let reservation_warnings = mem::take(&mut mbcx.reservation_warnings); @@ -296,7 +297,7 @@ fn do_mir_borrowck<'a, 'tcx>( id }; - // Span and message don't matter; we overwrite them below anyway + // Span and message don't matter; we overwrite them below anyway. let mut diag = mbcx.infcx.tcx.struct_span_lint_hir( MUTABLE_BORROW_RESERVATION_CONFLICT, lint_root, DUMMY_SP, ""); @@ -309,7 +310,7 @@ fn do_mir_borrowck<'a, 'tcx>( // For each non-user used mutable variable, check if it's been assigned from // a user-declared local. If so, then put that local into the used_mut set. - // Note that this set is expected to be small - only upvars from closures + // Note that this set is expected to be small -- only upvars from closures // would have a chance of erroneously adding non-user-defined mutable vars // to the set. let temporary_used_locals: FxHashSet = mbcx.used_mut.iter() @@ -330,12 +331,12 @@ fn do_mir_borrowck<'a, 'tcx>( if let ClearCrossCrate::Set(ref vsi) = mbcx.body.source_scope_local_data { let local_decl = &mbcx.body.local_decls[local]; - // Skip implicit `self` argument for closures + // Skip implicit `self` argument for closures. if local.index() == 1 && tcx.is_closure(mbcx.mir_def_id) { continue; } - // Skip over locals that begin with an underscore or have no name + // Skip over locals that begin with an underscore or have no name. match local_decl.name { Some(name) => if name.as_str().starts_with("_") { continue; @@ -375,17 +376,17 @@ fn do_mir_borrowck<'a, 'tcx>( mbcx.errors_buffer.sort_by_key(|diag| diag.span.primary_span()); if !mbcx.disable_error_downgrading && tcx.migrate_borrowck() { - // When borrowck=migrate, check if AST-borrowck would + // When `borrowck=migrate`, check if AST-borrowck would // error on the given code. - // rust-lang/rust#55492, rust-lang/rust#58776 check the base def id + // rust-lang/rust#55492, rust-lang/rust#58776 check the base `DefId` // for errors. AST borrowck is responsible for aggregating // `signalled_any_error` from all of the nested closures here. let base_def_id = tcx.closure_base_def_id(def_id); match tcx.borrowck(base_def_id).signalled_any_error { SignalledError::NoErrorsSeen => { - // if AST-borrowck signalled no errors, then + // If AST-borrowck signalled no errors, then // downgrade all the buffered MIR-borrowck errors // to warnings. @@ -394,7 +395,7 @@ fn do_mir_borrowck<'a, 'tcx>( } } SignalledError::SawSomeError => { - // if AST-borrowck signalled a (cancelled) error, + // If AST-borrowck signalled a (cancelled) error, // then we will just emit the buffered // MIR-borrowck errors as normal. } @@ -421,10 +422,10 @@ fn downgrade_if_error(diag: &mut Diagnostic) { diag.level = Level::Warning; diag.warn( "this error has been downgraded to a warning for backwards \ - compatibility with previous releases", + compatibility with previous releases", ).warn( "this represents potential undefined behavior in your code and \ - this warning will become a hard error in the future", + this warning will become a hard error in the future", ).note( "for more information, try `rustc --explain E0729`" ); @@ -438,17 +439,16 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> { param_env: ty::ParamEnv<'tcx>, move_data: &'cx MoveData<'tcx>, - /// Map from MIR `Location` to `LocationIndex`; created - /// when MIR borrowck begins. + /// Map from MIR `Location` to `LocationIndex`; created when MIR borrowck begins. location_table: &'cx LocationTable, movable_generator: bool, /// This keeps track of whether local variables are free-ed when the function - /// exits even without a `StorageDead`, which appears to be the case for + /// exits even without a StorageDead, which appears to be the case for /// constants. - /// - /// I'm not sure this is the right approach - @eddyb could you try and - /// figure this out? + // + // FIXME: I'm not sure this is the right approach -- @eddyb, could you try and + // figure this out? locals_are_invalidated_at_exit: bool, /// This field keeps track of when borrow errors are reported in the access_place function /// so that there is no duplicate reporting. This field cannot also be used for the conflicting @@ -489,7 +489,7 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> { /// This field keeps track of errors reported in the checking of uninitialized variables, /// so that we don't report seemingly duplicate errors. uninitialized_error_reported: FxHashSet>, - /// Errors to be reported buffer + /// Errors to be reported buffer. errors_buffer: Vec, /// If there are no errors reported by the HIR borrow checker, we downgrade /// all NLL errors to warnings. Setting this flag disables downgrading. @@ -501,25 +501,25 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> { /// mutable upvars that have been used. This field keeps track of them. used_mut_upvars: SmallVec<[Field; 8]>, /// Non-lexical region inference context, if NLL is enabled. This - /// contains the results from region inference and lets us e.g. + /// contains the results from region inference and lets us, e.g., /// find out which CFG points are contained in each borrow region. nonlexical_regioncx: Rc>, - /// The set of borrows extracted from the MIR + /// The set of borrows extracted from the MIR. borrow_set: Rc>, - /// Dominators for MIR + /// Dominators for MIR. dominators: Dominators, - /// Information about upvars not necessarily preserved in types or MIR + /// Information about upvars not necessarily preserved in types or MIR. upvars: Vec, } -// Check that: -// 1. assignments are always made to mutable locations (FIXME: does that still really go here?) -// 2. loans made in overlapping scopes do not conflict -// 3. assignments do not affect things loaned out as immutable -// 4. moves do not affect things loaned out in any way +// Checks that: +// 1. Assignments are always made to mutable locations. (FIXME: does that still really go here?) +// 2. Loans made in overlapping scopes do not conflict. +// 3. Assignments do not affect things loaned out as immutable. +// 4. Moves do not affect things loaned out in any way. impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx> { type FlowState = Flows<'cx, 'tcx>; @@ -594,7 +594,7 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx StatementKind::InlineAsm(ref asm) => { for (o, output) in asm.asm.outputs.iter().zip(asm.outputs.iter()) { if o.is_indirect { - // FIXME(eddyb) indirect inline asm outputs should + // FIXME(eddyb): indirect inline ASM outputs should // be encoded through MIR place derefs instead. self.access_place( location, @@ -623,12 +623,11 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx self.consume_operand(location, (input, span), flow_state); } } - StatementKind::Nop - | StatementKind::AscribeUserType(..) - | StatementKind::Retag { .. } - | StatementKind::StorageLive(..) => { - // `Nop`, `AscribeUserType`, `Retag`, and `StorageLive` are irrelevant - // to borrow check. + StatementKind::AscribeUserType(..) | + StatementKind::Retag { .. } | + StatementKind::StorageLive(..) | + StatementKind::Nop => { + // Irrelevant to borrowck. } StatementKind::StorageDead(local) => { self.access_place( @@ -763,7 +762,7 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx self.consume_operand(loc, (value, span), flow_state); if self.movable_generator { - // Look for any active borrows to locals + // Look for any active borrows to locals. let borrow_set = self.borrow_set.clone(); flow_state.with_outgoing_borrows(|borrows| { for i in borrows { @@ -798,7 +797,7 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx real_target: _, unwind: _, } => { - // no data used, thus irrelevant to borrowck + // No data used, thus irrelevant to borrowck. } } } @@ -822,7 +821,7 @@ enum ArtificialField { #[derive(Copy, Clone, PartialEq, Eq, Debug)] enum AccessDepth { /// From the RFC: "A *shallow* access means that the immediate - /// fields reached at P are accessed, but references or pointers + /// fields reached at `P` are accessed, but references or pointers /// found within are not dereferenced. Right now, the only access /// that is shallow is an assignment like `x = ...;`, which would /// be a *shallow write* of `x`." @@ -833,13 +832,13 @@ enum AccessDepth { /// this action." Deep, - /// Access is Deep only when there is a Drop implementation that + /// Access is deep only when there is a `Drop` implementation that /// can reach the data behind the reference. Drop, } -/// Kind of access to a value: read or write -/// (For informational purposes only) +/// Kind of access to a value: read or write. +/// (For informational purposes only.) #[derive(Copy, Clone, PartialEq, Eq, Debug)] enum ReadOrWrite { /// From the RFC: "A *read* means that the existing data may be @@ -858,16 +857,16 @@ enum ReadOrWrite { Activation(WriteKind, BorrowIndex), } -/// Kind of read access to a value -/// (For informational purposes only) +/// Kind of read access to a value. +/// (For informational purposes only.) #[derive(Copy, Clone, PartialEq, Eq, Debug)] enum ReadKind { Borrow(BorrowKind), Copy, } -/// Kind of write access to a value -/// (For informational purposes only) +/// Kind of write access to a value. +/// (For informational purposes only.) #[derive(Copy, Clone, PartialEq, Eq, Debug)] enum WriteKind { StorageDeadOrDrop, @@ -967,8 +966,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - // Check is_empty() first because it's the common case, and doing that - // way we avoid the clone() call. + // Check `is_empty()` first because it's the common case, and doing that + // way we avoid the `clone()` call. if !self.access_place_error_reported.is_empty() && self .access_place_error_reported @@ -1171,7 +1170,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { mode: MutateMode, flow_state: &Flows<'cx, 'tcx>, ) { - // Write of P[i] or *P, or WriteAndRead of any P, requires P init'd. + // Write of `P[i]` or `*P`, or `WriteAndRead` of any `P`, requires `P` init'd. match mode { MutateMode::WriteAndRead => { self.check_if_path_or_subpath_is_moved( @@ -1262,8 +1261,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Rvalue::Use(ref operand) | Rvalue::Repeat(ref operand, _) - | Rvalue::UnaryOp(_ /*un_op*/, ref operand) - | Rvalue::Cast(_ /*cast_kind*/, ref operand, _ /*ty*/) => { + | Rvalue::UnaryOp(_ /* un_op */, ref operand) + | Rvalue::Cast(_ /* cast_kind */, ref operand, _ /* ty */) => { self.consume_operand(location, (operand, span), flow_state) } @@ -1295,7 +1294,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } Rvalue::NullaryOp(_op, _ty) => { - // nullary ops take no dynamic input; no borrowck effect. + // Nullary ops take no dynamic input; no borrowck effect. // // FIXME: is above actually true? Do we want to track // the fact that uninitialized data can be created via @@ -1305,7 +1304,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Rvalue::Aggregate(ref aggregate_kind, ref operands) => { // We need to report back the list of mutable upvars that were // moved into the closure and subsequently used by the closure, - // in order to populate our used_mut set. + // in order to populate our `used_mut` set. match **aggregate_kind { AggregateKind::Closure(def_id, _) | AggregateKind::Generator(def_id, _, _) => { @@ -1391,7 +1390,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { propagate_closure_used_mut_place(self, source); } else { bug!("closures should only capture user variables \ - or references to user variables"); + or references to user variables"); } } Operand::Move(ref place) @@ -1410,8 +1409,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) { match *operand { Operand::Copy(ref place) => { - // copy of place: check if this is "copy of frozen path" - // (FIXME: see check_loans.rs) + // Copy of place: check if this is "copy of frozen path". + // FIXME: see `check_loans.rs` self.access_place( location, (place, span), @@ -1429,7 +1428,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); } Operand::Move(ref place) => { - // move of place: check if this is move of already borrowed path + // Move of place: check if this is move of already borrowed path. self.access_place( location, (place, span), @@ -1450,8 +1449,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - /// Checks whether a borrow of this place is invalidated when the function - /// exits + /// Checks whether a borrow of this place is invalidated when the functions exits. fn check_for_invalidation_at_exit( &mut self, location: Location, @@ -1525,7 +1523,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } /// Reports an error if this is a borrow of local data. - /// This is called for all Yield statements on movable generators + /// This is called for all `Yield` statements on movable generators. fn check_for_local_borrow(&mut self, borrow: &BorrowData<'tcx>, yield_span: Span) { debug!("check_for_local_borrow({:?})", borrow); @@ -1547,7 +1545,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { for &borrow_index in borrow_set.activations_at_location(location) { let borrow = &borrow_set[borrow_index]; - // only mutable borrows should be 2-phase + // Only mutable borrows should be two-phase. assert!(match borrow.kind { BorrowKind::Shared | BorrowKind::Shallow => false, BorrowKind::Unique | BorrowKind::Mut { .. } => true, @@ -1580,9 +1578,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) { debug!("check_if_reassignment_to_immutable_state({:?})", local); - // Check if any of the initializiations of `local` have happened yet: + // Check if any of the initializiations of `local` have happened yet. if let Some(init_index) = self.is_local_ever_initialized(local, flow_state) { - // And, if so, report an error. + // If so, report an error. let init = &self.move_data.inits[init_index]; let span = init.span(&self.body); self.report_illegal_reassignment( @@ -1613,16 +1611,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // 4. Move of `a.b.c`, use of `a.b.d` // 5. Uninitialized `a.x`, initialized `a.b`, use of `a.b` // 6. Copied `(a.b: &_)`, use of `*(a.b).c`; note that `a.b` - // must have been initialized for the use to be sound. - // 7. Move of `a.b.c` then reinit of `a.b.c.d`, use of `a.b.c.d` + // must have been initialized for the use to be sound + // 7. Move of `a.b.c` then reinit of `a.b.c.d`, use of `a.b.c.d`s // The dataflow tracks shallow prefixes distinctly (that is, - // field-accesses on P distinctly from P itself), in order to + // field-accesses on `P` distinctly from `P` itself), in order to // track substructure initialization separately from the whole // structure. // - // E.g., when looking at (*a.b.c).d, if the closest prefix for - // which we have a MovePath is `a.b`, then that means that the + // E.g., when looking at `(*a.b.c).d`, if the closest prefix for + // which we have a `MovePath` is `a.b`, then that means that the // initialization state of `a.b` is all we need to inspect to // know if `a.b.c` is valid (and from that we infer that the // dereference and `.d` access is also valid, since we assume @@ -1630,7 +1628,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // well-formed record structure.) // Therefore, if we seek out the *closest* prefix for which we - // have a MovePath, that should capture the initialization + // have a `MovePath`, that should capture the initialization // state for the place scenario. // // This code covers scenarios 1, 2, and 3. @@ -1645,17 +1643,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { (prefix, place_span.0, place_span.1), mpi, ); - return; // don't bother finding other problems. + return; // Don't bother finding other problems. } } Err(NoMovePathFound::ReachedStatic) => { - // Okay: we do not build MoveData for static variables - } // Only query longest prefix with a MovePath, not further - // ancestors; dataflow recurs on children when parents - // move (to support partial (re)inits). - // - // (I.e., querying parents breaks scenario 7; but may want - // to do such a query based on partial-init feature-gate.) + // Okay: we do not build `MoveData` for static variables. + } + // Only query longest prefix with a `MovePath`, not further + // ancestors; dataflow recurs on children when parents + // move (to support partial (re)inits). + // + // (I.e., querying parents breaks scenario 7; but may want + // to do such a query based on partial-init feature gate.) } } @@ -1670,7 +1669,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // Bad scenarios: // - // 1. Move of `a.b.c`, use of `a` or `a.b` + // 1. Move of `a.b.c`, use of `a` or `a.b`; note that with // partial initialization support, one might have `a.x` // initialized but not `a.b`. // 2. All bad scenarios from `check_if_full_path_is_moved` @@ -1688,7 +1687,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // A move of any shallow suffix of `place` also interferes // with an attempt to use `place`. This is scenario 3 above. // - // (Distinct from handling of scenarios 1+2+4 above because + // (Distinct from handling of scenarios 1/2/4 above, because // `place` does not interfere with suffixes of its prefixes, // e.g., `a.b.c` does not interfere with `a.b.d`) // @@ -1703,21 +1702,21 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { (place_span.0, place_span.0, place_span.1), child_mpi, ); - return; // don't bother finding other problems. + return; // Don't bother finding other problems. } } } - /// Currently MoveData does not store entries for all places in + /// Currently, `MoveData` does not store entries for all places in /// the input MIR. For example it will currently filter out - /// places that are Copy; thus we do not track places of shared + /// places that are `Copy`; thus we do not track places of shared /// reference type. This routine will walk up a place along its /// prefixes, searching for a foundational place that *is* - /// tracked in the MoveData. + /// tracked in the `MoveData`. /// - /// An Err result includes a tag indicated why the search failed. + /// An `Err` result includes a tag indicated why the search failed. /// Currently this can only occur if the place is built off of a - /// static variable, as we do not track those in the MoveData. + /// static variable, as we do not track those in the `MoveData`. fn move_path_closest_to( &mut self, place: PlaceRef<'cx, 'tcx>, @@ -1733,13 +1732,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } match last_prefix { - PlaceBase::Local(_) => panic!("should have move path for every Local"), + PlaceBase::Local(_) => panic!("should have move path for every `Local`"), PlaceBase::Static(_) => Err(NoMovePathFound::ReachedStatic), } } fn move_path_for_place(&mut self, place: PlaceRef<'cx, 'tcx>) -> Option { - // If returns None, then there is no move path corresponding + // If returns `None`, then there is no move path corresponding // to a direct owner of `place` (which means there is nothing // that borrowck tracks for its analysis). @@ -1756,23 +1755,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { flow_state: &Flows<'cx, 'tcx>, ) { debug!("check_if_assigned_path_is_moved place: {:?}", place); - // recur down place; dispatch to external checks when necessary + // Recurse down place; dispatch to external checks when necessary. let mut place_projection = &place.projection; // None case => assigning to `x` does not require `x` be initialized. while let Some(proj) = place_projection { let Projection { ref base, ref elem } = **proj; match *elem { - ProjectionElem::Index(_/*operand*/) | + ProjectionElem::Index(_ /* operand */) | ProjectionElem::ConstantIndex { .. } | - // assigning to P[i] requires P to be valid. - ProjectionElem::Downcast(_/*adt_def*/, _/*variant_idx*/) => - // assigning to (P->variant) is okay if assigning to `P` is okay + // Assigning to `P[i]` requires `P` to be valid. + ProjectionElem::Downcast(_ /* adt_def */, _ /* variant_idx */) => + // Assigning to (P->variant) is okay if assigning to `P` is okay. // - // FIXME: is this true even if P is a adt with a dtor? + // FIXME: is this true even if `P` is an ADT with a dtor? { } - // assigning to (*P) requires P to be initialized + // Assigning to `*P` requires `P` to be initialized. ProjectionElem::Deref => { self.check_if_full_path_is_moved( location, InitializationRequiringAction::Use, @@ -1780,8 +1779,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { base: &place.base, projection: base, }, span), flow_state); - // (base initialized; no need to - // recur further) + // (Base initialized; no need to recurse further.) break; } @@ -1791,9 +1789,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } ProjectionElem::Field(..) => { - // if type of `P` has a dtor, then - // assigning to `P.f` requires `P` itself - // be already initialized + // If type of `P` has a dtor, then assigning to `P.f` requires `P` itself + // be already initialized. let tcx = self.infcx.tcx; let base_ty = Place::ty_from(&place.base, base, self.body, tcx).ty; match base_ty.sty { @@ -1805,13 +1802,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { projection: base, }, span), flow_state); - // (base initialized; no need to - // recur further) + // (Base initialized; no need to recurse further.) break; } - // Once `let s; s.x = V; read(s.x);`, - // is allowed, remove this match arm. + // Once `let s; s.x = V; read(s.x);`, is allowed, remove this match arm. ty::Adt(..) | ty::Tuple(..) => { check_parent_of_field(self, location, PlaceRef { base: &place.base, @@ -1819,14 +1814,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { }, span, flow_state); if let PlaceBase::Local(local) = place.base { - // rust-lang/rust#21232, - // #54499, #54986: during - // period where we reject - // partial initialization, do - // not complain about - // unnecessary `mut` on an - // attempt to do a partial - // initialization. + // rust-lang/rust#21232, #54499, #54986: during period where we + // reject partial initialization, do not complain about unnecessary + // `mut` on an attempt to do a partial initialization. self.used_mut.insert(local); } } @@ -1869,7 +1859,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // // `let mut s = ...; drop(s.x); s.x=Val;` // - // This does not use check_if_full_path_is_moved on + // This does not use `check_if_full_path_is_moved` on // `base`, because that would report an error about the // `base` as a whole, but in this scenario we *really* // want to report an error about the actual thing that was @@ -1880,7 +1870,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let maybe_uninits = &flow_state.uninits; // Find the shortest uninitialized prefix you can reach - // without going over a Deref. + // without going over a `Deref`. let mut shortest_uninit_seen = None; for prefix in this.prefixes(base, PrefixSet::Shallow) { let mpi = match this.move_path_for_place(prefix) { @@ -1901,7 +1891,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // after a move out). In this case, do not report a error here. There is an // exception, if this is the first assignment into the union (that is, there is // no move out from an earlier location) then this is an attempt at initialization - // of the union - we should error in that case. + // of the union -- we should error in that case. let tcx = this.infcx.tcx; if let ty::Adt(def, _) = Place::ty_from(base.base, base.projection, this.body, tcx).ty.sty @@ -1927,7 +1917,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - /// Checks the permissions for the given place and read or write kind + /// Checks the permissions for the given place and read or write kind. /// /// Returns `true` if an error is reported. fn check_access_permissions( @@ -2007,7 +1997,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // code path should be unreachable (and thus // we signal an ICE in the else branch // here). But we can legitimately get here - // under borrowck=migrate mode, so instead of + // under `borrowck=migrate` mode, so instead of // ICE'ing we instead report a legitimate // error (which will then be downgraded to a // warning by the migrate machinery). @@ -2036,7 +2026,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { return false; } Activation(..) => { - // permission checks are done at Reservation point. + // Permission checks are done at Reservation point. return false; } Read(ReadKind::Borrow(BorrowKind::Unique)) @@ -2044,12 +2034,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { | Read(ReadKind::Borrow(BorrowKind::Shared)) | Read(ReadKind::Borrow(BorrowKind::Shallow)) | Read(ReadKind::Copy) => { - // Access authorized + // Access authorized. return false; } } - // at this point, we have set up the error reporting state. + // At this point, we have set up the error reporting state. return if previously_initialized { self.report_mutability_error( place, @@ -2079,7 +2069,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { None } - /// Adds the place into the used mutable variables set + /// Adds the place into the used mutable variables set. fn add_used_mut<'d>(&mut self, root_place: RootPlace<'d, 'tcx>, flow_state: &Flows<'cx, 'tcx>) { match root_place { RootPlace { @@ -2121,7 +2111,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - /// Whether this value can be written or borrowed mutably. + /// Checks whether this value can be written or borrowed mutably. /// Returns the root place if the place passed in is a projection. fn is_mutable<'d>( &self, @@ -2196,14 +2186,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let base_ty = Place::ty_from(place.base, &proj.base, self.body, self.infcx.tcx).ty; - // Check the kind of deref to decide + // Check the kind of deref to decide. match base_ty.sty { ty::Ref(_, _, mutbl) => { match mutbl { - // Shared borrowed data is never mutable + // Shared borrowed data is never mutable. hir::MutImmutable => Err(place), // Mutably borrowed data is mutable, but only if we have a - // unique path to the `&mut` + // unique path to the `&mut`. hir::MutMutable => { let mode = match self.is_upvar_field_projection(place) { Some(field) @@ -2223,7 +2213,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } ty::RawPtr(tnm) => { match tnm.mutbl { - // `*const` raw pointers are not mutable + // `*const` raw pointers are not mutable. hir::MutImmutable => Err(place), // `*mut` raw pointers are always mutable, regardless of // context. The users have to check by themselves. @@ -2236,19 +2226,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } } - // `Box` owns its content, so mutable if its location is mutable + // `Box` owns its content, so mutable if its location is mutable. _ if base_ty.is_box() => { self.is_mutable(PlaceRef { base: place.base, projection: &proj.base, }, is_local_mutation_allowed) } - // Deref should only be for reference, pointers or boxes + // Deref should only be for reference, pointers or boxes. _ => bug!("Deref of unexpected type: {:?}", base_ty), } } // All other projections are owned by their base path, so mutable if - // base path is mutable + // base path is mutable. ProjectionElem::Field(..) | ProjectionElem::Index(..) | ProjectionElem::ConstantIndex { .. } @@ -2269,22 +2259,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } (Mutability::Not, LocalMutationIsAllowed::Yes) | (Mutability::Mut, _) => { - // Subtle: this is an upvar - // reference, so it looks like - // `self.foo` -- we want to double - // check that the location `*self` - // is mutable (i.e., this is not a - // `Fn` closure). But if that - // check succeeds, we want to - // *blame* the mutability on - // `place` (that is, - // `self.foo`). This is used to - // propagate the info about - // whether mutability declarations - // are used outwards, so that we register - // the outer variable as mutable. Otherwise a - // test like this fails to record the `mut` - // as needed: + // Subtle: this is an upvar reference, so it looks like + // `self.foo` -- we want to double-check that the location + // `*self` is mutable (i.e., this is not a `Fn` closure). + // But if that check succeeds, we want to *blame* the mutability + // on `place` (that is, `self.foo`). This is used to propagate + // the info about whether mutability declarations are used + // outwards, so that we register the outer variable as mutable. + // Otherwise, a test like this fails to record the `mut` as + // needed: // // ``` // fn foo(_f: F) { } @@ -2360,16 +2343,16 @@ enum NoMovePathFound { ReachedStatic, } -/// The degree of overlap between 2 places for borrow-checking. +/// The degree of overlap between two places for borrow-checking. enum Overlap { - /// The places might partially overlap - in this case, we give + /// The places might partially overlap -- in this case, we give /// up and say that they might conflict. This occurs when /// different fields of a union are borrowed. For example, /// if `u` is a union, we have no way of telling how disjoint /// `u.a.x` and `a.b.y` are. Arbitrary, /// The places have the same type, and are either completely disjoint - /// or equal - i.e., they can't "partially" overlap as can occur with + /// or equal -- i.e., they can't "partially" overlap as can occur with /// unions. This is the "base case" on which we recur for extensions /// of the place. EqualOrDisjoint, diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index 0d13db2f5a413..a3bff3217a8fc 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -109,9 +109,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // whether or not the right-hand side is a place expression if let Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { opt_match_place: Some((ref opt_match_place, match_span)), - binding_mode: _, - opt_ty_info: _, - pat_span: _, + .. }))) = local_decl.is_user_variable { let stmt_source_info = self.body.source_info(location); diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 8f2ce80aafa22..bc839f63950c7 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -1,3 +1,7 @@ +use crate::borrow_check::MirBorrowckCtxt; +use crate::borrow_check::error_reporting::BorrowedContentSource; +use crate::util::collect_writes::FindAssignments; + use rustc::hir; use rustc::hir::Node; use rustc::mir::{self, BindingForm, ClearCrossCrate, Local, Location, Body}; @@ -6,14 +10,10 @@ use rustc::mir::{ }; use rustc::ty::{self, Ty, TyCtxt}; use rustc_data_structures::indexed_vec::Idx; +use rustc_errors::Applicability; use syntax_pos::Span; use syntax_pos::symbol::kw; -use crate::borrow_check::MirBorrowckCtxt; -use crate::borrow_check::error_reporting::BorrowedContentSource; -use crate::util::collect_writes::FindAssignments; -use rustc_errors::Applicability; - #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub(super) enum AccessKind { MutableBorrow, diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs index aba3ef1cbbfc9..47ca6abbeb9c2 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -260,7 +260,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let later_use = self.later_use_kind(borrow, spans, location); BorrowExplanation::UsedLaterInLoop(later_use.0, later_use.1) } else { - // Check if the location represents a `FakeRead`, and adapt the error + // Check if the location represents a FakeRead, and adapt the error // message to the `FakeReadCause` it is from: in particular, // the ones inserted in optimized `let var = ` patterns. let later_use = self.later_use_kind(borrow, spans, location); diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs index 71106af767064..08d2c7ed2b8a5 100644 --- a/src/librustc_mir/borrow_check/nll/invalidation.rs +++ b/src/librustc_mir/borrow_check/nll/invalidation.rs @@ -117,12 +117,11 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, input); } } - StatementKind::Nop | StatementKind::AscribeUserType(..) | StatementKind::Retag { .. } | - StatementKind::StorageLive(..) => { - // `Nop`, `AscribeUserType`, `Retag`, and `StorageLive` are irrelevant - // to borrow check. + StatementKind::StorageLive(..) | + StatementKind::Nop => { + // Irrelevant to borrow check. } StatementKind::StorageDead(local) => { self.access_place( @@ -481,7 +480,7 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> { for &borrow_index in self.borrow_set.activations_at_location(location) { let borrow = &self.borrow_set[borrow_index]; - // only mutable borrows should be 2-phase + // only mutable borrows should be two-phase assert!(match borrow.kind { BorrowKind::Shared | BorrowKind::Shallow => false, BorrowKind::Unique | BorrowKind::Mut { .. } => true, diff --git a/src/librustc_mir/borrow_check/nll/type_check/liveness/local_use_map.rs b/src/librustc_mir/borrow_check/nll/type_check/liveness/local_use_map.rs index 049d83bb22f1b..69922db25a5c3 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/liveness/local_use_map.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/liveness/local_use_map.rs @@ -5,11 +5,11 @@ use rustc::mir::{Body, Local, Location}; use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use rustc_data_structures::vec_linked_list as vll; -/// A map that cross references each local with the locations where it +/// A map that cross-references each local with the locations where it /// is defined (assigned), used, or dropped. Used during liveness /// computation. /// -/// We keep track only of `Local`s we'll do the liveness analysis later, +/// We only keep track of `Local`s that we later perform liveness analysis on; /// this means that our internal `IndexVec`s will only be sparsely populated. /// In the time-memory trade-off between keeping compact vectors with new /// indexes (and needing to continuously map the `Local` index to its compact diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index da1f64b05151b..73df721033949 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -79,7 +79,7 @@ mod input_output; crate mod liveness; mod relate_tys; -/// Type checks the given `mir` in the context of the inference +/// Type-checks the given `mir` in the context of the inference /// context `infcx`. Returns any region constraints that have yet to /// be proven. This result is includes liveness constraints that /// ensure that regions appearing in the types of all local variables @@ -206,7 +206,7 @@ fn type_check_internal<'a, 'tcx, R>( }; if !errors_reported { - // if verifier failed, don't do further checks to avoid ICEs + // If verifier failed, don't do further checks to avoid ICEs. checker.typeck_mir(body); } @@ -1474,12 +1474,12 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ); } } - StatementKind::FakeRead(..) - | StatementKind::StorageLive(..) - | StatementKind::StorageDead(..) - | StatementKind::InlineAsm { .. } - | StatementKind::Retag { .. } - | StatementKind::Nop => {} + StatementKind::FakeRead(..) | + StatementKind::StorageLive(..) | + StatementKind::StorageDead(..) | + StatementKind::InlineAsm { .. } | + StatementKind::Retag { .. } | + StatementKind::Nop => {} } } diff --git a/src/librustc_mir/borrow_check/path_utils.rs b/src/librustc_mir/borrow_check/path_utils.rs index bac08090817d9..b426a051df90b 100644 --- a/src/librustc_mir/borrow_check/path_utils.rs +++ b/src/librustc_mir/borrow_check/path_utils.rs @@ -76,9 +76,9 @@ pub(super) fn is_active<'tcx>( debug!("is_active(borrow_data={:?}, location={:?})", borrow_data, location); let activation_location = match borrow_data.activation_location { - // If this is not a 2-phase borrow, it is always active. + // If this is not a two-phase borrow, it is always active. TwoPhaseActivation::NotTwoPhase => return true, - // And if the unique 2-phase use is not an activation, then it is *never* active. + // And if the unique two-phase use is not an activation, then it is *never* active. TwoPhaseActivation::NotActivated => return false, // Otherwise, we derive info from the activation point `loc`: TwoPhaseActivation::ActivatedAt(loc) => loc, diff --git a/src/librustc_mir/borrow_check/places_conflict.rs b/src/librustc_mir/borrow_check/places_conflict.rs index 4f469174b392d..178a63ec47da2 100644 --- a/src/librustc_mir/borrow_check/places_conflict.rs +++ b/src/librustc_mir/borrow_check/places_conflict.rs @@ -249,12 +249,12 @@ fn place_components_conflict<'tcx>( return false; } (ProjectionElem::Deref, ty::Ref(_, _, hir::MutImmutable), _) => { - // Shouldn't be tracked + // Shouldn't be tracked. bug!("Tracking borrow behind shared reference."); } (ProjectionElem::Deref, ty::Ref(_, _, hir::MutMutable), AccessDepth::Drop) => { // Values behind a mutable reference are not access either by dropping a - // value, or by StorageDead + // value, or by StorageDead. debug!("borrow_conflicts_with_place: drop access behind ptr"); return false; } diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 7ea08b15b443d..f73de43536d7d 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -23,10 +23,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { safety_mode } = self.hir.mirror(ast_block); - self.in_opt_scope(opt_destruction_scope.map(|de|(de, source_info)), move |this| { + self.in_opt_scope(opt_destruction_scope.map(|de| (de, source_info)), move |this| { this.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| { if targeted_by_break { - // This is a `break`-able block + // This is a `break`-able block. let exit_block = this.cfg.start_new_block(); let block_exit = this.in_breakable_scope( None, exit_block, destination.clone(), |this| { @@ -83,7 +83,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { StmtKind::Expr { scope, expr } => { this.block_context.push(BlockFrame::Statement { ignores_expr_result: true }); unpack!(block = this.in_opt_scope( - opt_destruction_scope.map(|de|(de, source_info)), |this| { + opt_destruction_scope.map(|de| (de, source_info)), |this| { let si = (scope, source_info); this.in_scope(si, LintLevel::Inherited, |this| { let expr = this.hir.mirror(expr); @@ -121,7 +121,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let initializer_span = init.span(); unpack!(block = this.in_opt_scope( - opt_destruction_scope.map(|de|(de, source_info)), |this| { + opt_destruction_scope.map(|de| (de, source_info)), |this| { let scope = (init_scope, source_info); this.in_scope(scope, lint_level, |this| { this.declare_bindings( @@ -165,7 +165,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } let popped = this.block_context.pop(); - assert!(popped.map_or(false, |bf|bf.is_statement())); + assert!(popped.map_or(false, |bf| bf.is_statement())); } // Then, the block may have an optional trailing expression which is a “return” value @@ -180,7 +180,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { unpack!(block = this.into(destination, block, expr)); let popped = this.block_context.pop(); - assert!(popped.map_or(false, |bf|bf.is_tail_expr())); + assert!(popped.map_or(false, |bf| bf.is_tail_expr())); } else { // If a block has no trailing expression, then it is given an implicit return type. // This return type is usually `()`, unless the block is diverging, in which case the @@ -204,10 +204,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.unit() } - /// If we are changing the safety mode, create a new source scope + /// If we are changing the safety mode, creates a new source scope. fn update_source_scope_for_safety_mode(&mut self, - span: Span, - safety_mode: BlockSafety) + span: Span, + safety_mode: BlockSafety) { debug!("update_source_scope_for({:?}, {:?})", span, safety_mode); let new_unsafety = match safety_mode { diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index 1a186fa932ddb..ded0e9d5c1c0c 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -13,7 +13,7 @@ use rustc::ty::{self, CanonicalUserTypeAnnotation, Ty, UpvarSubsts}; use syntax_pos::Span; impl<'a, 'tcx> Builder<'a, 'tcx> { - /// See comment on `as_local_operand` + /// See comment on `as_local_operand`. pub fn as_local_rvalue(&mut self, block: BasicBlock, expr: M) -> BlockAnd> where M: Mirror<'tcx, Output = Expr<'tcx>>, @@ -22,7 +22,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.as_rvalue(block, local_scope, expr) } - /// Compile `expr`, yielding an rvalue. + /// Compiles `expr`, yielding an rvalue. pub fn as_rvalue( &mut self, block: BasicBlock, @@ -83,7 +83,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } ExprKind::Unary { op, arg } => { let arg = unpack!(block = this.as_operand(block, scope, arg)); - // Check for -MIN on signed integers + // Check for `-MIN` on signed integers. if this.hir.check_overflow() && op == UnOp::Neg && expr.ty.is_signed() { let bool_ty = this.hir.bool_ty(); @@ -123,7 +123,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }, ); if let Some(scope) = scope { - // schedule a shallow free of that memory, lest we unwind: + // Schedule a shallow free of that memory, lest we unwind. this.schedule_drop_storage_and_value( expr_span, scope, @@ -132,12 +132,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); } - // malloc some memory of suitable type (thus far, uninitialized): + // Allocate some memory of suitable type (thus far, uninitialized). let box_ = Rvalue::NullaryOp(NullOp::Box, value.ty); this.cfg .push_assign(block, source_info, &Place::from(result), box_); - // initialize the box contents: + // Initialize the box contents. unpack!( block = this.into( &Place::from(result).deref(), @@ -181,7 +181,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // fields. We don't want `let x: ();` to compile // to the same MIR as `let x = ();`. - // first process the set of fields + // First, process the set of fields. let el_ty = expr.ty.sequence_element_type(this.hir.tcx()); let fields: Vec<_> = fields .into_iter() @@ -191,8 +191,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(Rvalue::Aggregate(box AggregateKind::Array(el_ty), fields)) } ExprKind::Tuple { fields } => { - // see (*) above - // first process the set of fields + // See (*) above. + // First, process the set of fields. let fields: Vec<_> = fields .into_iter() .map(|f| unpack!(block = this.as_operand(block, scope, f))) @@ -206,20 +206,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { upvars, movability, } => { - // see (*) above + // See (*) above. let operands: Vec<_> = upvars .into_iter() .map(|upvar| { let upvar = this.hir.mirror(upvar); match Category::of(&upvar.kind) { - // Use as_place to avoid creating a temporary when + // Use `as_place` to avoid creating a temporary when // moving a variable into a closure, so that // borrowck knows which variables to mark as being - // used as mut. This is OK here because the upvar + // used as mut. This is okay here because the upvar // expressions have no side effects and act on // disjoint places. // This occurs when capturing by copy/move, while - // by reference captures use as_operand + // by reference captures use `as_operand`. Some(Category::Place) => { let place = unpack!(block = this.as_place(block, upvar)); this.consume_by_copy_or_move(place) @@ -248,8 +248,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }).collect(); let result = match substs { UpvarSubsts::Generator(substs) => { - // We implicitly set the discriminant to 0. See - // librustc_mir/transform/deaggregator.rs for details. + // We implicitly set the discriminant to `0`. See + // `librustc_mir/transform/deaggregator.rs` for details. let movability = movability.unwrap(); box AggregateKind::Generator(closure_id, substs, movability) } @@ -265,7 +265,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fields, base, } => { - // see (*) above + // See (*) above. let is_union = adt_def.is_union(); let active_field_index = if is_union { Some(fields[0].name.index()) @@ -273,8 +273,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { None }; - // first process the set of fields that were provided - // (evaluating them in order given by user) + // First, process the set of fields that were provided + // (evaluating them in order given by user). let fields_map: FxHashMap<_, _> = fields .into_iter() .map(|f| { @@ -342,28 +342,28 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); resume.and(this.unit_rvalue()) } - ExprKind::Literal { .. } - | ExprKind::Block { .. } - | ExprKind::Match { .. } - | ExprKind::NeverToAny { .. } - | ExprKind::Use { .. } - | ExprKind::Loop { .. } - | ExprKind::LogicalOp { .. } - | ExprKind::Call { .. } - | ExprKind::Field { .. } - | ExprKind::Deref { .. } - | ExprKind::Index { .. } - | ExprKind::VarRef { .. } - | ExprKind::SelfRef - | ExprKind::Break { .. } - | ExprKind::Continue { .. } - | ExprKind::Return { .. } - | ExprKind::InlineAsm { .. } - | ExprKind::StaticRef { .. } - | ExprKind::PlaceTypeAscription { .. } - | ExprKind::ValueTypeAscription { .. } => { - // these do not have corresponding `Rvalue` variants, - // so make an operand and then return that + ExprKind::Literal { .. } | + ExprKind::Block { .. } | + ExprKind::Match { .. } | + ExprKind::NeverToAny { .. } | + ExprKind::Use { .. } | + ExprKind::Loop { .. } | + ExprKind::LogicalOp { .. } | + ExprKind::Call { .. } | + ExprKind::Field { .. } | + ExprKind::Deref { .. } | + ExprKind::Index { .. } | + ExprKind::VarRef { .. } | + ExprKind::SelfRef | + ExprKind::Break { .. } | + ExprKind::Continue { .. } | + ExprKind::Return { .. } | + ExprKind::InlineAsm { .. } | + ExprKind::StaticRef { .. } | + ExprKind::PlaceTypeAscription { .. } | + ExprKind::ValueTypeAscription { .. } => { + // These do not have corresponding `Rvalue` variants, + // so make an operand and then return that. debug_assert!(match Category::of(&expr.kind) { Some(Category::Rvalue(RvalueFunc::AsRvalue)) => false, _ => true, @@ -418,7 +418,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }; let overflow_err = PanicInfo::Overflow(op); - // Check for / 0 + // Check for division by zero. let is_zero = self.temp(bool_ty, span); let zero = self.zero_literal(span, ty); self.cfg.push_assign( @@ -431,7 +431,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block = self.assert(block, Operand::Move(is_zero), false, zero_err, span); // We only need to check for the overflow in one case: - // MIN / -1, and only for signed values. + // `MIN / -1`, and only for signed values. if ty.is_signed() { let neg_1 = self.neg_1_literal(span, ty); let min = self.minval_literal(span, ty); @@ -440,7 +440,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let is_min = self.temp(bool_ty, span); let of = self.temp(bool_ty, span); - // this does (rhs == -1) & (lhs == MIN). It could short-circuit instead + // This does `(rhs == -1) & (lhs == MIN)`. It could short-circuit instead. self.cfg.push_assign( block, @@ -545,7 +545,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }, "Unexpected capture place" ); - // Not in a closure + // Not in a closure. debug_assert!( this.upvar_mutbls.len() > upvar_index.index(), "Unexpected capture place" @@ -569,7 +569,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Rvalue::Ref(this.hir.tcx().lifetimes.re_erased, borrow_kind, arg_place), ); - // In constants, temp_lifetime is None. We should not need to drop + // In constants, `temp_lifetime` is `None`. We should not need to drop // anything because no values with a destructor can be created in // a constant at this time, even if the type may need dropping. if let Some(temp_lifetime) = temp_lifetime { @@ -584,7 +584,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(Operand::Move(Place::from(temp))) } - // Helper to get a `-1` value of the appropriate type + // Helper to get a `-1` value of the appropriate type. fn neg_1_literal(&mut self, span: Span, ty: Ty<'tcx>) -> Operand<'tcx> { let param_ty = ty::ParamEnv::empty().and(ty); let bits = self.hir.tcx().layout_of(param_ty).unwrap().size.bits(); @@ -594,7 +594,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.literal_operand(span, literal) } - // Helper to get the minimum value of the appropriate type + // Helper to get the minimum value of the appropriate type. fn minval_literal(&mut self, span: Span, ty: Ty<'tcx>) -> Operand<'tcx> { assert!(ty.is_signed()); let param_ty = ty::ParamEnv::empty().and(ty); diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 889861b856748..d48fbe25cbd93 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -1,4 +1,4 @@ -//! See docs in build/expr/mod.rs +//! See docs in `build/expr/mod.rs`. use crate::build::expr::category::{Category, RvalueFunc}; use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; @@ -9,7 +9,7 @@ use rustc::ty; use rustc_target::spec::abi::Abi; impl<'a, 'tcx> Builder<'a, 'tcx> { - /// Compile `expr`, storing the result into `destination`, which + /// Compiles `expr`, storing the result into `destination`, which /// is assumed to be uninitialized. pub fn into_expr( &mut self, @@ -22,9 +22,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { destination, block, expr ); - // since we frequently have to reference `self` from within a + // Since we frequently have to reference `self` from within a // closure, where `self` would be shadowed, it's easier to - // just use the name `this` uniformly + // just use the name `this` uniformly. let this = self; let expr_span = expr.span; let source_info = this.source_info(expr_span); @@ -269,19 +269,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.into(destination, block, source) } - // These cases don't actually need a destination - ExprKind::Assign { .. } - | ExprKind::AssignOp { .. } - | ExprKind::Continue { .. } - | ExprKind::Break { .. } - | ExprKind::InlineAsm { .. } - | ExprKind::Return { .. } => { + // These cases don't actually need a destination. + ExprKind::Assign { .. } | + ExprKind::AssignOp { .. } | + ExprKind::Continue { .. } | + ExprKind::Break { .. } | + ExprKind::InlineAsm { .. } | + ExprKind::Return { .. } => { unpack!(block = this.stmt_expr(block, expr, None)); this.cfg.push_assign_unit(block, source_info, destination); block.unit() } - // Avoid creating a temporary + // Avoid creating a temporary. ExprKind::VarRef { .. } | ExprKind::SelfRef | ExprKind::StaticRef { .. } | @@ -315,7 +315,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.unit() } - // these are the cases that are more naturally handled by some other mode + // These are the cases that are more naturally handled by some other mode. ExprKind::Unary { .. } | ExprKind::Binary { .. } | ExprKind::Box { .. } @@ -330,12 +330,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Literal { .. } | ExprKind::Yield { .. } => { debug_assert!(match Category::of(&expr.kind).unwrap() { - // should be handled above + // Should be handled above. Category::Rvalue(RvalueFunc::Into) => false, - // must be handled above or else we get an + // Must be handled above or else we get an // infinite loop in the builder; see - // e.g., `ExprKind::VarRef` above + // e.g., `ExprKind::VarRef` above. Category::Place => false, _ => true, diff --git a/src/librustc_mir/build/expr/stmt.rs b/src/librustc_mir/build/expr/stmt.rs index cf3d8778da193..78b85c0e06209 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -40,7 +40,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Note: we evaluate assignments right-to-left. This // is better for borrowck interaction with overloaded - // operators like x[j] = x[i]. + // operators like `x[j] = x[i]`. debug!("stmt_expr Assign block_context.push(SubExpr) : {:?}", expr2); this.block_context.push(BlockFrame::SubExpr); @@ -144,7 +144,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { _ => { assert!( statement_scope.is_some(), - "Should not be calling `stmt_expr` on a general expression \ + "should not be calling `stmt_expr` on a general expression \ without a statement scope", ); diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 94323b15b696f..968282fd1aae6 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -10,6 +10,7 @@ use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; use crate::build::{GuardFrame, GuardFrameLocal, LocalsForNode}; use crate::hair::{self, *}; + use rustc::hir::HirId; use rustc::mir::*; use rustc::middle::region; @@ -20,13 +21,13 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use syntax::ast::Name; use syntax_pos::Span; -// helper functions, broken out by category: +use std::convert::TryFrom; + +// Helper functions, broken out by category: mod simplify; mod test; mod util; -use std::convert::TryFrom; - impl<'a, 'tcx> Builder<'a, 'tcx> { /// Generates MIR for a `match` expression. /// @@ -141,7 +142,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Step 2. Create the otherwise and prebinding blocks. - // create binding start block for link them by false edges + // Create binding start block for link them by false edges. let candidate_count = arms.iter().map(|c| c.patterns.len()).sum::(); let pre_binding_blocks: Vec<_> = (0..candidate_count) .map(|_| self.cfg.start_new_block()) @@ -208,8 +209,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let outer_source_info = self.source_info(span); - // this will generate code to test scrutinee_place and - // branch to the appropriate arm block + // This will generate code to test `scrutinee_place` and + // branch to the appropriate arm block. self.match_candidates( scrutinee_span, &mut Some(block), @@ -228,6 +229,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }; // Step 5. Create everything else: the guards and the arms. + let match_scope = self.scopes.topmost(); let arm_end_blocks: Vec<_> = arm_candidates.into_iter().map(|(arm, mut candidates)| { @@ -279,7 +281,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }) }).collect(); - // all the arm blocks will rejoin here + // All the arm blocks will rejoin here. let end_block = self.cfg.start_new_block(); for arm_block in arm_end_blocks { @@ -302,7 +304,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { initializer: ExprRef<'tcx>, ) -> BlockAnd<()> { match *irrefutable_pat.kind { - // Optimize the case of `let x = ...` to write directly into `x` + // Optimize the case of `let x = ...` to write directly into `x`. PatternKind::Binding { mode: BindingMode::ByValue, var, @@ -313,7 +315,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard); unpack!(block = self.into(&place, block, initializer)); - // Inject a fake read, see comments on `FakeReadCause::ForLet`. let source_info = self.source_info(irrefutable_pat.span); self.cfg.push( @@ -416,14 +417,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { initializer: &Place<'tcx>, set_match_place: bool, ) -> BlockAnd<()> { - // create a dummy candidate + // Create a dummy candidate. let mut candidate = Candidate { span: irrefutable_pat.span, match_pairs: vec![MatchPair::new(initializer.clone(), &irrefutable_pat)], bindings: vec![], ascriptions: vec![], - // since we don't call `match_candidates`, next fields are unused + // Since we don't call `match_candidates`, next fields are unused. otherwise_block: None, pre_binding_block: block, next_candidate_pre_binding_block: None, @@ -448,10 +449,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); } - // for matches and function arguments, the place that is being matched + // For matches and function arguments, the place that is being matched // can be set when creating the variables. But the place for - // let PATTERN = ... might not even exist until we do the assignment. - // so we set it here instead + // `let PATTERN = ...` might not even exist until we do the assignment. + // so we set it here instead. if set_match_place { for binding in &candidate.bindings { let local = self.var_local_id(binding.var_id, OutsideGuard); @@ -463,14 +464,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { { *match_place = Some(initializer.clone()); } else { - bug!("Let binding to non-user variable.") + bug!("let-binding to non-user variable.") } } } self.ascribe_types(block, &candidate.ascriptions); - // now apply the bindings, which will also declare the variables + // Now apply the bindings, which will also declare the variables. self.bind_matched_candidate_for_arm_body(block, &candidate.bindings); block.unit() @@ -478,8 +479,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// Declares the bindings of the given patterns and returns the visibility /// scope for the bindings in these patterns, if such a scope had to be - /// created. NOTE: Declaring the bindings should always be done in their - /// drop scope. + /// created. + /// NOTE: Declaring the bindings should always be done in their drop scope. pub fn declare_bindings( &mut self, mut visibility_scope: Option, @@ -668,22 +669,22 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { #[derive(Debug)] pub struct Candidate<'pat, 'tcx> { - // span of the original pattern that gave rise to this candidate + // The span of the original pattern that gave rise to this candidate. span: Span, - // all of these must be satisfied... + // All of these must be satisfied... match_pairs: Vec>, - // ...these bindings established... + // ... these bindings established... bindings: Vec>, - // ...and these types asserted... + // ... and these types asserted... ascriptions: Vec>, - // ...and the guard must be evaluated, if false branch to Block... + // ... and the guard must be evaluated, if false branch to `Block`... otherwise_block: Option, - // ...and the blocks for add false edges between candidates + // ... and the blocks for add false edges between candidates. pre_binding_block: BasicBlock, next_candidate_pre_binding_block: Option, } @@ -712,7 +713,7 @@ struct Ascription<'tcx> { #[derive(Clone, Debug)] pub struct MatchPair<'pat, 'tcx> { - // this place... + // This place... place: Place<'tcx>, // ... must match this pattern. @@ -773,8 +774,7 @@ pub struct Test<'tcx> { kind: TestKind<'tcx>, } -/// ArmHasGuard is isomorphic to a boolean flag. It indicates whether -/// a match arm has a guard expression attached to it. +/// Indicates whether a match arm has a guard expression attached to it. #[derive(Copy, Clone, Debug)] pub(crate) struct ArmHasGuard(pub bool); @@ -893,7 +893,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); } - /// Link up matched candidates. For example, if we have something like + /// Links up matched candidates. For example, if we have something like /// this: /// /// ... @@ -1148,15 +1148,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { mut otherwise_block: Option, fake_borrows: &mut Option>>, ) { - // extract the match-pair from the highest priority candidate + // Extract the match-pair from the highest priority candidate. let match_pair = &candidates.first().unwrap().match_pairs[0]; let mut test = self.test(match_pair); let match_place = match_pair.place.clone(); - // most of the time, the test to perform is simply a function + // Most of the time, the test to perform is simply a function // of the main candidate; but for a test like SwitchInt, we // may want to add cases based on the candidates that are - // available + // available. match test.kind { TestKind::SwitchInt { switch_ty, @@ -1188,12 +1188,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { _ => {} } - // Insert a Shallow borrow of any places that is switched on. + // Insert a Shallow borrow of any places that are switched on. fake_borrows.as_mut().map(|fb| { fb.insert(match_place.clone()) }); - // perform the test, branching to one of N blocks. For each of + // Perform the test, branching to one of N blocks. For each of // those N possible outcomes, create a (initially empty) // vector of candidates. Those are the candidates that still // apply if the test has that particular outcome. @@ -1219,7 +1219,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { break; } } - // at least the first candidate ought to be tested + // At least the first candidate ought to be tested. assert!(total_candidate_count > candidates.len()); debug!("tested_candidates: {}", total_candidate_count - candidates.len()); debug!("untested_candidates: {}", candidates.len()); @@ -1227,7 +1227,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // HACK(matthewjasper) This is a closure so that we can let the test // create its blocks before the rest of the match. This currently // improves the speed of llvm when optimizing long string literal - // matches + // matches. let make_target_blocks = move |this: &mut Self| -> Vec { // For each outcome of test, process the candidates that still // apply. Collect a list of blocks where control flow will @@ -1279,7 +1279,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); } - // Determine the fake borrows that are needed to ensure that the place + // Determines the fake borrows that are needed to ensure that the place // will evaluate to the same thing until an arm has been chosen. fn calculate_fake_borrows<'b>( &mut self, @@ -1594,7 +1594,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } - /// Append `AscribeUserType` statements onto the end of `block` + /// Appends `AscribeUserType` statements onto the end of `block` /// for each ascription fn ascribe_types(&mut self, block: BasicBlock, ascriptions: &[Ascription<'tcx>]) { for ascription in ascriptions { @@ -1739,8 +1739,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { is_block_tail: None, is_user_variable: Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { binding_mode, - // hypothetically, `visit_bindings` could try to unzip - // an outermost hir::Ty as we descend, matching up + // Hypothetically, `visit_bindings` could try to unzip + // an outermost `hir::Ty` as we descend, matching up // idents in pat; but complex w/ unclear UI payoff. // Instead, just abandon providing diagnostic info. opt_ty_info: None, @@ -1752,7 +1752,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let locals = if has_guard.0 { let ref_for_guard = self.local_decls.push(LocalDecl::<'tcx> { // This variable isn't mutated but has a name, so has to be - // immutable to avoid the unused mut lint. + // immutable to avoid the "unused mut" lint. mutability: Mutability::Not, ty: tcx.mk_imm_ref(tcx.lifetimes.re_erased, var_ty), user_ty: UserTypeProjections::none(), diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 7ab0bf7d66a64..679bbc555adda 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -1,9 +1,12 @@ +use super::lints; + use crate::build; use crate::build::scope::DropKind; use crate::hair::cx::Cx; use crate::hair::{LintLevel, BindingMode, PatternKind}; use crate::transform::MirSource; use crate::util as mir_util; + use rustc::hir; use rustc::hir::Node; use rustc::hir::def_id::DefId; @@ -19,9 +22,7 @@ use syntax::attr::{self, UnwindAttr}; use syntax::symbol::kw; use syntax_pos::Span; -use super::lints; - -/// Construct the MIR for a given `DefId`. +/// Constructs the MIR for a given `DefId`. pub fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> Body<'_> { let id = tcx.hir().as_local_hir_id(def_id).unwrap(); @@ -780,7 +781,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ast_body: &'tcx hir::Expr) -> BlockAnd<()> { - // Allocate locals for the function arguments + // Allocate locals for the function arguments. for &ArgInfo(ty, _, arg_opt, _) in arguments.iter() { // If this is a simple binding pattern, give the local a name for // debuginfo and so that error reporting knows that this is a user @@ -807,9 +808,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } let mut scope = None; - // Bind the argument patterns + // Bind the argument patterns. for (index, arg_info) in arguments.iter().enumerate() { - // Function arguments always get the first Local indices after the return place + // Function arguments always get the first Local indices after the return place. let local = Local::new(index + 1); let place = Place::from(local); let &ArgInfo(ty, opt_ty_info, arg_opt, ref self_binding) = arg_info; @@ -826,7 +827,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let span = pattern.span; self.set_correct_source_scope_for_arg(arg.hir_id, original_source_scope, span); match *pattern.kind { - // Don't introduce extra copies for simple bindings + // Don't introduce extra copies for simple bindings. PatternKind::Binding { mutability, var, diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index a04c041ca9dc7..8021de2349a23 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -1,4 +1,5 @@ /*! + Managing the scope stack. The scopes are tied to lexical scopes, so as we descend the HAIR, we push a scope on the stack, build its contents, and then pop it off. Every scope is named by a @@ -19,7 +20,7 @@ paragraph). This is because region scopes are tied to them. Eventually, when we shift to non-lexical lifetimes, there should be no need to remember this mapping. -### Not so SEME Regions +### Not-so-SEME Regions In the course of building matches, it sometimes happens that certain code (namely guards) gets executed multiple times. This means that the scope lexical @@ -98,32 +99,32 @@ struct Scope { /// The source scope this scope was created in. source_scope: SourceScope, - /// the region span of this scope within source code. + /// The region span of this scope within source code. region_scope: region::Scope, - /// the span of that region_scope + /// The span of `region_scope`. region_scope_span: Span, /// Whether there's anything to do for the cleanup path, that is, /// when unwinding through this scope. This includes destructors, /// but not StorageDead statements, which don't get emitted at all /// for unwinding, for several reasons: - /// * clang doesn't emit llvm.lifetime.end for C++ unwinding + /// * clang doesn't emit `llvm.lifetime.end` for C++ unwinding /// * LLVM's memory dependency analysis can't handle it atm /// * polluting the cleanup MIR with StorageDead creates /// landing pads even though there's no actual destructors /// * freeing up stack space has no effect during unwinding - /// Note that for generators we do emit StorageDeads, for the + /// Note that for generators we do emit StorageDead's, for the /// use of optimizations in the MIR generator transform. needs_cleanup: bool, - /// set of places to drop when exiting this scope. This starts + /// The set of places to drop when exiting this scope. This starts /// out empty but grows as variables are declared during the /// building process. This is a stack, so we always drop from the /// end of the vector (top of the stack) first. drops: Vec, - /// The cache for drop chain on “normal” exit into a particular BasicBlock. + /// The cache for drop chain on “normal” exit into a particular `BasicBlock`. cached_exits: FxHashMap<(BasicBlock, region::Scope), BasicBlock>, /// The cache for drop chain on "generator drop" exit. @@ -142,10 +143,10 @@ pub struct Scopes<'tcx> { #[derive(Debug)] struct DropData { - /// span where drop obligation was incurred (typically where place was declared) + /// The span where the drop obligation was incurred (typically where the place was declared). span: Span, - /// local to drop + /// The local to drop. local: Local, /// Whether this is a value Drop or a StorageDead. @@ -180,19 +181,19 @@ pub(crate) enum DropKind { #[derive(Clone, Debug)] struct BreakableScope<'tcx> { - /// Region scope of the loop + /// The region scope of the loop. region_scope: region::Scope, - /// Where the body of the loop begins. `None` if block + /// Where the body of the loop begins. `None` if a block. continue_block: Option, - /// Block to branch into when the loop or block terminates (either by being `break`-en out - /// from, or by having its condition to become false) + /// The block to branch into when the loop or block terminates (either by being exited via + /// `break`, or by having its condition become false). break_block: BasicBlock, /// The destination of the loop/block expression itself (i.e., where to put the result of a - /// `break` expression) + /// `break` expression). break_destination: Place<'tcx>, } -/// The target of an expression that breaks out of a scope +/// The target of an expression that breaks out of a scope. #[derive(Clone, Copy, Debug)] pub enum BreakableTarget { Continue(region::Scope), @@ -229,17 +230,17 @@ impl Scope { /// Should always be run for all inner scopes when a drop is pushed into some scope enclosing a /// larger extent of code. /// - /// `storage_only` controls whether to invalidate only drop paths that run `StorageDead`. + /// `storage_only` controls whether to invalidate only drop paths that run StorageDead. /// `this_scope_only` controls whether to invalidate only drop paths that refer to the current /// top-of-scope (as opposed to dependent scopes). fn invalidate_cache(&mut self, storage_only: bool, is_generator: bool, this_scope_only: bool) { // FIXME: maybe do shared caching of `cached_exits` etc. to handle functions // with lots of `try!`? - // cached exits drop storage and refer to the top-of-scope + // Cached exits drop storage and refer to the top-of-scope. self.cached_exits.clear(); - // the current generator drop and unwind refer to top-of-scope + // The current generator drop and unwind refer to top-of-scope. self.cached_generator_drop = None; let ignore_unwinds = storage_only && !is_generator; @@ -254,7 +255,7 @@ impl Scope { } } - /// Given a span and this scope's source scope, make a SourceInfo. + /// Given a span and this scope's source scope, makes a `SourceInfo`. fn source_info(&self, span: Span) -> SourceInfo { SourceInfo { span, @@ -306,7 +307,7 @@ impl<'tcx> Scopes<'tcx> { target: BreakableTarget, ) -> (BasicBlock, region::Scope, Option>) { let get_scope = |scope: region::Scope| { - // find the loop-scope by its `region::Scope`. + // Find the loop-scope by its `region::Scope`. self.breakable_scopes.iter() .rfind(|breakable_scope| breakable_scope.region_scope == scope) .unwrap_or_else(|| span_bug!(span, "no enclosing breakable scope found")) @@ -449,7 +450,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(rv) } - /// Push a scope onto the stack. You can then build code in this + /// Pushes a scope onto the stack. You can then build code in this /// scope and call `pop_scope` afterwards. Note that these two /// calls must be paired; using `in_scope` as a convenience /// wrapper maybe preferable. @@ -518,7 +519,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.cfg.start_new_block().unit() } - /// Branch out of `block` to `target`, exiting all scopes up to + /// Branches out of `block` to `target`, exiting all scopes up to /// and including `region_scope`. This will insert whatever drops are /// needed. See module comment for details. pub fn exit_scope(&mut self, @@ -658,7 +659,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { scope } - /// Given a span and the current source scope, make a SourceInfo. + /// Given a span and the current source scope, makes a `SourceInfo`. pub fn source_info(&self, span: Span) -> SourceInfo { SourceInfo { span, @@ -668,6 +669,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Finding scopes // ============== + /// Returns the scope that we should use as the lifetime of an /// operand. Basically, an operand must live until it is consumed. /// This is similar to, but not quite the same as, the temporary @@ -713,6 +715,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Scheduling drops // ================ + pub fn schedule_drop_storage_and_value( &mut self, span: Span, @@ -824,7 +827,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Other // ===== - /// Branch based on a boolean condition. + + /// Branches based on a boolean condition. /// /// This is a special case because the temporary for the condition needs to /// be dropped on both the true and the false arm. @@ -846,7 +850,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.cfg.terminate(block, source_info, term); match cond { - // Don't try to drop a constant + // Don't try to drop a constant. Operand::Constant(_) => (), // If constants and statics, we don't generate StorageLive for this // temporary, so don't try to generate StorageDead for it either. @@ -865,12 +869,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match top_drop_data.kind { DropKind::Value { .. } => { - bug!("Drop scheduled on top of condition variable") + bug!("drop scheduled on top of condition variable") } DropKind::Storage => { let source_info = top_scope.source_info(top_drop_data.span); let local = top_drop_data.local; - assert_eq!(local, cond_temp, "Drop scheduled on top of condition"); + assert_eq!(local, cond_temp, "drop scheduled on top of condition"); self.cfg.push( true_block, Statement { @@ -890,7 +894,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { top_scope.invalidate_cache(true, self.is_generator, true); } - _ => bug!("Expected as_local_operand to produce a temporary"), + _ => bug!("expected `as_local_operand` to produce a temporary"), } (true_block, false_block) @@ -950,7 +954,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { target } - /// Utility function for *non*-scope code to build their own drops + /// Utility function for *non*-scope code to build their own drops. pub fn build_drop_and_replace(&mut self, block: BasicBlock, span: Span, @@ -997,6 +1001,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // `match` arm scopes // ================== + /// Unschedules any drops in the top scope. /// /// This is only needed for `match` arm scopes, because they have one @@ -1011,7 +1016,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } -/// Builds drops for pop_scope and exit_scope. +/// Builds drops for `pop_scope` and `exit_scope`. fn build_scope_drops<'tcx>( cfg: &mut CFG<'tcx>, is_generator: bool, @@ -1128,7 +1133,7 @@ fn build_diverge_scope<'tcx>(cfg: &mut CFG<'tcx>, let mut target_built_by_us = false; // Build up the drops. Here we iterate the vector in - // *forward* order, so that we generate drops[0] first (right to + // *forward* order, so that we generate `drops[0]` first (right to // left in diagram above). debug!("build_diverge_scope({:?})", scope.drops); for (j, drop_data) in scope.drops.iter_mut().enumerate() { @@ -1152,8 +1157,7 @@ fn build_diverge_scope<'tcx>(cfg: &mut CFG<'tcx>, // block for our StorageDead statements. let block = cfg.start_new_cleanup_block(); let source_info = SourceInfo { span: DUMMY_SP, scope: source_scope }; - cfg.terminate(block, source_info, - TerminatorKind::Goto { target: target }); + cfg.terminate(block, source_info, TerminatorKind::Goto { target: target }); target = block; target_built_by_us = true; } diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index db679024dc5a2..c5920e58efc3a 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -1,4 +1,4 @@ -// Not in interpret to make sure we do not use private implementation details +// Not in `interpret` module to make sure we do not use private implementation details. use std::fmt; use std::error::Error; @@ -9,8 +9,8 @@ use std::convert::TryInto; use rustc::hir::def::DefKind; use rustc::hir::def_id::DefId; -use rustc::mir::interpret::{ConstEvalErr, ErrorHandled, ScalarMaybeUndef}; use rustc::mir; +use rustc::mir::interpret::{ConstEvalErr, ErrorHandled, ScalarMaybeUndef}; use rustc::ty::{self, Ty, TyCtxt, subst::Subst}; use rustc::ty::layout::{self, LayoutOf, VariantIdx}; use rustc::traits::Reveal; @@ -26,7 +26,7 @@ use crate::interpret::{self, snapshot, RefTracking, intern_const_alloc_recursive, }; -/// Number of steps until the detector even starts doing anything. +/// The number of steps until the detector even starts doing anything. /// Also, a warning is shown to the user when this number is reached. const STEPS_UNTIL_DETECTOR_ENABLED: isize = 1_000_000; /// The number of steps between loop detector snapshots. @@ -129,7 +129,7 @@ fn op_to_const<'tcx>( ecx.tcx.mk_const(ty::Const { val, ty: op.layout.ty }) } -// Returns a pointer to where the result lives +// Returns a pointer to where the result lives. fn eval_body_using_ecx<'mir, 'tcx>( ecx: &mut CompileTimeEvalContext<'mir, 'tcx>, cid: GlobalId<'tcx>, @@ -154,10 +154,10 @@ fn eval_body_using_ecx<'mir, 'tcx>( StackPopCleanup::None { cleanup: false }, )?; - // The main interpreter loop. + // Run the main interpreter loop. ecx.run()?; - // Intern the result + // Intern the result. intern_const_alloc_recursive( ecx, cid.instance.def_id(), @@ -208,7 +208,7 @@ impl Error for ConstEvalError { } } -// Extra machine state for CTFE, and the Machine instance +// Extra machine state for CTFE, and the `Machine` instance. pub struct CompileTimeInterpreter<'mir, 'tcx> { /// When this value is negative, it indicates the number of interpreter /// steps *until* the loop detector is enabled. When it is positive, it is @@ -222,7 +222,7 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> { impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> { fn new() -> Self { - CompileTimeInterpreter { + Self { loop_detector: Default::default(), steps_since_detector_enabled: -STEPS_UNTIL_DETECTOR_ENABLED, } @@ -262,13 +262,12 @@ impl interpret::AllocMap for FxHashMap { &self, k: K, vacant: impl FnOnce() -> Result - ) -> Result<&V, E> - { + ) -> Result<&V, E> { match self.get(&k) { Some(v) => Ok(v), None => { vacant()?; - bug!("The CTFE machine shouldn't ever need to extend the alloc_map when reading") + bug!("the CTFE machine shouldn't ever need to extend the `alloc_map` when reading"); } } } @@ -278,8 +277,7 @@ impl interpret::AllocMap for FxHashMap { &mut self, k: K, vacant: impl FnOnce() -> Result - ) -> Result<&mut V, E> - { + ) -> Result<&mut V, E> { match self.entry(k) { Entry::Occupied(e) => Ok(e.into_mut()), Entry::Vacant(e) => { @@ -296,7 +294,7 @@ crate type CompileTimeEvalContext<'mir, 'tcx> = impl interpret::MayLeak for ! { #[inline(always)] fn may_leak(self) -> bool { - // `self` is uninhabited + // `self` is uninhabited. self } } @@ -320,7 +318,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, #[inline(always)] fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { - false // for now, we don't enforce validity + false // For now, we don't enforce validity. } fn find_fn( @@ -331,7 +329,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, ret: Option, ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { debug!("eval_fn_call: {:?}", instance); - // Only check non-glue functions + // Only check non-glue functions. if let ty::InstanceDef::Item(def_id) = instance.def { // Execution might have wandered off into other crates, so we cannot do a stability- // sensitive check here. But we can at least rule out functions that are not const @@ -348,7 +346,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, }; } } - // This is a const fn. Call it. + // This is a const fn; call it. Ok(Some(match ecx.load_mir(instance.def, None) { Ok(body) => body, Err(err) => { @@ -423,7 +421,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, alloc: Cow<'b, Allocation>, _kind: Option>, ) -> (Cow<'b, Allocation>, Self::PointerTag) { - // We do not use a tag so we can just cheaply forward the allocation + // We do not use a tag so that we can just cheaply forward the allocation. (alloc, ()) } @@ -481,7 +479,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, } /// Extracts a field of a (variant of a) const. -// this function uses `unwrap` copiously, because an already validated constant must have valid +// +// This function uses `unwrap` copiously, because an already validated constant must have valid // fields and can thus never fail outside of compiler bugs pub fn const_field<'tcx>( tcx: TyCtxt<'tcx>, @@ -492,22 +491,22 @@ pub fn const_field<'tcx>( ) -> &'tcx ty::Const<'tcx> { trace!("const_field: {:?}, {:?}", field, value); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); - // get the operand again + // Get the operand again, ... let op = ecx.eval_const_to_op(value, None).unwrap(); - // downcast + // ... downcast, ... let down = match variant { None => op, Some(variant) => ecx.operand_downcast(op, variant).unwrap(), }; - // then project + // ... then project, ... let field = ecx.operand_field(down, field.index() as u64).unwrap(); // and finally move back to the const world, always normalizing because // this is not called for statics. op_to_const(&ecx, field) } -// this function uses `unwrap` copiously, because an already validated constant must have valid -// fields and can thus never fail outside of compiler bugs +// This function uses `unwrap` copiously, because an already validated constant must have valid +// fields and can thus never fail outside of compiler bugs. pub fn const_variant_index<'tcx>( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, @@ -589,18 +588,18 @@ pub fn const_eval_provider<'tcx>( tcx: TyCtxt<'tcx>, key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>, ) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> { - // see comment in const_eval_provider for what we're doing here + // See comment in `const_eval_provider` for what we're doing here. if key.param_env.reveal == Reveal::All { let mut key = key.clone(); key.param_env.reveal = Reveal::UserFacing; match tcx.const_eval(key) { - // try again with reveal all as requested + // Try again with reveal all as requested. Err(ErrorHandled::TooGeneric) => { // Promoteds should never be "too generic" when getting evaluated. // They either don't get evaluated, or we are in a monomorphic context assert!(key.value.promoted.is_none()); }, - // dedupliate calls + // Dedupliate calls. other => return other, } } @@ -625,9 +624,9 @@ pub fn const_eval_raw_provider<'tcx>( let mut key = key.clone(); key.param_env.reveal = Reveal::UserFacing; match tcx.const_eval_raw(key) { - // try again with reveal all as requested + // Try again with reveal-all as requested. Err(ErrorHandled::TooGeneric) => {}, - // dedupliate calls + // Dedupliate calls. other => return other, } } @@ -653,7 +652,7 @@ pub fn const_eval_raw_provider<'tcx>( tcx.at(span), key.param_env, CompileTimeInterpreter::new(), - Default::default() + Default::default(), ); let res = ecx.load_mir(cid.instance.def, cid.promoted); @@ -666,7 +665,7 @@ pub fn const_eval_raw_provider<'tcx>( }) }).map_err(|error| { let err = error_to_const_error(&ecx, error); - // errors in statics are always emitted as fatal errors + // Errors in statics are always emitted as fatal errors. if tcx.is_static(def_id) { // Ensure that if the above error was either `TooGeneric` or `Reported` // an error must be reported. @@ -677,14 +676,15 @@ pub fn const_eval_raw_provider<'tcx>( ); v } else if def_id.is_local() { - // constant defined in this crate, we can figure out a lint level! + // Constant is defined in this crate; we can figure out a lint level! match tcx.def_kind(def_id) { - // constants never produce a hard error at the definition site. Anything else is - // a backwards compatibility hazard (and will break old versions of winapi for sure) + // Constants never produce a hard error at the definition site. Anything else is + // a backwards compatibility hazard (and will break old versions of WinAPI for + // sure). // - // note that validation may still cause a hard error on this very same constant, + // Note that validation may still cause a hard error on this very same constant, // because any code that existed before validation could not have failed validation - // thus preventing such a hard error from being a backwards compatibility hazard + // thus preventing such a hard error from being a backwards compatibility hazard. Some(DefKind::Const) | Some(DefKind::AssocConst) => { let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap(); err.report_as_lint( @@ -694,8 +694,8 @@ pub fn const_eval_raw_provider<'tcx>( Some(err.span), ) }, - // promoting runtime code is only allowed to error if it references broken constants - // any other kind of error will be reported to the user as a deny-by-default lint + // Promoting runtime code is only allowed to error if it references broken constants + // any other kind of error will be reported to the user as a deny-by-default lint. _ => if let Some(p) = cid.promoted { let span = tcx.promoted_mir(def_id)[p].span; if let err_inval!(ReferencedConstant) = err.error { @@ -711,8 +711,8 @@ pub fn const_eval_raw_provider<'tcx>( Some(err.span), ) } - // anything else (array lengths, enum initializers, constant patterns) are reported - // as hard errors + // Anything else (array lengths, enum initializers, constant patterns) are reported + // as hard errors. } else { err.report_as_error( ecx.tcx, @@ -721,7 +721,7 @@ pub fn const_eval_raw_provider<'tcx>( }, } } else { - // use of broken constant from other crate + // Use of broken constant from other crate. err.report_as_error(ecx.tcx, "could not evaluate constant") } }) diff --git a/src/librustc_mir/dataflow/drop_flag_effects.rs b/src/librustc_mir/dataflow/drop_flag_effects.rs index c071b3101fce3..a09a2f95a25d8 100644 --- a/src/librustc_mir/dataflow/drop_flag_effects.rs +++ b/src/librustc_mir/dataflow/drop_flag_effects.rs @@ -1,11 +1,12 @@ -use rustc::mir::{self, Body, Location}; -use rustc::ty::{self, TyCtxt}; -use crate::util::elaborate_drops::DropFlagState; - use super::{MoveDataParamEnv}; use super::indexes::MovePathIndex; use super::move_paths::{MoveData, LookupResult, InitKind}; +use crate::util::elaborate_drops::DropFlagState; + +use rustc::mir::{self, Body, Location}; +use rustc::ty::{self, TyCtxt}; + pub fn move_path_children_matching<'tcx, F>(move_data: &MoveData<'tcx>, path: MovePathIndex, mut cond: F) @@ -37,9 +38,9 @@ pub fn move_path_children_matching<'tcx, F>(move_data: &MoveData<'tcx>, /// references and indexes can be reseated, trying to track them can /// only lead to trouble. /// -/// Places behind ADT's with a Drop impl are not tracked by +/// Places behind ADT's with a `Drop` impl are not tracked by /// elaboration since they can never have a drop-flag state that -/// differs from that of the parent with the Drop impl. +/// differs from that of the parent with the `Drop` impl. /// /// In both cases, the contents can only be accessed if and only if /// their parents are initialized. This implies for example that there diff --git a/src/librustc_mir/dataflow/impls/borrowed_locals.rs b/src/librustc_mir/dataflow/impls/borrowed_locals.rs index 1c43a553cc3c9..dbfea14cbc14a 100644 --- a/src/librustc_mir/dataflow/impls/borrowed_locals.rs +++ b/src/librustc_mir/dataflow/impls/borrowed_locals.rs @@ -1,8 +1,9 @@ pub use super::*; +use crate::dataflow::{BitDenotation, GenKillSet}; + use rustc::mir::*; use rustc::mir::visit::Visitor; -use crate::dataflow::{BitDenotation, GenKillSet}; /// This calculates if any part of a MIR local could have previously been borrowed. /// This means that once a local has been borrowed, its bit will be set @@ -28,13 +29,17 @@ impl<'a, 'tcx> HaveBeenBorrowedLocals<'a, 'tcx> { impl<'a, 'tcx> BitDenotation<'tcx> for HaveBeenBorrowedLocals<'a, 'tcx> { type Idx = Local; - fn name() -> &'static str { "has_been_borrowed_locals" } + + fn name() -> &'static str { + "has_been_borrowed_locals" + } + fn bits_per_block(&self) -> usize { self.body.local_decls.len() } fn start_block_effect(&self, _on_entry: &mut BitSet) { - // Nothing is borrowed on function entry + // Nothing is borrowed on function entry. } fn statement_effect(&self, @@ -46,7 +51,7 @@ impl<'a, 'tcx> BitDenotation<'tcx> for HaveBeenBorrowedLocals<'a, 'tcx> { trans, }.visit_statement(stmt, loc); - // StorageDead invalidates all borrows and raw pointers to a local + // StorageDead invalidates all borrows and raw pointers to a local. match stmt.kind { StatementKind::StorageDead(l) => trans.kill(l), _ => (), @@ -61,7 +66,7 @@ impl<'a, 'tcx> BitDenotation<'tcx> for HaveBeenBorrowedLocals<'a, 'tcx> { trans, }.visit_terminator(terminator, loc); match &terminator.kind { - // Drop terminators borrows the location + // Drop terminators borrows the location. TerminatorKind::Drop { location, .. } | TerminatorKind::DropAndReplace { location, .. } => { if let Some(local) = find_local(location) { @@ -79,7 +84,7 @@ impl<'a, 'tcx> BitDenotation<'tcx> for HaveBeenBorrowedLocals<'a, 'tcx> { _dest_bb: mir::BasicBlock, _dest_place: &mir::Place<'tcx>, ) { - // Nothing to do when a call returns successfully + // Nothing to do when a call returns successfully. } } diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 2ea6c4ae10fdc..1514bdb7bbdb5 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -37,7 +37,7 @@ pub struct Borrows<'a, 'tcx> { borrow_set: Rc>, borrows_out_of_scope_at_location: FxHashMap>, - /// NLL region inference context with which NLL queries should be resolved + /// NLL region inference context with which NLL queries should be resolved. _nonlexical_regioncx: Rc>, } @@ -235,13 +235,17 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { impl<'a, 'tcx> BitDenotation<'tcx> for Borrows<'a, 'tcx> { type Idx = BorrowIndex; - fn name() -> &'static str { "borrows" } + + fn name() -> &'static str { + "borrows" + } + fn bits_per_block(&self) -> usize { self.borrow_set.borrows.len() * 2 } fn start_block_effect(&self, _entry_set: &mut BitSet) { - // no borrows of code region_scopes have been taken prior to + // No borrows of code `region_scopes` have been taken prior to // function execution, so this method has no effect. } @@ -336,6 +340,6 @@ impl<'a, 'tcx> BitDenotation<'tcx> for Borrows<'a, 'tcx> { } impl<'a, 'tcx> BottomValue for Borrows<'a, 'tcx> { - /// bottom = nothing is reserved or activated yet; + /// bottom = nothing is reserved or activated yet const BOTTOM_VALUE: bool = false; } diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index 69bbe08792140..40a7f5fb84ea0 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -263,7 +263,11 @@ impl<'a, 'tcx> DefinitelyInitializedPlaces<'a, 'tcx> { impl<'a, 'tcx> BitDenotation<'tcx> for MaybeInitializedPlaces<'a, 'tcx> { type Idx = MovePathIndex; - fn name() -> &'static str { "maybe_init" } + + fn name() -> &'static str { + "maybe_init" + } + fn bits_per_block(&self) -> usize { self.move_data().move_paths.len() } @@ -316,7 +320,11 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeInitializedPlaces<'a, 'tcx> { impl<'a, 'tcx> BitDenotation<'tcx> for MaybeUninitializedPlaces<'a, 'tcx> { type Idx = MovePathIndex; - fn name() -> &'static str { "maybe_uninit" } + + fn name() -> &'static str { + "maybe_uninit" + } + fn bits_per_block(&self) -> usize { self.move_data().move_paths.len() } @@ -374,7 +382,11 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeUninitializedPlaces<'a, 'tcx> { impl<'a, 'tcx> BitDenotation<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { type Idx = MovePathIndex; - fn name() -> &'static str { "definite_init" } + + fn name() -> &'static str { + "definite_init" + } + fn bits_per_block(&self) -> usize { self.move_data().move_paths.len() } @@ -430,7 +442,11 @@ impl<'a, 'tcx> BitDenotation<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { impl<'a, 'tcx> BitDenotation<'tcx> for EverInitializedPlaces<'a, 'tcx> { type Idx = InitIndex; - fn name() -> &'static str { "ever_init" } + + fn name() -> &'static str { + "ever_init" + } + fn bits_per_block(&self) -> usize { self.move_data().inits.len() } @@ -511,12 +527,12 @@ impl<'a, 'tcx> BottomValue for MaybeInitializedPlaces<'a, 'tcx> { } impl<'a, 'tcx> BottomValue for MaybeUninitializedPlaces<'a, 'tcx> { - /// bottom = initialized (start_block_effect counters this at outset) + /// bottom = initialized (`start_block_effect` counters this at outset) const BOTTOM_VALUE: bool = false; } impl<'a, 'tcx> BottomValue for DefinitelyInitializedPlaces<'a, 'tcx> { - /// bottom = initialized (start_block_effect counters this at outset) + /// bottom = initialized (`start_block_effect` counters this at outset) const BOTTOM_VALUE: bool = true; } diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs index 0e01701ea9e44..7b2d8536c8881 100644 --- a/src/librustc_mir/dataflow/impls/storage_liveness.rs +++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs @@ -2,7 +2,7 @@ pub use super::*; use rustc::mir::*; use rustc::mir::visit::{ - PlaceContext, Visitor, NonMutatingUseContext, + NonMutatingUseContext, PlaceContext, Visitor, }; use std::cell::RefCell; use crate::dataflow::BitDenotation; @@ -34,7 +34,7 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeStorageLive<'a, 'tcx> { fn start_block_effect(&self, _on_entry: &mut BitSet) { // Nothing is live on function entry (generators only have a self - // argument, and we don't care about that) + // argument, and we don't care about that). assert_eq!(1, self.body.arg_count); } @@ -53,7 +53,7 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeStorageLive<'a, 'tcx> { fn terminator_effect(&self, _trans: &mut GenKillSet, _loc: Location) { - // Terminators have no effect + // Terminators have no effect. } fn propagate_call_return( @@ -63,7 +63,7 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeStorageLive<'a, 'tcx> { _dest_bb: mir::BasicBlock, _dest_place: &mir::Place<'tcx>, ) { - // Nothing to do when a call returns successfully + // Nothing to do when a call returns successfully. } } @@ -98,14 +98,18 @@ impl<'mir, 'tcx: 'mir> RequiresStorage<'mir, 'tcx> { impl<'mir, 'tcx> BitDenotation<'tcx> for RequiresStorage<'mir, 'tcx> { type Idx = Local; - fn name() -> &'static str { "requires_storage" } + + fn name() -> &'static str { + "requires_storage" + } + fn bits_per_block(&self) -> usize { self.body.local_decls.len() } fn start_block_effect(&self, _sets: &mut BitSet) { - // Nothing is live on function entry (generators only have a self - // argument, and we don't care about that) + // Nothing is live on function entry (generators only have a `self` + // argument, and we don't care about that). assert_eq!(1, self.body.arg_count); } @@ -119,8 +123,8 @@ impl<'mir, 'tcx> BitDenotation<'tcx> for RequiresStorage<'mir, 'tcx> { match stmt.kind { StatementKind::StorageLive(l) => sets.gen(l), StatementKind::StorageDead(l) => sets.kill(l), - StatementKind::Assign(ref place, _) - | StatementKind::SetDiscriminant { ref place, .. } => { + StatementKind::Assign(ref place, _) | + StatementKind::SetDiscriminant { ref place, .. } => { if let PlaceBase::Local(local) = place.base { sets.gen(local); } @@ -132,7 +136,7 @@ impl<'mir, 'tcx> BitDenotation<'tcx> for RequiresStorage<'mir, 'tcx> { } } } - _ => (), + _ => {} } } @@ -157,7 +161,7 @@ impl<'mir, 'tcx> BitDenotation<'tcx> for RequiresStorage<'mir, 'tcx> { } impl<'mir, 'tcx> RequiresStorage<'mir, 'tcx> { - /// Kill locals that are fully moved and have not been borrowed. + /// Kills locals that are fully moved and have not been borrowed. fn check_for_move(&self, sets: &mut GenKillSet, loc: Location) { let mut visitor = MoveVisitor { sets, @@ -166,8 +170,9 @@ impl<'mir, 'tcx> RequiresStorage<'mir, 'tcx> { visitor.visit_location(self.body, loc); } - /// Gen locals that are newly borrowed. This includes borrowing any part of - /// a local (we rely on this behavior of `HaveBeenBorrowedLocals`). + /// Sets GEN bit for locals that are newly borrowed. This includes borrowing + /// any part of a local (we rely on this behavior of + /// `HaveBeenBorrowedLocals`). fn check_for_borrow(&self, sets: &mut GenKillSet, loc: Location) { let mut borrowed_locals = self.borrowed_locals.borrow_mut(); borrowed_locals.seek(loc); diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 7fe2a890a5371..a79d28d680686 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -579,7 +579,7 @@ impl GenKill { } } -impl GenKillSet { +impl GenKillSet { pub(crate) fn clear(&mut self) { self.gen_set.clear(); self.kill_set.clear(); @@ -630,7 +630,7 @@ pub struct AllSets { trans: Vec>, } -impl AllSets { +impl AllSets { pub fn bits_per_block(&self) -> usize { self.bits_per_block } pub fn get_mut(&mut self, block_idx: usize) -> (&mut BitSet, &mut GenKillSet) { @@ -703,7 +703,8 @@ pub trait BitDenotation<'tcx>: BottomValue { /// plugged into a filename. fn name() -> &'static str; - /// Size of each bitvector allocated for each block in the analysis. + /// Gets the size of each bitvector allocated for each block in the + /// analysis. fn bits_per_block(&self) -> usize; /// Mutates the entry set according to the effects that diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index 5028e9650918c..6b8b507bb1a85 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -45,7 +45,7 @@ impl MoveOutIndex { /// Given `struct X { m: M, n: N }` and `x: X`, moves like `drop x.m;` /// move *out* of the place `x.m`. /// -/// The MovePaths representing `x.m` and `x.n` are siblings (that is, +/// The `MovePath`s representing `x.m` and `x.n` are siblings (that is, /// one of them will link to the other via the `next_sibling` field, /// and the other will have no entry in its `next_sibling` field), and /// they both have the MovePath representing `x` as their parent. @@ -100,9 +100,9 @@ impl<'tcx> fmt::Display for MovePath<'tcx> { pub struct MoveData<'tcx> { pub move_paths: IndexVec>, pub moves: IndexVec, - /// Each Location `l` is mapped to the MoveOut's that are effects - /// of executing the code at `l`. (There can be multiple MoveOut's - /// for a given `l` because each MoveOut is associated with one + /// Each `Location` `l` is mapped to the `MoveOut`'s that are effects + /// of executing the code at `l`. (There can be multiple `MoveOut`'s + /// for a given `l` because each `MoveOut` is associated with one /// particular path being moved.) pub loc_map: LocationMap>, pub path_map: IndexVec>, @@ -120,8 +120,8 @@ pub trait HasMoveData<'tcx> { #[derive(Debug)] pub struct LocationMap { - /// Location-indexed (BasicBlock for outer index, index within BB - /// for inner index) map. + /// Location-indexed map (`BasicBlock` for outer index, index within + /// `BasicBlock` for inner index). pub(crate) map: IndexVec>, } @@ -185,7 +185,7 @@ pub struct Init { } /// Initializations can be from an argument or from a statement. Arguments -/// do not have locations, in those cases the `Local` is kept.. +/// do not have locations, in whose cases the `Local` is kept. #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum InitLocation { Argument(Local), @@ -195,11 +195,11 @@ pub enum InitLocation { /// Additional information about the initialization. #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum InitKind { - /// Deep init, even on panic + /// Deep init, even on panic. Deep, - /// Only does a shallow init + /// Only does a shallow init. Shallow, - /// This doesn't initialize the variable on panic (and a panic is possible). + /// Doesn't initialize the variable on panic (and a panic is possible). NonPanicPathOnly, } @@ -218,17 +218,17 @@ impl Init { } } -/// Tables mapping from a place to its MovePathIndex. +/// Tables mapping from a place to its `MovePathIndex`. #[derive(Debug)] pub struct MovePathLookup { locals: IndexVec, - /// projections are made from a base-place and a projection - /// elem. The base-place will have a unique MovePathIndex; we use + /// Projections are made from a base-place and a projection + /// elem. The base-place will have a unique `MovePathIndex`; we use /// the latter as the index into the outer vector (narrowing /// subsequent search so that it is solely relative to that /// base-place). For the remaining lookup, we map the projection - /// elem to the associated MovePathIndex. + /// elem to the associated `MovePathIndex`. projections: FxHashMap<(MovePathIndex, AbstractElem), MovePathIndex>, } @@ -242,7 +242,7 @@ pub enum LookupResult { impl MovePathLookup { // Unlike the builder `fn move_path_for` below, this lookup - // alternative will *not* create a MovePath on the fly for an + // alternative will *not* create a `MovePath` on-the-fly for an // unknown place, but will rather return the nearest available // parent. pub fn find(&self, place_ref: PlaceRef<'_, '_>) -> LookupResult { diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index 9a73842d2f02a..58ff09a70c96d 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -64,7 +64,7 @@ fn mirror_stmts<'a, 'tcx>( }))) } hir::StmtKind::Item(..) => { - // ignore for purposes of the MIR + // Ignore for purposes of the MIR. } hir::StmtKind::Local(ref local) => { let remainder_scope = region::Scope { diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 0638cb462f73b..0d59e608af794 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -64,36 +64,36 @@ pub struct Stmt<'tcx> { #[derive(Clone, Debug)] pub enum StmtKind<'tcx> { Expr { - /// scope for this statement; may be used as lifetime of temporaries + /// The scope for this statement; may be used as lifetime of temporaries. scope: region::Scope, - /// expression being evaluated in this statement + /// The expression being evaluated in this statement. expr: ExprRef<'tcx>, }, Let { - /// scope for variables bound in this let; covers this and - /// remaining statements in block + /// The scope for variables bound in this `let`; covers this and + /// remaining statements in block. remainder_scope: region::Scope, - /// scope for the initialization itself; might be used as - /// lifetime of temporaries + /// The scope for the initialization itself; might be used as + /// lifetime of temporaries. init_scope: region::Scope, /// `let = ...` /// - /// if a type is included, it is added as an ascription pattern + /// If a type is included, it is added as an ascription pattern. pattern: Pattern<'tcx>, - /// let pat: ty = ... + /// `let pat: ty = ...` initializer: Option>, - /// the lint level for this let-statement + /// The lint level for this let-statement. lint_level: LintLevel, }, } -/// The Hair trait implementor lowers their expressions (`&'tcx H::Expr`) +/// The `Hair` trait implementor lowers its expressions (`&'tcx H::Expr`) /// into instances of this `Expr` enum. This lowering can be done /// basically as lazily or as eagerly as desired: every recursive /// reference to an expression in this enum is an `ExprRef<'tcx>`, which @@ -109,17 +109,17 @@ pub enum StmtKind<'tcx> { /// expected to be converted into `Expr::Call` instances. #[derive(Clone, Debug)] pub struct Expr<'tcx> { - /// type of this expression + /// The type of this expression. pub ty: Ty<'tcx>, - /// lifetime of this expression if it should be spilled into a - /// temporary; should be None only if in a constant context + /// The lifetime of this expression if it should be spilled into a + /// temporary; should be `None` only if in a constant context. pub temp_lifetime: Option, - /// span of the expression in the source + /// The span of the expression in the source. pub span: Span, - /// kind of expression + /// The kind of expression. pub kind: ExprKind<'tcx>, } diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index 210647ac1e9a3..75742ed6c99a2 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -247,14 +247,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { sty: Ty<'tcx>, dty: Ty<'tcx>, ) -> InterpResult<'tcx> { - // A -> A conversion + // `A` -> `A` conversion let (src_pointee_ty, dest_pointee_ty) = self.tcx.struct_lockstep_tails_erasing_lifetimes(sty, dty, self.param_env); match (&src_pointee_ty.sty, &dest_pointee_ty.sty) { (&ty::Array(_, length), &ty::Slice(_)) => { let ptr = self.read_immediate(src)?.to_scalar_ptr()?; - // u64 cast is from usize to u64, which is always good + // `u64` cast is from `usize` to `u64`, which is always good. let val = Immediate::new_slice( ptr, length.eval_usize(self.tcx.tcx, self.param_env), @@ -308,9 +308,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ); } - // unsizing of generic struct with pointer fields + // Unsizing of generic struct with pointer fields // Example: `Arc` -> `Arc` - // here we need to increase the size of every &T thin ptr field to a fat ptr + // Here, we need to increase the size of every `&T` thin ptr field to a fat ptr. for i in 0..src.layout.fields.count() { let dst_field = self.place_field(dest, i as u64)?; if dst_field.layout.is_zst() { diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 054b65f0e1a9e..d48b6b1acb841 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -41,21 +41,22 @@ pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> { /// The virtual call stack. pub(crate) stack: Vec>, - /// A cache for deduplicating vtables + /// A cache for deduplicating vtables. pub(super) vtables: FxHashMap<(Ty<'tcx>, Option>), Pointer>, } /// A stack frame. #[derive(Clone)] -pub struct Frame<'mir, 'tcx, Tag=(), Extra=()> { +pub struct Frame<'mir, 'tcx, Tag = (), Extra = ()> { //////////////////////////////////////////////////////////////////////////////// - // Function and callsite information + // Function and call-site information //////////////////////////////////////////////////////////////////////////////// + /// The MIR for the function called on this frame. pub body: &'mir mir::Body<'tcx>, - /// The def_id and substs of the current function. + /// The `DefId` and substs of the current function. pub instance: ty::Instance<'tcx>, /// The span of the call site. @@ -81,17 +82,19 @@ pub struct Frame<'mir, 'tcx, Tag=(), Extra=()> { //////////////////////////////////////////////////////////////////////////////// // Current position within the function //////////////////////////////////////////////////////////////////////////////// + /// The block that is currently executed (or will be executed after the above call stacks /// return). pub block: mir::BasicBlock, - /// The index of the currently evaluated statement. + /// The index of the statement currently being interpreted. pub stmt: usize, /// Extra data for the machine. pub extra: Extra, } +/// The action to perform when popping a particular frame off the stack. #[derive(Clone, Debug, Eq, PartialEq, Hash)] pub enum StackPopCleanup { /// Jump to the next block in the caller, or cause UB if None (that's a function @@ -107,15 +110,15 @@ pub enum StackPopCleanup { /// State of a local variable including a memoized layout #[derive(Clone, PartialEq, Eq)] -pub struct LocalState<'tcx, Tag=(), Id=AllocId> { +pub struct LocalState<'tcx, Tag = (), Id = AllocId> { pub value: LocalValue, - /// Don't modify if `Some`, this is only used to prevent computing the layout twice + /// Don't modify if `Some`; this is only used to prevent computing the layout twice. pub layout: Cell>>, } -/// Current value of a local variable +/// The current value of a local variable. #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] -pub enum LocalValue { +pub enum LocalValue { /// This local is not currently alive, and cannot be used at all. Dead, /// This local is alive but not yet initialized. It can be written to @@ -136,12 +139,12 @@ impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> { match self.value { LocalValue::Dead => throw_unsup!(DeadLocal), LocalValue::Uninitialized => - bug!("The type checker should prevent reading from a never-written local"), + bug!("the type checker should prevent reading from a never-written local"), LocalValue::Live(val) => Ok(val), } } - /// Overwrite the local. If the local can be overwritten in place, return a reference + /// Overwrite the local. If the local can be overwritten in place, return a reference /// to do so; otherwise return the `MemPlace` to consult instead. pub fn access_mut( &mut self, @@ -296,7 +299,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { instance: ty::InstanceDef<'tcx>, promoted: Option, ) -> InterpResult<'tcx, &'tcx mir::Body<'tcx>> { - // do not continue if typeck errors occurred (can only occur in local crate) + // Do not continue if typeck errors occurred (can only occur in local crate). let did = instance.def_id(); if did.is_local() && self.tcx.has_typeck_tables(did) @@ -475,6 +478,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { _ => bug!("size_and_align_of::<{:?}> not supported", layout.ty), } } + #[inline] pub fn size_and_align_of_mplace( &self, @@ -496,15 +500,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } ::log_settings::settings().indentation += 1; - // first push a stack frame so we have access to the local substs + // First, push a stack frame so we have access to the local substs. let extra = M::stack_push(self)?; self.stack.push(Frame { body, block: mir::START_BLOCK, return_to_block, return_place, - // empty local array, we fill it in below, after we are inside the stack frame and - // all methods actually know about the frame + // Empty local array; we fill it in below, after we are inside the stack frame and + // all methods actually know about the frame. locals: IndexVec::new(), span, instance, @@ -512,7 +516,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { extra, }); - // don't allocate at all for trivial constants + // Don't allocate at all for trivial constants. if body.local_decls.len() > 1 { // Locals are initially uninitialized. let dummy = LocalState { @@ -523,9 +527,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Return place is handled specially by the `eval_place` functions, and the // entry in `locals` should never be used. Make it dead, to be sure. locals[mir::RETURN_PLACE].value = LocalValue::Dead; - // Now mark those locals as dead that we do not want to initialize + // Now mark those locals as dead that we do not want to initialize. match self.tcx.def_kind(instance.def_id()) { - // statics and constants don't have `Storage*` statements, no need to look for them + // Statics and constants don't have `Storage*` statements; no need to look for them. Some(DefKind::Static) | Some(DefKind::Const) | Some(DefKind::AssocConst) => {}, @@ -545,7 +549,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } }, } - // done + // Done. self.frame_mut().locals = locals; } @@ -599,9 +603,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { )?; } } else { - // Uh, that shouldn't happen... the function did not intend to return + // This shouldn't happen, since the function did not intend to return. throw_ub!(Unreachable) } + // Jump to new block -- *after* validation so that the spans make more sense. match frame.return_to_block { StackPopCleanup::Goto(block) => { @@ -617,13 +622,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } - /// Mark a storage as live, killing the previous content and returning it. + /// Marks a storage as live, killing the previous content and returning it. /// Remember to deallocate that! pub fn storage_live( &mut self, local: mir::Local ) -> InterpResult<'tcx, LocalValue> { - assert!(local != mir::RETURN_PLACE, "Cannot make return place live"); + assert!(local != mir::RETURN_PLACE, "cannot make return place live"); trace!("{:?} is now live", local); let local_val = LocalValue::Uninitialized; @@ -636,7 +641,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Returns the old value of the local. /// Remember to deallocate that! pub fn storage_dead(&mut self, local: mir::Local) -> LocalValue { - assert!(local != mir::RETURN_PLACE, "Cannot make return place dead"); + assert!(local != mir::RETURN_PLACE, "cannot make return place dead"); trace!("{:?} is now dead", local); mem::replace(&mut self.frame_mut().locals[local].value, LocalValue::Dead) @@ -646,7 +651,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &mut self, local: LocalValue, ) -> InterpResult<'tcx> { - // FIXME: should we tell the user that there was a local which was never written to? + // FIXME: should we tell the user that there was a local that was never written to? if let LocalValue::Live(Operand::Indirect(MemPlace { ptr, .. })) = local { trace!("deallocating local"); let ptr = ptr.to_ptr()?; @@ -660,9 +665,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &self, gid: GlobalId<'tcx>, ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - // FIXME(oli-obk): make this check an assertion that it's not a static here + // FIXME(oli-obk): make this check an assertion that it's not a static here. // FIXME(RalfJ, oli-obk): document that `Place::Static` can never be anything but a static - // and `ConstValue::Unevaluated` can never be a static + // and `ConstValue::Unevaluated` can never be a static. let param_env = if self.tcx.is_static(gid.instance.def_id()) { ty::ParamEnv::reveal_all() } else { @@ -683,8 +688,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.raw_const_to_mplace(val) } + /// Prints the contents of the place, for debugging purposes. pub fn dump_place(&self, place: Place) { - // Debug output if !log_enabled!(::log::Level::Trace) { return; } @@ -751,7 +756,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let mut last_span = None; let mut frames = Vec::new(); for &Frame { instance, span, body, block, stmt, .. } in self.stack().iter().rev() { - // make sure we don't emit frames that are duplicates of the previous + // Make sure we don't emit frames that are duplicates of the previous. if explicit_span == Some(span) { last_span = Some(span); continue; diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 4cbbc0ffe17cc..4fae74880b47d 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -1,4 +1,4 @@ -//! This module specifies the type based interner for constants. +//! This module specifies the type-based interner for constants. //! //! After a const evaluation has computed a value, before we destroy the const evaluator's session //! memory, we need to extract all memory allocations to the global memory pool so they stay around. @@ -19,7 +19,7 @@ use super::{ use crate::const_eval::{CompileTimeInterpreter, CompileTimeEvalContext}; struct InternVisitor<'rt, 'mir, 'tcx> { - /// previously encountered safe references + /// Previously-encountered safe references. ref_tracking: &'rt mut RefTracking<(MPlaceTy<'tcx>, Mutability, InternMode)>, ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>, param_env: ParamEnv<'tcx>, @@ -27,7 +27,7 @@ struct InternVisitor<'rt, 'mir, 'tcx> { /// for sanity assertions that will ICE when `const_qualif` screws up. mode: InternMode, /// This field stores the mutability of the value *currently* being checked. - /// It is set to mutable when an `UnsafeCell` is encountered + /// It is set to mutable when an `UnsafeCell` is encountered. /// When recursing across a reference, we don't recurse but store the /// value to be checked in `ref_tracking` together with the mutability at which we are checking /// the value. @@ -45,9 +45,9 @@ enum InternMode { /// `static`. In a `static mut` we start out as mutable and thus can also contain further `&mut` /// that will actually be treated as mutable. Static, - /// UnsafeCell is OK in the value of a constant, but not behind references in a constant + /// `UnsafeCell` is okay in the value of a constant, but not behind references in a constant. ConstBase, - /// `UnsafeCell` ICEs + /// `UnsafeCell` ICEs. Const, } @@ -56,7 +56,7 @@ enum InternMode { struct IsStaticOrFn; impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> { - /// Intern an allocation without looking at its children + /// Interns an allocation without looking at its children. fn intern_shallow( &mut self, ptr: Pointer, @@ -66,20 +66,20 @@ impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> { "InternVisitor::intern {:?} with {:?}", ptr, mutability, ); - // remove allocation + // Remove allocation. let tcx = self.ecx.tcx; let memory = self.ecx.memory_mut(); let (kind, mut alloc) = match memory.alloc_map.remove(&ptr.alloc_id) { Some(entry) => entry, None => { - // if the pointer is dangling (neither in local nor global memory), we leave it + // If the pointer is dangling (neither in local nor global memory), we leave it // to validation to error. The `delay_span_bug` ensures that we don't forget such // a check in validation. if tcx.alloc_map.lock().get(ptr.alloc_id).is_none() { tcx.sess.delay_span_bug(self.ecx.tcx.span, "tried to intern dangling pointer"); } - // treat dangling pointers like other statics - // just to stop trying to recurse into them + // Treat dangling pointers like other statics + // just to stop trying to recurse into them. return Ok(Some(IsStaticOrFn)); }, }; @@ -88,11 +88,11 @@ impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> { match kind { MemoryKind::Stack | MemoryKind::Vtable => {}, } - // Ensure llvm knows to only put this into immutable memory if the value is immutable either + // Ensure LLVM knows to only put this into immutable memory if the value is immutable either // by being behind a reference or by being part of a static or const without interior // mutability alloc.mutability = mutability; - // link the alloc id to the actual allocation + // Link the alloc ID to the actual allocation. let alloc = tcx.intern_const_alloc(alloc); self.leftover_relocations.extend(alloc.relocations().iter().map(|&(_, ((), reloc))| reloc)); tcx.alloc_map.lock().set_alloc_id_memory(ptr.alloc_id, alloc); @@ -142,14 +142,14 @@ for let ty = mplace.layout.ty; if let ty::Ref(_, referenced_ty, mutability) = ty.sty { let value = self.ecx.read_immediate(mplace.into())?; - // Handle trait object vtables + // Handle trait object vtables. if let Ok(meta) = value.to_meta() { if let ty::Dynamic(..) = self.ecx.tcx.struct_tail_erasing_lifetimes(referenced_ty, self.param_env).sty { if let Ok(vtable) = meta.unwrap().to_ptr() { - // explitly choose `Immutable` here, since vtables are immutable, even - // if the reference of the fat pointer is mutable + // Explitly choose `Immutable` here, since vtables are immutable, even + // if the reference of the fat pointer is mutable. self.intern_shallow(vtable, Mutability::Immutable)?; } } @@ -164,15 +164,15 @@ for // This is not an inherent limitation, but one that we know to be true, because // const qualification enforces it. We can lift it in the future. match (self.mode, mutability) { - // immutable references are fine everywhere + // Immutable references are fine everywhere. (_, hir::Mutability::MutImmutable) => {}, - // all is "good and well" in the unsoundness of `static mut` + // All is "good and well" in the unsoundness of `static mut`. - // mutable references are ok in `static`. Either they are treated as immutable + // Mutable references are ok in `static`. Either they are treated as immutable // because they are behind an immutable one, or they are behind an `UnsafeCell` // and thus ok. (InternMode::Static, hir::Mutability::MutMutable) => {}, - // we statically prevent `&mut T` via `const_qualif` and double check this here + // We statically prevent `&mut T` via `const_qualif` and double check this here. (InternMode::ConstBase, hir::Mutability::MutMutable) | (InternMode::Const, hir::Mutability::MutMutable) => { match referenced_ty.sty { @@ -195,7 +195,7 @@ for (Mutability::Mutable, hir::Mutability::MutMutable) => Mutability::Mutable, _ => Mutability::Immutable, }; - // Compute the mutability of the allocation + // Compute the mutability of the allocation. let intern_mutability = intern_mutability( self.ecx.tcx.tcx, self.param_env, @@ -210,12 +210,12 @@ for other => other, }; match self.intern_shallow(ptr, intern_mutability)? { - // No need to recurse, these are interned already and statics may have - // cycles, so we don't want to recurse there + // No need to recurse; these are interned already and statics may have + // cycles, so we don't want to recurse there. Some(IsStaticOrFn) => {}, - // intern everything referenced by this value. The mutability is taken from the + // Intern everything referenced by this value. The mutability is taken from the // reference. It is checked above that mutable references only happen in - // `static mut` + // `static mut`. None => self.ref_tracking.track((mplace, mutability, mode), || ()), } } @@ -224,8 +224,8 @@ for } } -/// Figure out the mutability of the allocation. -/// Mutable if it has interior mutability *anywhere* in the type. +/// Figures out the mutability of the allocation. +/// It is mutable if it has interior mutability *anywhere* in the type. fn intern_mutability<'tcx>( tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, @@ -245,20 +245,20 @@ pub fn intern_const_alloc_recursive( ecx: &mut CompileTimeEvalContext<'mir, 'tcx>, def_id: DefId, ret: MPlaceTy<'tcx>, - // FIXME(oli-obk): can we scrap the param env? I think we can, the final value of a const eval - // must always be monomorphic, right? + // FIXME(oli-obk): can we scrap the param env? I think we can, since the final value of a const + // eval must always be monomorphic, right? param_env: ty::ParamEnv<'tcx>, ) -> InterpResult<'tcx> { let tcx = ecx.tcx; - // this `mutability` is the mutability of the place, ignoring the type + // This `mutability` is the mutability of the place, ignoring the type let (mutability, base_intern_mode) = match tcx.static_mutability(def_id) { Some(hir::Mutability::MutImmutable) => (Mutability::Immutable, InternMode::Static), None => (Mutability::Immutable, InternMode::ConstBase), - // `static mut` doesn't care about interior mutability, it's mutable anyway + // `static mut` doesn't care about interior mutability; it's mutable anyway. Some(hir::Mutability::MutMutable) => (Mutability::Mutable, InternMode::Static), }; - // type based interning + // Type-based interning let mut ref_tracking = RefTracking::new((ret, mutability, base_intern_mode)); let leftover_relocations = &mut FxHashSet::default(); @@ -270,7 +270,7 @@ pub fn intern_const_alloc_recursive( tcx.tcx, param_env, ret.layout.ty, tcx.span, mutability, ); - // start with the outermost allocation + // Start with the outermost allocation. InternVisitor { ref_tracking: &mut ref_tracking, ecx, @@ -290,8 +290,8 @@ pub fn intern_const_alloc_recursive( mutability, }.visit_value(mplace); if let Err(error) = interned { - // This can happen when e.g. the tag of an enum is not a valid discriminant. We do have - // to read enum discriminants in order to find references in enum variant fields. + // This can happen when, e.g., the tag of an enum is not a valid discriminant. We do + // have to read enum discriminants in order to find references in enum variant fields. if let err_unsup!(ValidationFailure(_)) = error.kind { let err = crate::const_eval::error_to_const_error(&ecx, error); match err.struct_error(ecx.tcx, "it is undefined behavior to use this value") { @@ -307,13 +307,12 @@ pub fn intern_const_alloc_recursive( } // Intern the rest of the allocations as mutable. These might be inside unions, padding, raw - // pointers, ... So we can't intern them according to their type rules - + // pointers, etc., so we can't intern them according to their type rules. let mut todo: Vec<_> = leftover_relocations.iter().cloned().collect(); while let Some(alloc_id) = todo.pop() { if let Some((_, alloc)) = ecx.memory_mut().alloc_map.remove(&alloc_id) { // We can't call the `intern` method here, as its logic is tailored to safe references. - // So we hand-roll the interning logic here again + // So, we hand-roll the interning logic here again. let alloc = tcx.intern_const_alloc(alloc); tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); for &(_, ((), reloc)) in alloc.relocations().iter() { @@ -326,5 +325,6 @@ pub fn intern_const_alloc_recursive( throw_unsup!(ValidationFailure("encountered dangling pointer in final constant".into())) } } + Ok(()) } diff --git a/src/librustc_mir/interpret/intrinsics/type_name.rs b/src/librustc_mir/interpret/intrinsics/type_name.rs index 032d16a49db4b..313d207665512 100644 --- a/src/librustc_mir/interpret/intrinsics/type_name.rs +++ b/src/librustc_mir/interpret/intrinsics/type_name.rs @@ -33,7 +33,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> { fn print_type(mut self, ty: Ty<'tcx>) -> Result { match ty.sty { - // Types without identity. + // Types without identity | ty::Bool | ty::Char | ty::Int(_) @@ -50,7 +50,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> { | ty::Dynamic(_, _) => self.pretty_print_type(ty), - // Placeholders (all printed as `_` to uniformize them). + // Placeholders (all printed as `_` to make them uniform) | ty::Param(_) | ty::Bound(..) | ty::Placeholder(_) @@ -61,7 +61,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> { Ok(self) } - // Types with identity (print the module path). + // Types with identity (print the module path) | ty::Adt(&ty::AdtDef { did: def_id, .. }, substs) | ty::FnDef(def_id, substs) | ty::Opaque(def_id, substs) @@ -72,9 +72,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> { => self.print_def_path(def_id, substs), ty::Foreign(def_id) => self.print_def_path(def_id, &[]), - ty::GeneratorWitness(_) => { - bug!("type_name: unexpected `GeneratorWitness`") - } + ty::GeneratorWitness(_) => bug!("type_name: unexpected `GeneratorWitness`"), } } @@ -82,7 +80,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> { self, _: &'tcx ty::Const<'tcx>, ) -> Result { - // don't print constants to the user + // Don't print constants to the user. Ok(self) } @@ -172,6 +170,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> { } } } + impl PrettyPrinter<'tcx> for AbsolutePathPrinter<'tcx> { fn region_should_not_be_omitted( &self, @@ -179,6 +178,7 @@ impl PrettyPrinter<'tcx> for AbsolutePathPrinter<'tcx> { ) -> bool { false } + fn comma_sep(mut self, mut elems: impl Iterator) -> Result where T: Print<'tcx, Self, Output = Self, Error = Self::Error>, diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index bb74a50156e56..890aaa34b54e2 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -14,12 +14,12 @@ use super::{ InterpCx, PlaceTy, OpTy, ImmTy, MemoryKind, Pointer, Memory, }; -/// Whether this kind of memory is allowed to leak +/// Whether this kind of memory is allowed to leak. pub trait MayLeak: Copy { fn may_leak(self) -> bool; } -/// The functionality needed by memory to manage its allocations +/// The functionality needed by memory to manage its allocations. pub trait AllocMap { /// Tests if the map contains the given key. /// Deliberately takes `&mut` because that is sufficient, and some implementations @@ -69,10 +69,10 @@ pub trait AllocMap { /// Methods of this trait signifies a point where CTFE evaluation would fail /// and some use case dependent behaviour can instead be applied. pub trait Machine<'mir, 'tcx>: Sized { - /// Additional memory kinds a machine wishes to distinguish from the builtin ones + /// Additional memory kinds that a machine wishes to distinguish from the builtin ones. type MemoryKinds: ::std::fmt::Debug + MayLeak + Eq + 'static; - /// Tag tracked alongside every pointer. This is used to implement "Stacked Borrows" + /// Tags tracked alongside every pointer. This is used to implement "Stacked Borrows" /// . /// The `default()` is used for pointers to consts, statics, vtables and functions. type PointerTag: ::std::fmt::Debug + Copy + Eq + Hash + 'static; @@ -93,7 +93,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Extra data stored in every allocation. type AllocExtra: AllocationExtra + 'static; - /// Memory's allocation map + /// The allocation map of `Memory`. type MemoryMap: AllocMap< AllocId, @@ -102,24 +102,24 @@ pub trait Machine<'mir, 'tcx>: Sized { Default + Clone; - /// The memory kind to use for copied statics -- or None if statics should not be mutated + /// The memory kind to use for copied statics -- or `None` if statics should not be mutated` /// and thus any such attempt will cause a `ModifiedStatic` error to be raised. /// Statics are copied under two circumstances: When they are mutated, and when /// `tag_allocation` or `find_foreign_static` (see below) returns an owned allocation /// that is added to the memory so that the work is not done twice. const STATIC_KIND: Option; - /// Whether memory accesses should be alignment-checked. + /// `true` if memory accesses should be alignment-checked. const CHECK_ALIGN: bool; - /// Whether to enforce the validity invariant + /// Indicates whether to enforce the validity invariant. fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; /// Called before a basic block terminator is executed. /// You can use this to detect endlessly running programs. fn before_terminator(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx>; - /// Entry point to all function calls. + /// The entry point to all function calls. /// /// Returns either the mir to use for the call, or `None` if execution should /// just proceed (which usually means this hook did all the work that the @@ -137,7 +137,7 @@ pub trait Machine<'mir, 'tcx>: Sized { ret: Option, ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>>; - /// Execute `fn_val`. it is the hook's responsibility to advance the instruction + /// Executes `fn_val`. It is the hook's responsibility to advance the instruction /// pointer as appropriate. fn call_extra_fn( ecx: &mut InterpCx<'mir, 'tcx, Self>, @@ -147,7 +147,7 @@ pub trait Machine<'mir, 'tcx>: Sized { ret: Option, ) -> InterpResult<'tcx>; - /// Directly process an intrinsic without pushing a stack frame. + /// Directly processes an intrinsic without pushing a stack frame. /// If this returns successfully, the engine will take care of jumping to the next block. fn call_intrinsic( ecx: &mut InterpCx<'mir, 'tcx, Self>, @@ -170,7 +170,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Called for all binary operations where the LHS has pointer type. /// - /// Returns a (value, overflowed) pair if the operation succeeded + /// Returns a `(value, overflowed)` pair if the operation succeeded. fn binary_ptr_op( ecx: &InterpCx<'mir, 'tcx, Self>, bin_op: mir::BinOp, @@ -178,7 +178,7 @@ pub trait Machine<'mir, 'tcx>: Sized { right: ImmTy<'tcx, Self::PointerTag>, ) -> InterpResult<'tcx, (Scalar, bool, Ty<'tcx>)>; - /// Heap allocations via the `box` keyword. + /// Performs heap allocations via the `box` keyword. fn box_alloc( ecx: &mut InterpCx<'mir, 'tcx, Self>, dest: PlaceTy<'tcx, Self::PointerTag>, @@ -207,7 +207,7 @@ pub trait Machine<'mir, 'tcx>: Sized { kind: Option>, ) -> (Cow<'b, Allocation>, Self::PointerTag); - /// Return the "base" tag for the given static allocation: the one that is used for direct + /// Returns the "base" tag for the given static allocation: the one that is used for direct /// accesses to this static/const/fn allocation. /// /// Be aware that requesting the `Allocation` for that `id` will lead to cycles @@ -217,7 +217,7 @@ pub trait Machine<'mir, 'tcx>: Sized { id: AllocId, ) -> Self::PointerTag; - /// Executes a retagging operation + /// Executes a retagging operation. #[inline] fn retag( _ecx: &mut InterpCx<'mir, 'tcx, Self>, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 62b1760508b4c..f40ca6ff8344c 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -24,11 +24,11 @@ use super::{ #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] pub enum MemoryKind { - /// Error if deallocated except during a stack pop + /// Error if deallocated except during a stack pop. Stack, - /// Error if ever deallocated + /// Error if ever deallocated. Vtable, - /// Additional memory kinds a machine wishes to distinguish from the builtin ones + /// Additional memory kinds that a machine wishes to distinguish from the builtin ones. Machine(T), } @@ -85,7 +85,8 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { /// the wrong type), so we let the machine override this type. /// Either way, if the machine allows writing to a static, doing so will /// create a copy of the static allocation here. - // FIXME: this should not be public, but interning currently needs access to it + // + // FIXME: this should not be public, but interning currently needs access to it. pub(super) alloc_map: M::MemoryMap, /// Map for "extra" function pointers. @@ -94,13 +95,14 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { /// To be able to compare pointers with NULL, and to check alignment for accesses /// to ZSTs (where pointers may dangle), we keep track of the size even for allocations /// that do not exist any more. - // FIXME: this should not be public, but interning currently needs access to it + // + // FIXME: this should not be public, but interning currently needs access to it. pub(super) dead_alloc_map: FxHashMap, /// Extra data added by the machine. pub extra: M::MemoryExtra, - /// Lets us implement `HasDataLayout`, which is awfully convenient. + /// Lets us implement `HasDataLayout`, which is very convenient. pub tcx: TyCtxtAt<'tcx>, } @@ -111,7 +113,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for Memory<'mir, 'tcx, M> } } -// FIXME: Really we shouldn't clone memory, ever. Snapshot machinery should instead +// FIXME: Really, we shouldn't clone memory, ever. Snapshot machinery should instead // carefully copy only the reachable parts. impl<'mir, 'tcx, M> Clone for Memory<'mir, 'tcx, M> where @@ -119,7 +121,7 @@ where M::MemoryMap: AllocMap, Allocation)>, { fn clone(&self) -> Self { - Memory { + Self { alloc_map: self.alloc_map.clone(), extra_fn_ptr_map: self.extra_fn_ptr_map.clone(), dead_alloc_map: self.dead_alloc_map.clone(), @@ -131,7 +133,7 @@ where impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { pub fn new(tcx: TyCtxtAt<'tcx>, extra: M::MemoryExtra) -> Self { - Memory { + Self { alloc_map: M::MemoryMap::default(), extra_fn_ptr_map: FxHashMap::default(), dead_alloc_map: FxHashMap::default(), @@ -148,8 +150,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { pub fn create_fn_alloc( &mut self, fn_val: FnVal<'tcx, M::ExtraFnVal>, - ) -> Pointer - { + ) -> Pointer { let id = match fn_val { FnVal::Instance(instance) => self.tcx.alloc_map.lock().create_fn_alloc(instance), FnVal::Other(extra) => { @@ -205,8 +206,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { throw_unsup!(ReallocateNonBasePtr) } - // For simplicities' sake, we implement reallocate as "alloc, copy, dealloc". - // This happens so rarely, the perf advantage is outweighed by the maintenance cost. + // For simplicity's sake, we implement reallocate as "alloc, copy, dealloc". + // This happens so rarely, the performance advantage is outweighed by the maintenance cost. let new_ptr = self.allocate(new_size, new_align, kind); let old_size = match old_size_and_align { Some((size, _align)) => size, @@ -216,14 +217,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ptr, new_ptr, old_size.min(new_size), - /*nonoverlapping*/ true, + /* nonoverlapping */ true, )?; self.deallocate(ptr, old_size_and_align, kind)?; Ok(new_ptr) } - /// Deallocate a local, or do nothing if that local has been made into a static + /// Deallocates a local, or do nothing if that local has been made into a static pub fn deallocate_local(&mut self, ptr: Pointer) -> InterpResult<'tcx> { // The allocation might be already removed by static interning. // This can only really happen in the CTFE instance, not in miri. @@ -249,7 +250,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let (alloc_kind, mut alloc) = match self.alloc_map.remove(&ptr.alloc_id) { Some(alloc) => alloc, None => { - // Deallocating static memory -- always an error + // Deallocating static memory -- always an error. return Err(match self.tcx.alloc_map.lock().get(ptr.alloc_id) { Some(GlobalAlloc::Function(..)) => err_unsup!(DeallocatedWrongMemoryKind( "function".to_string(), @@ -277,23 +278,23 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } } - // Let the machine take some extra action + // Let the machine take some extra action. let size = alloc.size; AllocationExtra::memory_deallocated(&mut alloc, ptr, size)?; - // Don't forget to remember size and align of this now-dead allocation + // Don't forget to remember size and align of this now-dead allocation. let old = self.dead_alloc_map.insert( ptr.alloc_id, (alloc.size, alloc.align) ); if old.is_some() { - bug!("Nothing can be deallocated twice"); + bug!("nothing can be deallocated twice"); } Ok(()) } - /// Check if the given scalar is allowed to do a memory access of given `size` + /// Checks if the given scalar is allowed to do a memory access of given `size` /// and `align`. On success, returns `None` for zero-sized accesses (where /// nothing else is left to do) and a `Pointer` to use for the actual access otherwise. /// Crucially, if the input is a `Pointer`, we will test it for liveness @@ -393,18 +394,18 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { }) } - /// Test if the pointer might be NULL. + /// Tests if the pointer might be NULL. pub fn ptr_may_be_null( &self, ptr: Pointer, ) -> bool { let (size, _align) = self.get_size_and_align(ptr.alloc_id, AllocCheck::MaybeDead) - .expect("alloc info with MaybeDead cannot fail"); + .expect("alloc info with `MaybeDead` cannot fail"); ptr.check_inbounds_alloc(size, CheckInAllocMsg::NullPointerTest).is_err() } } -/// Allocation accessors +/// Allocation accessors. impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// Helper function to obtain the global (tcx) allocation for a static. /// This attempts to return a reference to an existing allocation if @@ -445,12 +446,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { instance, promoted: None, }; - // use the raw query here to break validation cycles. Later uses of the static - // will call the full query anyway + // Use the raw query here to break validation cycles. Later uses of the static + // will call the full query anyway. let raw_const = tcx.const_eval_raw(ty::ParamEnv::reveal_all().and(gid)) .map_err(|err| { - // no need to report anything, the const_eval call takes care of that - // for statics + // No need to report anything, the `const_eval` call takes care of that + // for statics. assert!(tcx.is_static(def_id)); match err { ErrorHandled::Reported => @@ -493,7 +494,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { Err(Ok(alloc)) } Cow::Owned(alloc) => { - // Need to put it into the map and return a ref to that + // Need to put it into the map and return a ref to that. let kind = M::STATIC_KIND.expect( "I got an owned allocation that I have to copy but the machine does \ not expect that to happen" @@ -502,7 +503,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } } }); - // Now unpack that funny error type + // Now unpack that funny error type. match a { Ok(a) => Ok(&a.1), Err(a) => a @@ -541,7 +542,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } } - /// Obtain the size and alignment of an allocation, even if that allocation has + /// Obtains the size and alignment of an allocation, even if that allocation has /// been deallocated. /// /// If `liveness` is `AllocCheck::MaybeDead`, this function always returns `Ok`. @@ -654,7 +655,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { relocations.push((i, target_id)); } if alloc.undef_mask().is_range_defined(i, i + Size::from_bytes(1)).is_ok() { - // this `as usize` is fine, since `i` came from a `usize` + // This `as usize` is fine, since `i` came from a `usize`. let i = i.bytes() as usize; // Checked definedness (and thus range) and relocations. This access also doesn't @@ -680,10 +681,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let mut pos = Size::ZERO; let relocation_width = (self.pointer_size().bytes() - 1) * 3; for (i, target_id) in relocations { - // this `as usize` is fine, since we can't print more chars than `usize::MAX` + // This `as usize` is fine, since we can't print more chars than `usize::MAX`. write!(msg, "{:1$}", "", ((i - pos) * 3).bytes() as usize).unwrap(); let target = format!("({})", target_id); - // this `as usize` is fine, since we can't print more chars than `usize::MAX` + // This `as usize` is fine, since we can't print more chars than `usize::MAX`. write!(msg, "└{0:─^1$}┘ ", target, relocation_width as usize).unwrap(); pos = i + self.pointer_size(); } @@ -691,7 +692,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } } - /// For debugging, print a list of allocations and all allocations they point to, recursively. + /// For debugging, prints a list of allocations and all allocations they point to, recursively. pub fn dump_allocs(&self, mut allocs: Vec) { if !log_enabled!(::log::Level::Trace) { return; @@ -738,7 +739,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } }, }; - } } @@ -752,7 +752,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { n } - /// This is used by [priroda](https://github.com/oli-obk/priroda) + /// This is used by [priroda](https://github.com/oli-obk/priroda). pub fn alloc_map(&self) -> &M::MemoryMap { &self.alloc_map } @@ -803,7 +803,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { length: u64, nonoverlapping: bool, ) -> InterpResult<'tcx> { - // first copy the relocations to a temporary buffer, because + // First, copy the relocations to a temporary buffer, because // `get_bytes_mut` will clear the relocations, which is correct, // since we don't want to keep any relocations at the target. // (`get_bytes_with_undef_and_ptr` below checks that there are no @@ -813,7 +813,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let tcx = self.tcx.tcx; - // This checks relocation edges on the src. + // This checks relocation edges on `src`. let src_bytes = self.get(src.alloc_id)? .get_bytes_with_undef_and_ptr(&tcx, src, size)? .as_ptr(); @@ -853,18 +853,18 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } } - // copy definedness to the destination + // Copy definedness to the destination. self.copy_undef_mask(src, dest, size, length)?; - // copy the relocations to the destination + // Copy the relocations to the destination. self.get_mut(dest.alloc_id)?.mark_relocation_range(relocations); Ok(()) } } -/// Undefined bytes +/// Undefined bytes. impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { - // FIXME: Add a fast version for the common, nonoverlapping case + // FIXME: Add a fast version for the common, nonoverlapping case. fn copy_undef_mask( &mut self, src: Pointer, @@ -878,7 +878,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let src_alloc = self.get(src.alloc_id)?; let compressed = src_alloc.compress_undef_range(src, size); - // now fill in all the data + // Now, fill in all the data. let dest_allocation = self.get_mut(dest.alloc_id)?; dest_allocation.mark_compressed_undef_range(&compressed, dest, size, repeat); diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index b5aab992e3adb..9c8891ed713eb 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -26,8 +26,8 @@ pub use rustc::mir::interpret::ScalarMaybeUndef; /// operations and fat pointers. This idea was taken from rustc's codegen. /// In particular, thanks to `ScalarPair`, arithmetic operations and casts can be entirely /// defined on `Immediate`, and do not have to work with a `Place`. -#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -pub enum Immediate { +#[derive(RustcEncodable, RustcDecodable, Copy, Clone, Debug, Hash, PartialEq, Eq)] +pub enum Immediate { Scalar(ScalarMaybeUndef), ScalarPair(ScalarMaybeUndef, ScalarMaybeUndef), } @@ -66,7 +66,7 @@ impl<'tcx, Tag> Immediate { pub fn to_scalar_or_undef(self) -> ScalarMaybeUndef { match self { Immediate::Scalar(val) => val, - Immediate::ScalarPair(..) => bug!("Got a fat pointer where a scalar was expected"), + Immediate::ScalarPair(..) => bug!("got a fat pointer where a scalar was expected"), } } @@ -78,13 +78,13 @@ impl<'tcx, Tag> Immediate { #[inline] pub fn to_scalar_pair(self) -> InterpResult<'tcx, (Scalar, Scalar)> { match self { - Immediate::Scalar(..) => bug!("Got a thin pointer where a scalar pair was expected"), + Immediate::Scalar(..) => bug!("got a thin pointer where a scalar pair was expected"), Immediate::ScalarPair(a, b) => Ok((a.not_undef()?, b.not_undef()?)) } } /// Converts the immediate into a pointer (or a pointer-sized integer). - /// Throws away the second half of a ScalarPair! + /// Throws away the second half of a `ScalarPair`! #[inline] pub fn to_scalar_ptr(self) -> InterpResult<'tcx, Scalar> { match self { @@ -94,7 +94,7 @@ impl<'tcx, Tag> Immediate { } /// Converts the value into its metadata. - /// Throws away the first half of a ScalarPair! + /// Throws away the first half of a `ScalarPair`! #[inline] pub fn to_meta(self) -> InterpResult<'tcx, Option>> { Ok(match self { @@ -104,10 +104,10 @@ impl<'tcx, Tag> Immediate { } } -// ScalarPair needs a type to interpret, so we often have an immediate and a type together -// as input for binary and cast operations. +/// `ScalarPair` needs a type to interpret, so we often have an immediate and a type together +/// as input for binary and cast operations. #[derive(Copy, Clone, Debug)] -pub struct ImmTy<'tcx, Tag=()> { +pub struct ImmTy<'tcx, Tag = ()> { pub(crate) imm: Immediate, pub layout: TyLayout<'tcx>, } @@ -123,8 +123,8 @@ impl<'tcx, Tag> ::std::ops::Deref for ImmTy<'tcx, Tag> { /// An `Operand` is the result of computing a `mir::Operand`. It can be immediate, /// or still in memory. The latter is an optimization, to delay reading that chunk of /// memory and to avoid having to store arbitrary-sized data here. -#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -pub enum Operand { +#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, RustcEncodable, RustcDecodable)] +pub enum Operand { Immediate(Immediate), Indirect(MemPlace), } @@ -136,7 +136,7 @@ impl Operand { { match self { Operand::Indirect(mplace) => mplace, - _ => bug!("assert_mem_place: expected Operand::Indirect, got {:?}", self), + _ => bug!("assert_mem_place: expected `Operand::Indirect`, got {:?}", self), } } @@ -147,22 +147,22 @@ impl Operand { { match self { Operand::Immediate(imm) => imm, - _ => bug!("assert_immediate: expected Operand::Immediate, got {:?}", self), + _ => bug!("assert_immediate: expected `Operand::Immediate`, got {:?}", self), } } } #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -pub struct OpTy<'tcx, Tag=()> { - op: Operand, // Keep this private, it helps enforce invariants +pub struct OpTy<'tcx, Tag = ()> { + op: Operand, // Keep this private; it helps enforce invariants. pub layout: TyLayout<'tcx>, } impl<'tcx, Tag> ::std::ops::Deref for OpTy<'tcx, Tag> { type Target = Operand; #[inline(always)] - fn deref(&self) -> &Operand { + fn deref(&self) -> &Self::Target { &self.op } } @@ -209,8 +209,8 @@ impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag> { } } -// Use the existing layout if given (but sanity check in debug mode), -// or compute the layout. +// Uses the existing layout if given (but sanity check in debug mode), +// or computes the layout. #[inline(always)] pub(super) fn from_known_layout<'tcx>( layout: Option>, @@ -231,7 +231,7 @@ pub(super) fn from_known_layout<'tcx>( } impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { - /// Normalice `place.ptr` to a `Pointer` if this is a place and not a ZST. + /// Normalizes `place.ptr` to a `Pointer` if this is a place and not a ZST. /// Can be helpful to avoid lots of `force_ptr` calls later, if this place is used a lot. #[inline] pub fn force_op_ptr( @@ -244,14 +244,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - /// Try reading an immediate in memory; this is interesting particularly for `ScalarPair`. + /// Tries reading an immediate in memory; this is interesting particularly for `ScalarPair`. /// Returns `None` if the layout does not permit loading this as a value. fn try_read_immediate_from_mplace( &self, mplace: MPlaceTy<'tcx, M::PointerTag>, ) -> InterpResult<'tcx, Option>> { if mplace.layout.is_unsized() { - // Don't touch unsized + // Don't touch unsized. return Ok(None); } @@ -300,7 +300,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - /// Try returning an immediate for the operand. + /// Tries returning an immediate for the operand. /// If the layout does not permit loading this as an immediate, return where in memory /// we can find the data. /// Note that for a given layout, this operation will either always fail or always @@ -322,7 +322,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }) } - /// Read an immediate from a place, asserting that that is possible with the given layout. + /// Reads an immediate from a place, asserting that that is possible with the given layout. #[inline(always)] pub fn read_immediate( &self, @@ -335,15 +335,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - /// Read a scalar from a place + /// Reads a scalar from a place. pub fn read_scalar( &self, - op: OpTy<'tcx, M::PointerTag> + op: OpTy<'tcx, M::PointerTag>, ) -> InterpResult<'tcx, ScalarMaybeUndef> { Ok(self.read_immediate(op)?.to_scalar_or_undef()) } - // Turn the MPlace into a string (must already be dereferenced!) + // Turns the `MPlace` into a string (must already be dereferenced!). pub fn read_str( &self, mplace: MPlaceTy<'tcx, M::PointerTag>, @@ -356,7 +356,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(str) } - /// Projection functions + /// Projection functions. pub fn operand_field( &self, op: OpTy<'tcx, M::PointerTag>, @@ -379,9 +379,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } let offset = op.layout.fields.offset(field); let immediate = match *base { - // the field covers the entire type + // The field covers the entire type. _ if offset.bytes() == 0 && field_layout.size == op.layout.size => *base, - // extract fields from types with `ScalarPair` ABI + // Extract fields from types with `ScalarPair` ABI. Immediate::ScalarPair(a, b) => { let val = if offset.bytes() == 0 { a } else { b }; Immediate::from(val) @@ -397,7 +397,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { op: OpTy<'tcx, M::PointerTag>, variant: VariantIdx, ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { - // Downcasts only change the layout + // Downcasts only change the layout. Ok(match op.try_as_mplace() { Ok(mplace) => { self.mplace_downcast(mplace, variant)?.into() @@ -422,19 +422,19 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Subslice { .. } | ConstantIndex { .. } | Index(_) => if base.layout.is_zst() { OpTy { op: Operand::Immediate(Scalar::zst().into()), - // the actual index doesn't matter, so we just pick a convenient one like 0 + // The actual index doesn't matter, so we just pick a convenient one like 0. layout: base.layout.field(self, 0)?, } } else { - // The rest should only occur as mplace, we do not use Immediates for types - // allowing such operations. This matches place_projection forcing an allocation. + // The rest should only occur as mplace, we do not use `Immediate`s for types + // allowing such operations. This matches `place_projection` forcing an allocation. let mplace = base.assert_mem_place(); self.mplace_projection(mplace, proj_elem)?.into() } }) } - /// This is used by [priroda](https://github.com/oli-obk/priroda) to get an OpTy from a local + /// This is used by [priroda](https://github.com/oli-obk/priroda) to get an `OpTy` from a local. pub fn access_local( &self, frame: &super::Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>, @@ -444,7 +444,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { assert_ne!(local, mir::RETURN_PLACE); let layout = self.layout_of_local(frame, local, layout)?; let op = if layout.is_zst() { - // Do not read from ZST, they might not be initialized + // Do not read from ZST; it might not be initialized. Operand::Immediate(Scalar::zst().into()) } else { frame.locals[local].access()? @@ -452,7 +452,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(OpTy { op, layout }) } - /// Every place can be read from, so we can turn them into an operand + /// Converts a place into an operand. This is possible because every place + /// can be read from. #[inline(always)] pub fn place_to_op( &self, @@ -468,7 +469,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(OpTy { op, layout: place.layout }) } - // Evaluate a place with the goal of reading from it. This lets us sometimes + // Evaluates a place with the goal of reading from it. This lets us sometimes // avoid allocations. pub(super) fn eval_place_to_op( &self, @@ -484,7 +485,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { PlaceBase::Local(local) => { // Do not use the layout passed in as argument if the base we are looking at // here is not the entire place. - // FIXME use place_projection.is_empty() when is available + // FIXME: use `place_projection.is_empty()` when available. let layout = if mir_place.projection.is_none() { layout } else { @@ -507,7 +508,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }) } - /// Evaluate the operand, returning a place where you can then find the data. + /// Evaluates the operand, returning a place where you can then find the data. /// If you already know the layout, you can save two table lookups /// by passing it in here. pub fn eval_operand( @@ -517,7 +518,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { use rustc::mir::Operand::*; let op = match *mir_op { - // FIXME: do some more logic on `move` to invalidate the old location + // FIXME: do some more logic on `move` to invalidate the old location. Copy(ref place) | Move(ref place) => self.eval_place_to_op(place, layout)?, @@ -531,7 +532,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(op) } - /// Evaluate a bunch of operands at once + /// Evaluates a bunch of operands at once. pub(super) fn eval_operands( &self, ops: &[mir::Operand<'tcx>], @@ -541,10 +542,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { .collect() } - // Used when the miri-engine runs into a constant and for extracting information from constants - // in patterns via the `const_eval` module - /// The `val` and `layout` are assumed to already be in our interpreter - /// "universe" (param_env). + /// Used when the Miri engine runs into a constant and for extracting information from constants + /// in patterns via the `const_eval` module. + /// The `val` and `layouts` are assumed to already be in our interpreter + /// "universe" (`ParamEnv`). crate fn eval_const_to_op( &self, val: &'tcx ty::Const<'tcx>, @@ -603,7 +604,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(OpTy { op, layout }) } - /// Read discriminant, return the runtime value as well as the variant index. + /// Reads a discriminant and returns the runtime value as well as the variant index. pub fn read_discriminant( &self, rval: OpTy<'tcx, M::PointerTag>, @@ -621,12 +622,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { (discr_kind, discr_index), }; - // read raw discriminant value + // Read raw discriminant value. let discr_op = self.operand_field(rval, discr_index as u64)?; let discr_val = self.read_immediate(discr_op)?; let raw_discr = discr_val.to_scalar_or_undef(); trace!("discr value: {:?}", raw_discr); - // post-process + // Post-process. Ok(match *discr_kind { layout::DiscriminantKind::Tag => { let bits_discr = raw_discr @@ -634,10 +635,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { .and_then(|raw_discr| self.force_bits(raw_discr, discr_val.layout.size)) .map_err(|_| err_unsup!(InvalidDiscriminant(raw_discr.erase_tag())))?; let real_discr = if discr_val.layout.ty.is_signed() { - // going from layout tag type to typeck discriminant type - // requires first sign extending with the layout discriminant + // Going from layout tag type to typeck discriminant type + // requires first sign extending with the layout discriminant... let sexted = sign_extend(bits_discr, discr_val.layout.size) as i128; - // and then zeroing with the typeck discriminant type + // ... and then zeroing with the typeck discriminant type. let discr_ty = rval.layout.ty .ty_adt_def().expect("tagged layout corresponds to adt") .repr @@ -648,7 +649,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } else { bits_discr }; - // Make sure we catch invalid discriminants + // Make sure we catch invalid discriminants. let index = match &rval.layout.ty.sty { ty::Adt(adt, _) => adt .discriminants(self.tcx.tcx) @@ -674,7 +675,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { })?; match raw_discr.to_bits_or_ptr(discr_val.layout.size, self) { Err(ptr) => { - // The niche must be just 0 (which an inbounds pointer value never is) + // The niche must be just 0 (which an inbounds pointer value never is). let ptr_valid = niche_start == 0 && variants_start == variants_end && !self.memory.ptr_may_be_null(ptr); if !ptr_valid { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index f358bb00f4d12..4a22e68a0a429 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -1,4 +1,4 @@ -//! Computations on places -- field projections, going from mir::Place, and writing +//! Computations on places -- field projections, going from `mir::Place`, and writing //! into a place. //! All high-level functions to write to memory work on places as destinations. @@ -17,8 +17,8 @@ use super::{ RawConst, Immediate, ImmTy, ScalarMaybeUndef, Operand, OpTy, MemoryKind, LocalValue, }; -#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -pub struct MemPlace { +#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, RustcEncodable, RustcDecodable)] +pub struct MemPlace { /// A place may have an integral pointer for ZSTs, and since it might /// be turned back into a reference before ever being dereferenced. /// However, it may never be undef. @@ -31,7 +31,7 @@ pub struct MemPlace { } #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] -pub enum Place { +pub enum Place { /// A place referring to a value allocated in the `Memory` system. Ptr(MemPlace), @@ -44,8 +44,8 @@ pub enum Place { } #[derive(Copy, Clone, Debug)] -pub struct PlaceTy<'tcx, Tag=()> { - place: Place, // Keep this private, it helps enforce invariants +pub struct PlaceTy<'tcx, Tag = ()> { + place: Place, // Keep this private; it helps enforce invariants. pub layout: TyLayout<'tcx>, } @@ -57,9 +57,9 @@ impl<'tcx, Tag> ::std::ops::Deref for PlaceTy<'tcx, Tag> { } } -/// A MemPlace with its layout. Constructing it is only possible in this module. +/// A `MemPlace` with its layout. Constructing it is only possible in this module. #[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] -pub struct MPlaceTy<'tcx, Tag=()> { +pub struct MPlaceTy<'tcx, Tag = ()> { mplace: MemPlace, pub layout: TyLayout<'tcx>, } @@ -83,7 +83,7 @@ impl<'tcx, Tag> From> for PlaceTy<'tcx, Tag> { } impl MemPlace { - /// Replace ptr tag, maintain vtable tag (if any) + /// Replaces the `ptr` tag, maintaining the vtable tag (if any). #[inline] pub fn replace_tag(self, new_tag: Tag) -> Self { MemPlace { @@ -111,7 +111,7 @@ impl MemPlace { } } - /// Produces a Place that will error if attempted to be read from or written to + /// Produces a `Place` that will error if attempted to be read from or written to. #[inline(always)] pub fn null(cx: &impl HasDataLayout) -> Self { Self::from_scalar_ptr(Scalar::ptr_null(cx), Align::from_bytes(1).unwrap()) @@ -122,7 +122,7 @@ impl MemPlace { Self::from_scalar_ptr(ptr.into(), align) } - /// Turn a mplace into a (thin or fat) pointer, as a reference, pointing to the same space. + /// Turns a mplace into a (thin or fat) pointer, as a reference, pointing to the same space. /// This is the inverse of `ref_to_mplace`. #[inline(always)] pub fn to_ref(self) -> Immediate { @@ -147,7 +147,7 @@ impl MemPlace { } impl<'tcx, Tag> MPlaceTy<'tcx, Tag> { - /// Produces a MemPlace that works for ZST but nothing else + /// Produces a `MemPlace` that works for ZST but nothing else. #[inline] pub fn dangling(layout: TyLayout<'tcx>, cx: &impl HasDataLayout) -> Self { MPlaceTy { @@ -190,7 +190,7 @@ impl<'tcx, Tag> MPlaceTy<'tcx, Tag> { #[inline] pub(super) fn len(self, cx: &impl HasDataLayout) -> InterpResult<'tcx, u64> { if self.layout.is_unsized() { - // We need to consult `meta` metadata + // We need to consult `meta` metadata. match self.layout.ty.sty { ty::Slice(..) | ty::Str => return self.mplace.meta.unwrap().to_usize(cx), @@ -232,7 +232,7 @@ impl<'tcx, Tag: ::std::fmt::Debug + Copy> OpTy<'tcx, Tag> { } impl Place { - /// Produces a Place that will error if attempted to be read from or written to + /// Produces a `Place` that will error if attempted to be read from or written to. #[inline(always)] pub fn null(cx: &impl HasDataLayout) -> Self { Place::Ptr(MemPlace::null(cx)) @@ -265,7 +265,7 @@ impl<'tcx, Tag: ::std::fmt::Debug> PlaceTy<'tcx, Tag> { } } -// separating the pointer tag for `impl Trait`, see https://github.com/rust-lang/rust/issues/54385 +// Re. separating the pointer tag for `impl Trait`, see https://github.com/rust-lang/rust/issues/54385. impl<'mir, 'tcx, Tag, M> InterpCx<'mir, 'tcx, M> where // FIXME: Working around https://github.com/rust-lang/rust/issues/54385 @@ -275,7 +275,7 @@ where M::MemoryMap: AllocMap, Allocation)>, M::AllocExtra: AllocationExtra, { - /// Take a value, which represents a (thin or fat) reference, and make it a place. + /// Takes a value, which represents a (thin or fat) reference, and make it a place. /// Alignment is just based on the type. This is the inverse of `MemPlace::to_ref()`. /// /// Only call this if you are sure the place is "valid" (aligned and inbounds), or do not @@ -300,7 +300,7 @@ where Ok(MPlaceTy { mplace, layout }) } - /// Take an operand, representing a pointer, and dereference it to a place -- that + /// Takes an operand, representing a pointer, and dereference it to a place -- that /// will always be a MemPlace. Lives in `place.rs` because it creates a place. pub fn deref_operand( &self, @@ -312,7 +312,7 @@ where self.mplace_access_checked(place) } - /// Check if the given place is good for memory access with the given + /// Checks if the given place is good for memory access with the given /// size, falling back to the layout's size if `None` (in the latter case, /// this must be a statically sized type). /// @@ -332,7 +332,7 @@ where self.memory.check_ptr_access(place.ptr, size, place.align) } - /// Return the "access-checked" version of this `MPlace`, where for non-ZST + /// Returns the "access-checked" version of this `MPlace`, where for non-ZSTs /// this is definitely a `Pointer`. pub fn mplace_access_checked( &self, @@ -341,7 +341,7 @@ where let (size, align) = self.size_and_align_of_mplace(place)? .unwrap_or((place.layout.size, place.layout.align.abi)); assert!(place.mplace.align <= align, "dynamic alignment less strict than static one?"); - place.mplace.align = align; // maximally strict checking + place.mplace.align = align; // maximally-strict checking // When dereferencing a pointer, it must be non-NULL, aligned, and live. if let Some(ptr) = self.check_mplace_access(place, Some(size))? { place.mplace.ptr = ptr.into(); @@ -349,7 +349,7 @@ where Ok(place) } - /// Force `place.ptr` to a `Pointer`. + /// Forces `place.ptr` to a `Pointer`. /// Can be helpful to avoid lots of `force_ptr` calls later, if this place is used a lot. pub fn force_mplace_ptr( &self, @@ -359,7 +359,7 @@ where Ok(place) } - /// Offset a pointer to project to a field. Unlike `place_field`, this is always + /// Offsets a pointer to project to a field. Unlike `place_field`, this is always /// possible without allocating, so it can take `&self`. Also return the field's layout. /// This supports both struct and array fields. #[inline(always)] @@ -368,7 +368,7 @@ where base: MPlaceTy<'tcx, M::PointerTag>, field: u64, ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - // Not using the layout method because we want to compute on u64 + // Not using the layout method because we want to compute on `u64`. let offset = match base.layout.fields { layout::FieldPlacement::Arbitrary { ref offsets, .. } => offsets[usize::try_from(field).unwrap()], @@ -385,11 +385,11 @@ where layout::FieldPlacement::Union(count) => { assert!(field < count as u64, "Tried to access field {} of union with {} fields", field, count); - // Offset is always 0 + // Offset is always 0. Size::from_bytes(0) } }; - // the only way conversion can fail if is this is an array (otherwise we already panicked + // The only way conversion can fail if is this is an array (otherwise we already panicked // above). In that case, all fields are equal. let field_layout = base.layout.field(self, usize::try_from(field).unwrap_or(0))?; @@ -428,7 +428,7 @@ where base: MPlaceTy<'tcx, Tag>, ) -> InterpResult<'tcx, impl Iterator>> + 'tcx> { - let len = base.len(self)?; // also asserts that we have a type where this makes sense + let len = base.len(self)?; // Also asserts that we have a type where this makes sense. let stride = match base.layout.fields { layout::FieldPlacement::Array { stride, .. } => stride, _ => bug!("mplace_array_fields: expected an array layout"), @@ -455,7 +455,7 @@ where _ => bug!("Unexpected layout of index access: {:#?}", base.layout), }; - // Compute meta and new layout + // Compute meta and new layout. let inner_len = len - to - from; let (meta, ty) = match base.layout.ty.sty { // It is not nice to match on the type, but that seems to be the only way to @@ -478,12 +478,12 @@ where base: MPlaceTy<'tcx, M::PointerTag>, variant: VariantIdx, ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - // Downcasts only change the layout + // Downcasts only change the layout. assert!(base.meta.is_none()); Ok(MPlaceTy { layout: base.layout.for_variant(self, variant), ..base }) } - /// Project into an mplace + /// Projects into an mplace. pub fn mplace_projection( &self, base: MPlaceTy<'tcx, M::PointerTag>, @@ -576,8 +576,8 @@ where }) } - /// Evaluate statics and promoteds to an `MPlace`. Used to share some code between - /// `eval_place` and `eval_place_to_op`. + /// Evaluate statics and promoteds to an `MPlace`. + /// This is used to share some code between `eval_place` and `eval_place_to_op`. pub(super) fn eval_static_to_mplace( &self, place_static: &mir::Static<'tcx> @@ -604,20 +604,20 @@ where promoted: None }; // Just create a lazy reference, so we can support recursive statics. - // tcx takes care of assigning every static one and only one unique AllocId. + // tcx takes care of assigning every static one and only one unique `AllocId`. // When the data here is ever actually used, memory will notice, // and it knows how to deal with alloc_id that are present in the // global table but not in its local memory: It calls back into tcx through // a query, triggering the CTFE machinery to actually turn this lazy reference - // into a bunch of bytes. IOW, statics are evaluated with CTFE even when - // this InterpCx uses another Machine (e.g., in miri). This is what we - // want! This way, computing statics works consistently between codegen + // into a bunch of bytes. That is, statics are evaluated with CTFE even when + // this `InterpCx` uses another `Machine` (e.g., in miri). This is what we + // want! This way, computing statics works consistently between codegen // and miri: They use the same query to eventually obtain a `ty::Const` // and use that for further computation. // - // Notice that statics have *two* AllocIds: the lazy one, and the resolved - // one. Here we make sure that the interpreted program never sees the - // resolved ID. Also see the doc comment of `Memory::get_static_alloc`. + // Notice that statics have *two* `AllocId`s: the lazy one, and the resolved + // one. Here we make sure that the interpreted program never sees the + // resolved ID. Also see the doc comment of `Memory::get_static_alloc`. let alloc_id = self.tcx.alloc_map.lock().create_static_alloc(cid.instance.def_id()); let ptr = self.tag_static_base_pointer(Pointer::from(alloc_id)); MPlaceTy::from_aligned_ptr(ptr, layout) @@ -625,8 +625,7 @@ where }) } - /// Computes a place. You should only use this if you intend to write into this - /// place; for reading, a more efficient alternative is `eval_place_for_read`. + /// Computes a place. pub fn eval_place( &mut self, mir_place: &mir::Place<'tcx>, @@ -651,7 +650,7 @@ where None => throw_unsup!(InvalidNullPointerUsage), }, PlaceBase::Local(local) => PlaceTy { - // This works even for dead/uninitialized locals; we check further when writing + // This works even for dead/uninitialized locals; we check further when writing. place: Place::Local { frame: self.cur_frame(), local: *local, @@ -670,7 +669,7 @@ where }) } - /// Write a scalar to a place + /// Writes a scalar to a place. pub fn write_scalar( &mut self, val: impl Into>, @@ -679,7 +678,7 @@ where self.write_immediate(Immediate::Scalar(val.into()), dest) } - /// Write an immediate to a place + /// Writes an immediate to a place. #[inline(always)] pub fn write_immediate( &mut self, @@ -689,14 +688,14 @@ where self.write_immediate_no_validate(src, dest)?; if M::enforce_validity(self) { - // Data got changed, better make sure it matches the type! + // Data got changed; better make sure it matches the type! self.validate_operand(self.place_to_op(dest)?, vec![], None)?; } Ok(()) } - /// Write an `Immediate` to memory. + /// Writes an `Immediate` to memory. #[inline(always)] pub fn write_immediate_to_mplace( &mut self, @@ -713,7 +712,7 @@ where Ok(()) } - /// Write an immediate to a place. + /// Writes an immediate to a place. /// If you use this you are responsible for validating that things got copied at the /// right type. fn write_immediate_no_validate( @@ -763,7 +762,7 @@ where self.write_immediate_to_mplace_no_validate(src, dest) } - /// Write an immediate to memory. + /// Writes an immediate to memory. /// If you use this you are responsible for validating that things got copied at the /// right type. fn write_immediate_to_mplace_no_validate( @@ -895,7 +894,7 @@ where src_ptr, dest_ptr, size, - /*nonoverlapping*/ true, + /* nonoverlapping */ true, ) } @@ -992,12 +991,12 @@ where LocalValue::Live(Operand::Indirect(mplace)); (mplace, Some(size)) } - Err(mplace) => (mplace, None), // this already was an indirect local + Err(mplace) => (mplace, None), // This was already an indirect local. } } Place::Ptr(mplace) => (mplace, None) }; - // Return with the original layout, so that the caller can go on + // Return with the original layout, so that the caller can go on. Ok((MPlaceTy { mplace, layout: place.layout }, size)) } @@ -1037,9 +1036,9 @@ where let discr_val = dest.layout.ty.discriminant_for_variant(*self.tcx, variant_index).unwrap().val; - // raw discriminants for enums are isize or bigger during + // Raw discriminants for enums are isize or bigger during // their computation, but the in-memory tag is the smallest possible - // representation + // representation. let size = discr.value.size(self); let discr_val = truncate(discr_val, size); @@ -1079,7 +1078,7 @@ where &self, raw: RawConst<'tcx>, ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - // This must be an allocation in `tcx` + // This must be an allocation in `tcx`. assert!(self.tcx.alloc_map.lock().get(raw.alloc_id).is_some()); let ptr = self.tag_static_base_pointer(Pointer::from(raw.alloc_id)); let layout = self.layout_of(raw.ty)?; @@ -1090,7 +1089,7 @@ where /// Also return some more information so drop doesn't have to run the same code twice. pub(super) fn unpack_dyn_trait(&self, mplace: MPlaceTy<'tcx, M::PointerTag>) -> InterpResult<'tcx, (ty::Instance<'tcx>, MPlaceTy<'tcx, M::PointerTag>)> { - let vtable = mplace.vtable(); // also sanity checks the type + let vtable = mplace.vtable(); // Also sanity checks the type. let (instance, ty) = self.read_drop_type_from_vtable(vtable)?; let layout = self.layout_of(ty)?; @@ -1098,7 +1097,7 @@ where if cfg!(debug_assertions) { let (size, align) = self.read_size_and_align_from_vtable(vtable)?; assert_eq!(size, layout.size); - // only ABI alignment is preserved + // Only ABI alignment is preserved. assert_eq!(align, layout.align.abi); } diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index ca4da451a1f2d..cb4880fa2890f 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -75,7 +75,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { use rustc::mir::StatementKind::*; - // Some statements (e.g., box) push new stack frames. + // Some statements (e.g., `box`) push new stack frames. // We have to record the stack frame number *before* executing the statement. let frame_idx = self.cur_frame(); self.tcx.span = stmt.source_info.span; @@ -92,19 +92,19 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.write_discriminant_index(variant_index, dest)?; } - // Mark locals as alive + // Mark locals as alive. StorageLive(local) => { let old_val = self.storage_live(local)?; self.deallocate_local(old_val)?; } - // Mark locals as dead + // Mark locals as dead. StorageDead(local) => { let old_val = self.storage_dead(local); self.deallocate_local(old_val)?; } - // No dynamic semantics attached to `FakeRead`; MIR + // No dynamic semantics attached to FakeRead; MIR // interpreter is solely intended for borrowck'ed code. FakeRead(..) => {} @@ -128,7 +128,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } - /// Evaluate an assignment statement. + /// Evaluates an assignment statement. /// /// There is no separate `eval_rvalue` function. Instead, the code for handling each rvalue /// type writes its results directly into the memory specified by the place. @@ -142,7 +142,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { use rustc::mir::Rvalue::*; match *rvalue { Use(ref operand) => { - // Avoid recomputing the layout + // Avoid recomputing the layout. let op = self.eval_operand(operand, Some(dest.layout))?; self.copy_op(op, dest)?; } @@ -221,14 +221,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // for big static/const arrays! let rest_ptr = first_ptr.offset(elem_size, self)?; self.memory.copy_repeatedly( - first_ptr, rest_ptr, elem_size, length - 1, /*nonoverlapping:*/true + first_ptr, rest_ptr, elem_size, length - 1, /* nonoverlapping: */ true )?; } } } Len(ref place) => { - // FIXME(CTFE): don't allow computing the length of arrays in const eval + // FIXME(CTFE): don't allow computing the length of arrays in const eval. let src = self.eval_place(place)?; let mplace = self.force_allocation(src)?; let len = mplace.len(self)?; @@ -292,7 +292,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let old_bb = self.frame().block; self.eval_terminator(terminator)?; if !self.stack.is_empty() { - // This should change *something* + // This should change *something*. debug_assert!(self.cur_frame() != old_stack || self.frame().block != old_bb); info!("// {:?}", self.frame().block); } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 5de297923ce7b..507df2265a5e9 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -295,7 +295,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - // We need MIR for this fn + // We need MIR for this fn. let body = match M::find_fn(self, instance, args, dest, ret)? { Some(body) => body, None => return Ok(()), @@ -347,7 +347,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if caller_abi == Abi::RustCall && !args.is_empty() { // Untuple let (&untuple_arg, args) = args.split_last().unwrap(); - trace!("eval_fn_call: Will pass last argument by untupling"); + trace!("eval_fn_call: will pass last argument by untupling"); Cow::from(args.iter().map(|&a| Ok(a)) .chain((0..untuple_arg.layout.fields.count()).into_iter() .map(|i| self.operand_field(untuple_arg, i as u64)) @@ -476,7 +476,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { span: Span, target: mir::BasicBlock, ) -> InterpResult<'tcx> { - trace!("drop_in_place: {:?},\n {:?}, {:?}", *place, place.layout.ty, instance); + trace!( + "drop_in_place: {:?},\n\ + \t{:?}, {:?}", + *place, place.layout.ty, instance + ); + // We take the address of the object. This may well be unaligned, which is fine // for us here. However, unaligned accesses will probably make the actual drop // implementation fail -- a problem shared by rustc. diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 10b767ebba191..c5acdcde0bb1b 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -10,7 +10,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// /// The `trait_ref` encodes the erased self type. Hence if we are /// making an object `Foo` from a value of type `Foo`, then - /// `trait_ref` would map `T:Trait`. + /// `trait_ref` would map `T: Trait`. pub fn get_vtable( &mut self, ty: Ty<'tcx>, @@ -51,7 +51,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let ptr_align = self.tcx.data_layout.pointer_align.abi; // ///////////////////////////////////////////////////////////////////////////////////////// // If you touch this code, be sure to also make the corresponding changes to - // `get_vtable` in rust_codegen_llvm/meth.rs + // `get_vtable` in `rust_codegen_llvm/meth.rs`. // ///////////////////////////////////////////////////////////////////////////////////////// let vtable = self.memory.allocate( ptr_size * (3 + methods.len() as u64), @@ -102,12 +102,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(vtable) } - /// Returns the drop fn instance as well as the actual dynamic type + /// Returns the drop fn instance as well as the actual dynamic type. pub fn read_drop_type_from_vtable( &self, vtable: Scalar, ) -> InterpResult<'tcx, (ty::Instance<'tcx>, Ty<'tcx>)> { - // we don't care about the pointee type, we just want a pointer + // We don't care about the pointee type; we just want a pointer. let vtable = self.memory.check_ptr_access( vtable, self.tcx.data_layout.pointer_size, @@ -133,7 +133,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { vtable: Scalar, ) -> InterpResult<'tcx, (Size, Align)> { let pointer_size = self.pointer_size(); - // We check for size = 3*ptr_size, that covers the drop fn (unused here), + // We check for `size = 3 * ptr_size`, which covers the drop fn (unused here), // the size, and the align (which we read below). let vtable = self.memory.check_ptr_access( vtable, diff --git a/src/librustc_mir/transform/add_retag.rs b/src/librustc_mir/transform/add_retag.rs index 0fd75cd57b2ac..ab82e356d1c85 100644 --- a/src/librustc_mir/transform/add_retag.rs +++ b/src/librustc_mir/transform/add_retag.rs @@ -121,7 +121,7 @@ impl<'tcx> MirPass<'tcx> for AddRetag { } TerminatorKind::Drop { .. } | TerminatorKind::DropAndReplace { .. } => { - // `Drop` is also a call, but it doesn't return anything so we are good. + // Drop is also a call, but it doesn't return anything so we are good. } _ => { // Not a block ending in a Call -> ignore. diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index d87331195dd24..700b08f5036ed 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -257,7 +257,7 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> { context: PlaceContext, location: Location) { if let PlaceBase::Local(l) = place.base { - // Replace an Local in the remap with a generator struct access + // Replace an `Local` in the remap with a generator struct access. if let Some(&(ty, variant_index, idx)) = self.remap.get(&l) { replace_base(place, self.make_field(variant_index, idx, ty)); } @@ -269,7 +269,7 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> { fn visit_basic_block_data(&mut self, block: BasicBlock, data: &mut BasicBlockData<'tcx>) { - // Remove StorageLive and StorageDead statements for remapped locals + // Remove StorageLive and StorageDead statements for remapped locals. data.retain_statements(|s| { match s.kind { StatementKind::StorageLive(l) | StatementKind::StorageDead(l) => { @@ -293,7 +293,7 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> { if let Some((state_idx, resume, v, drop)) = ret_val { let source_info = data.terminator().source_info; - // We must assign the value first in case it gets declared dead below + // We must assign the value first in case it gets declared dead below. data.statements.push(Statement { source_info, kind: StatementKind::Assign(Place::RETURN_PLACE, @@ -340,10 +340,10 @@ fn make_generator_state_argument_indirect<'tcx>( mutbl: hir::MutMutable }); - // Replace the by value generator argument + // Replace the by value generator argument. body.local_decls.raw[1].ty = ref_gen_ty; - // Add a deref to accesses of the generator state + // Add a deref to accesses of the generator state. DerefArgVisitor.visit_body(body); } @@ -358,7 +358,7 @@ fn make_generator_state_argument_pinned<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body // Replace the by ref generator argument body.local_decls.raw[1].ty = pin_ref_gen_ty; - // Add the Pin field access to accesses of the generator state + // Add the `Pin` field access to accesses of the generator state. PinArgVisitor { ref_gen_ty }.visit_body(body); } @@ -408,7 +408,7 @@ struct LivenessInfo { /// Which locals are live across any suspension point. /// /// GeneratorSavedLocal is indexed in terms of the elements in this set; - /// i.e. GeneratorSavedLocal::new(1) corresponds to the second local + /// i.e., `GeneratorSavedLocal::new(1)` corresponds to the second local /// included in this set. live_locals: liveness::LiveVarSet, @@ -453,8 +453,7 @@ fn locals_live_across_suspend_points( do_dataflow(tcx, body, def_id, &[], &dead_unwinds, borrowed_locals_analysis, |bd, p| DebugFormatted::new(&bd.body().local_decls[p])); - // Calculate the MIR locals that we actually need to keep storage around - // for. + // Calculate the MIR locals that we actually need to keep storage around for. let requires_storage_analysis = RequiresStorage::new(body, &borrowed_locals_result); let requires_storage = do_dataflow(tcx, body, def_id, &[], &dead_unwinds, requires_storage_analysis, @@ -711,13 +710,13 @@ fn compute_layout<'tcx>( GeneratorLayout<'tcx>, FxHashMap, ) { - // Use a liveness analysis to compute locals which are live across a suspension point + // Use a liveness analysis to compute locals which are live across a suspension point. let LivenessInfo { live_locals, live_locals_at_suspension_points, storage_conflicts, storage_liveness } = locals_live_across_suspend_points(tcx, body, source, movable); // Erase regions from the types passed in from typeck so we can compare them with - // MIR types + // MIR types. let allowed_upvars = tcx.erase_regions(upvars); let allowed = match interior.sty { ty::GeneratorWitness(s) => tcx.erase_late_bound_regions(&s), @@ -725,13 +724,13 @@ fn compute_layout<'tcx>( }; for (local, decl) in body.local_decls.iter_enumerated() { - // Ignore locals which are internal or not live + // Ignore locals which are internal or not live. if !live_locals.contains(local) || decl.internal { continue; } // Sanity check that typeck knows about the type of locals which are - // live across a suspension point + // live across a suspension point. if !allowed.contains(&decl.ty) && !allowed_upvars.contains(&decl.ty) { span_bug!(body.span, "Broken MIR: generator contains type {} in MIR, \ @@ -892,7 +891,7 @@ fn create_generator_drop_shim<'tcx>( cases.insert(0, (UNRESUMED, drop_clean)); // The returned state and the poisoned state fall through to the default - // case which is just to return + // case which is just to return. insert_switch(&mut body, cases, &transform, TerminatorKind::Return); @@ -903,7 +902,7 @@ fn create_generator_drop_shim<'tcx>( } } - // Replace the return variable + // Replace the return variable. body.local_decls[RETURN_PLACE] = LocalDecl { mutability: Mutability::Mut, ty: tcx.mk_unit(), @@ -918,7 +917,7 @@ fn create_generator_drop_shim<'tcx>( make_generator_state_argument_indirect(tcx, def_id, &mut body); - // Change the generator argument from &mut to *mut + // Change the generator argument from `&mut` to `*mut`. body.local_decls[self_arg()] = LocalDecl { mutability: Mutability::Mut, ty: tcx.mk_ptr(ty::TypeAndMut { @@ -934,7 +933,7 @@ fn create_generator_drop_shim<'tcx>( is_user_variable: None, }; if tcx.sess.opts.debugging_opts.mir_emit_retag { - // Alias tracking must know we changed the type + // Alias tracking must know we changed the type. body.basic_blocks_mut()[START_BLOCK].statements.insert(0, Statement { source_info, kind: StatementKind::Retag(RetagKind::Raw, Place::from(self_arg())), @@ -944,7 +943,7 @@ fn create_generator_drop_shim<'tcx>( no_landing_pads(tcx, &mut body); // Make sure we remove dead blocks to remove - // unrelated code from the resume part of the function + // unrelated code from the resume part of the function. simplify::remove_dead_blocks(&mut body); dump_mir(tcx, None, "generator_drop", &0, source, &mut body, |_, _| Ok(()) ); diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index f31303c642faa..ca2e788d3a8eb 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -665,7 +665,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> { *local = l; return; }, - ref place => bug!("Return place is {:?}, not local", place) + ref place => bug!("return place is {:?}, not local", place) } } let idx = local.index() - 1; diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index ac291c2996d06..a57e20e4272ef 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -1,4 +1,5 @@ use crate::{build, shim}; + use rustc_data_structures::indexed_vec::IndexVec; use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc::mir::{Body, MirPhase, Promoted}; @@ -66,7 +67,7 @@ fn mir_keys(tcx: TyCtxt<'_>, krate: CrateNum) -> &DefIdSet { set.extend(tcx.body_owners()); // Additionally, tuple struct/variant constructors have MIR, but - // they don't have a BodyId, so we need to build them separately. + // they don't have a `BodyId`, so we need to build them separately. struct GatherCtors<'a, 'tcx> { tcx: TyCtxt<'tcx>, set: &'a mut DefIdSet, @@ -184,7 +185,7 @@ pub fn run_passes( } fn mir_const(tcx: TyCtxt<'_>, def_id: DefId) -> &Steal> { - // Unsafety check uses the raw mir, so make sure it is run + // Unsafety check uses the raw MIR, so make sure it is run. let _ = tcx.unsafety_check_result(def_id); let mut body = tcx.mir_built(def_id).steal(); @@ -226,7 +227,7 @@ fn run_optimization_passes<'tcx>( promoted: Option, ) { run_passes(tcx, body, InstanceDef::Item(def_id), promoted, MirPhase::Optimized, &[ - // Remove all things only needed by analysis + // Remove all things only needed by analysis. &no_landing_pads::NoLandingPads, &simplify_branches::SimplifyBranches::new("initial"), &remove_noop_landing_pads::RemoveNoopLandingPads, @@ -234,7 +235,7 @@ fn run_optimization_passes<'tcx>( &simplify::SimplifyCfg::new("early-opt"), - // These next passes must be executed together + // These next passes must be executed together. &add_call_guards::CriticalCallEdges, &elaborate_drops::ElaborateDrops, &no_landing_pads::NoLandingPads, @@ -242,7 +243,7 @@ fn run_optimization_passes<'tcx>( // elaboration. &add_moves_for_packed_drops::AddMovesForPackedDrops, // AddRetag needs to run after ElaborateDrops, and it needs - // an AllCallEdges pass right before it. Otherwise it should + // an AllCallEdges pass right before it. Otherwise, it should // run fairly late, but before optimizations begin. &add_call_guards::AllCallEdges, &add_retag::AddRetag, @@ -254,7 +255,6 @@ fn run_optimization_passes<'tcx>( // From here on out, regions are gone. &erase_regions::EraseRegions, - // Optimizations begin. &uniform_array_move_out::RestoreSubsliceArrayMoveOut, &inline::Inline, @@ -282,12 +282,12 @@ fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> &Body<'_> { if tcx.is_constructor(def_id) { // There's no reason to run all of the MIR passes on constructors when // we can just output the MIR we want directly. This also saves const - // qualification and borrow checking the trouble of special casing + // qualification and borrow checking the trouble of special-casing // constructors. return shim::build_adt_ctor(tcx, def_id); } - // (Mir-)Borrowck uses `mir_validated`, so we have to force it to + // (MIR-)borrowck uses `mir_validated`, so we have to force it to // execute before we can steal. tcx.ensure().mir_borrowck(def_id); diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index a77421ce15008..1a7ace74f9a48 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -1702,9 +1702,9 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { Checker::new(tcx, def_id, body, mode).check_const().1 }; - // In `const` and `static` everything without `StorageDead` + // In `const` and `static` everything without StorageDead // is `'static`, we don't have to create promoted MIR fragments, - // just remove `Drop` and `StorageDead` on "promoted" locals. + // just remove Drop and StorageDead on "promoted" locals. debug!("run_pass: promoted_temps={:?}", promoted_temps); for block in body.basic_blocks_mut() { block.statements.retain(|statement| { diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index 56093527aee24..b9cfdb4c9ea37 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -217,19 +217,19 @@ fn check_statement( StatementKind::FakeRead(_, place) => check_place(place, span), - // just an assignment + // This is just an assignment. StatementKind::SetDiscriminant { .. } => Ok(()), - | StatementKind::InlineAsm { .. } => { + StatementKind::InlineAsm { .. } => { Err((span, "cannot use inline assembly in const fn".into())) } - // These are all NOPs - | StatementKind::StorageLive(_) - | StatementKind::StorageDead(_) - | StatementKind::Retag { .. } - | StatementKind::AscribeUserType(..) - | StatementKind::Nop => Ok(()), + // These are all nops. + StatementKind::StorageLive(_) | + StatementKind::StorageDead(_) | + StatementKind::Retag { .. } | + StatementKind::AscribeUserType(..) | + StatementKind::Nop => Ok(()), } } diff --git a/src/librustc_mir/transform/remove_noop_landing_pads.rs b/src/librustc_mir/transform/remove_noop_landing_pads.rs index 73089a2106f6b..23016ae82dc7f 100644 --- a/src/librustc_mir/transform/remove_noop_landing_pads.rs +++ b/src/librustc_mir/transform/remove_noop_landing_pads.rs @@ -38,7 +38,7 @@ impl RemoveNoopLandingPads { StatementKind::StorageDead(_) | StatementKind::AscribeUserType(..) | StatementKind::Nop => { - // These are all nops in a landing pad + // These are all nops in a landing pad. } StatementKind::Assign(Place { @@ -46,7 +46,7 @@ impl RemoveNoopLandingPads { projection: None, }, box Rvalue::Use(_)) => { // Writing to a local (e.g., a drop flag) does not - // turn a landing pad to a non-nop + // turn a landing pad to a non-nop. } StatementKind::Assign { .. } | diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index 1d3bf247387a7..3a09ece1ac800 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -162,7 +162,7 @@ fn each_block<'tcx, O>( mir::StatementKind::Nop => continue, mir::StatementKind::SetDiscriminant{ .. } => span_bug!(stmt.source_info.span, - "sanity_check should run before Deaggregator inserts SetDiscriminant"), + "`sanity_check` should run before Deaggregator inserts SetDiscriminant"), }; if place == peek_arg_place { diff --git a/src/librustc_mir/transform/simplify.rs b/src/librustc_mir/transform/simplify.rs index d4599ee08aa46..c10673ee316e8 100644 --- a/src/librustc_mir/transform/simplify.rs +++ b/src/librustc_mir/transform/simplify.rs @@ -289,14 +289,13 @@ pub fn remove_dead_blocks(body: &mut Body<'_>) { } } - pub struct SimplifyLocals; impl<'tcx> MirPass<'tcx> for SimplifyLocals { fn run_pass(&self, tcx: TyCtxt<'tcx>, _: MirSource<'tcx>, body: &mut Body<'tcx>) { let mut marker = DeclMarker { locals: BitSet::new_empty(body.local_decls.len()) }; marker.visit_body(body); - // Return pointer and arguments are always live + // Return pointer and arguments are always live. marker.locals.insert(RETURN_PLACE); for arg in body.args_iter() { marker.locals.insert(arg); @@ -310,13 +309,13 @@ impl<'tcx> MirPass<'tcx> for SimplifyLocals { } let map = make_local_map(&mut body.local_decls, marker.locals); - // Update references to all vars and tmps now + // Update references to all vars and temps now. LocalUpdater { map }.visit_body(body); body.local_decls.shrink_to_fit(); } } -/// Construct the mapping while swapping out unused stuff out from the `vec`. +/// Constructs the mapping while swapping out unused stuff out from the `vec`. fn make_local_map( vec: &mut IndexVec, mask: BitSet, @@ -340,7 +339,7 @@ struct DeclMarker { impl<'tcx> Visitor<'tcx> for DeclMarker { fn visit_local(&mut self, local: &Local, ctx: PlaceContext, _: Location) { - // Ignore storage markers altogether, they get removed along with their otherwise unused + // Ignore storage markers altogether; they get removed along with their otherwise unused // decls. // FIXME: Extend this to all non-uses. if !ctx.is_storage_marker() { diff --git a/src/librustc_mir/util/pretty.rs b/src/librustc_mir/util/pretty.rs index ac2701971dfd5..63aabc39cebd7 100644 --- a/src/librustc_mir/util/pretty.rs +++ b/src/librustc_mir/util/pretty.rs @@ -78,7 +78,7 @@ pub fn dump_mir<'tcx, F>( } let node_path = ty::print::with_forced_impl_filename_line(|| { - // see notes on #41697 below + // See notes on #41697 below. tcx.def_path_str(source.def_id()) }); dump_matched_mir_node( @@ -99,7 +99,7 @@ pub fn dump_enabled<'tcx>(tcx: TyCtxt<'tcx>, pass_name: &str, source: MirSource< Some(ref filters) => filters, }; let node_path = ty::print::with_forced_impl_filename_line(|| { - // see notes on #41697 below + // See notes on #41697 below. tcx.def_path_str(source.def_id()) }); filters.split('|').any(|or_filter| { @@ -109,7 +109,7 @@ pub fn dump_enabled<'tcx>(tcx: TyCtxt<'tcx>, pass_name: &str, source: MirSource< }) } -// #41697 -- we use `with_forced_impl_filename_line()` because +// Issue #41697 -- we use `with_forced_impl_filename_line()` because // `def_path_str()` would otherwise trigger `type_of`, and this can // run while we are already attempting to evaluate `type_of`. @@ -235,7 +235,7 @@ pub(crate) fn create_dump_file( fs::File::create(&file_path) } -/// Write out a human-readable textual representation for the given MIR. +/// Writes out a human-readable textual representation for the given MIR. pub fn write_mir_pretty<'tcx>( tcx: TyCtxt<'tcx>, single: Option, @@ -247,7 +247,7 @@ pub fn write_mir_pretty<'tcx>( )?; writeln!( w, - "// and is subject to change without notice. Knock yourself out." + "// and is subject to change without notice." )?; let mut first = true; @@ -257,7 +257,7 @@ pub fn write_mir_pretty<'tcx>( if first { first = false; } else { - // Put empty lines between all items + // Put empty lines between all items. writeln!(w, "")?; } @@ -298,7 +298,7 @@ where Ok(()) } -/// Write out a human-readable textual representation for the given basic block. +/// Writes out a human-readable textual representation for the given basic block. pub fn write_basic_block<'tcx, F>( tcx: TyCtxt<'tcx>, block: BasicBlock, @@ -465,8 +465,8 @@ fn write_scope_tree( // Local variable types (including the user's name in a comment). for (local, local_decl) in body.local_decls.iter_enumerated() { - if (1..body.arg_count+1).contains(&local.index()) { - // Skip over argument locals, they're printed in the signature. + if (1..(body.arg_count + 1)).contains(&local.index()) { + // Skip over argument locals; they're printed in the signature. continue; } @@ -527,7 +527,7 @@ fn write_scope_tree( Ok(()) } -/// Write out a human-readable textual representation of the MIR's `fn` type and the types of its +/// Writes out a human-readable textual representation of the MIR's `fn` type and the types of its /// local variables (both user-defined bindings and compiler temporaries). pub fn write_mir_intro<'tcx>( tcx: TyCtxt<'tcx>, @@ -538,7 +538,7 @@ pub fn write_mir_intro<'tcx>( write_mir_sig(tcx, src, body, w)?; writeln!(w, "{{")?; - // construct a scope tree and write it out + // Construct a scope tree and write it out. let mut scope_tree: FxHashMap> = Default::default(); for (index, scope_data) in body.source_scopes.iter().enumerate() { if let Some(parent) = scope_data.parent_scope { @@ -615,7 +615,7 @@ fn write_mir_sig( } write!(w, " ")?; - // Next thing that gets printed is the opening { + // The next thing that gets printed is the opening `{`. Ok(()) }