From 98eb29cbba66561cf184f2d7f4277b38bd6b2aad Mon Sep 17 00:00:00 2001 From: Matthew McAllister Date: Sun, 24 May 2020 17:54:30 -0700 Subject: [PATCH 01/20] Fix trait alias inherent impl resolution Fixes #60021 and #72415. --- src/librustc_typeck/check/method/probe.rs | 3 ++- .../issue-60021-assoc-method-resolve.rs | 19 +++++++++++++++++++ .../issue-72415-assoc-const-resolve.rs | 14 ++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/traits/trait-alias/issue-60021-assoc-method-resolve.rs create mode 100644 src/test/ui/traits/trait-alias/issue-72415-assoc-const-resolve.rs diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index e21db9035e25d..33290b5ca82ba 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -795,6 +795,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { fn assemble_inherent_candidates_from_param(&mut self, param_ty: ty::ParamTy) { // FIXME: do we want to commit to this behavior for param bounds? + debug!("assemble_inherent_candidates_from_param(param_ty={:?})", param_ty); let bounds = self.param_env.caller_bounds.iter().filter_map(|predicate| match *predicate { ty::Predicate::Trait(ref trait_predicate, _) => { @@ -949,7 +950,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { import_ids: import_ids.clone(), kind: TraitCandidate(new_trait_ref), }, - true, + false, ); }); } else { diff --git a/src/test/ui/traits/trait-alias/issue-60021-assoc-method-resolve.rs b/src/test/ui/traits/trait-alias/issue-60021-assoc-method-resolve.rs new file mode 100644 index 0000000000000..5e27ed3c6460e --- /dev/null +++ b/src/test/ui/traits/trait-alias/issue-60021-assoc-method-resolve.rs @@ -0,0 +1,19 @@ +// check-pass + +#![feature(trait_alias)] + +trait SomeTrait { + fn map(&self) {} +} + +impl SomeTrait for Option {} + +trait SomeAlias = SomeTrait; + +fn main() { + let x = Some(123); + // This should resolve to the trait impl for Option + Option::map(x, |z| z); + // This should resolve to the trait impl for SomeTrait + SomeTrait::map(&x); +} diff --git a/src/test/ui/traits/trait-alias/issue-72415-assoc-const-resolve.rs b/src/test/ui/traits/trait-alias/issue-72415-assoc-const-resolve.rs new file mode 100644 index 0000000000000..e49125d10249d --- /dev/null +++ b/src/test/ui/traits/trait-alias/issue-72415-assoc-const-resolve.rs @@ -0,0 +1,14 @@ +// check-pass + +#![feature(trait_alias)] + +trait Bounded { const MAX: Self; } + +impl Bounded for u32 { + // This should correctly resolve to the associated const in the inherent impl of u32. + const MAX: Self = u32::MAX; +} + +trait Num = Bounded + Copy; + +fn main() {} From 738f8487fd78a7d860be4e65eee85c9a0e957c13 Mon Sep 17 00:00:00 2001 From: CAD97 Date: Mon, 25 May 2020 15:33:34 -0400 Subject: [PATCH 02/20] stabilize vec_drain_as_slice --- src/liballoc/vec.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index d26cd77aae4b7..9a32b80a81efc 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -2747,19 +2747,25 @@ impl<'a, T> Drain<'a, T> { /// # Examples /// /// ``` - /// # #![feature(vec_drain_as_slice)] /// let mut vec = vec!['a', 'b', 'c']; /// let mut drain = vec.drain(..); /// assert_eq!(drain.as_slice(), &['a', 'b', 'c']); /// let _ = drain.next().unwrap(); /// assert_eq!(drain.as_slice(), &['b', 'c']); /// ``` - #[unstable(feature = "vec_drain_as_slice", reason = "recently added", issue = "58957")] + #[stable(feature = "vec_drain_as_slice", since = "1.46.0")] pub fn as_slice(&self) -> &[T] { self.iter.as_slice() } } +#[stable(feature = "vec_drain_as_slice", since = "1.46.0")] +impl<'a, T> AsRef<[T]> for Drain<'a, T> { + fn as_ref(&self) -> &[T] { + self.as_slice() + } +} + #[stable(feature = "drain", since = "1.6.0")] unsafe impl Sync for Drain<'_, T> {} #[stable(feature = "drain", since = "1.6.0")] From 9cee22c1a401ace6dd8633335b131c47a37d7bac Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 23 May 2020 21:40:55 -0400 Subject: [PATCH 03/20] Display information about captured variable in `FnMut` error Fixes #69446 When we encounter a region error involving an `FnMut` closure, we display a specialized error message. However, we currently do not tell the user which upvar was captured. This makes it difficult to determine the cause of the error, especially when the closure is large. This commit records marks constraints involving closure upvars with `ConstraintCategory::ClosureUpvar`. When we decide to 'blame' a `ConstraintCategory::Return`, we additionall store the captured upvar if we found a `ConstraintCategory::ClosureUpvar` in the path. When generating an error message, we point to relevant spans if we have closure upvar information available. We further customize the message if an `async` closure is being returned, to make it clear that the captured variable is being returned indirectly. --- src/libcore/future/mod.rs | 1 + src/librustc_middle/mir/query.rs | 10 +++- .../diagnostics/conflict_errors.rs | 6 +- .../borrow_check/diagnostics/region_errors.rs | 57 +++++++++++++------ src/librustc_mir/borrow_check/mod.rs | 26 +-------- src/librustc_mir/borrow_check/nll.rs | 3 + src/librustc_mir/borrow_check/path_utils.rs | 38 ++++++++++++- .../borrow_check/region_infer/mod.rs | 18 +++++- .../borrow_check/type_check/mod.rs | 27 +++++++-- .../async-await/issue-69446-fnmut-capture.rs | 22 +++++++ .../issue-69446-fnmut-capture.stderr | 19 +++++++ .../borrowck/borrowck-describe-lvalue.stderr | 3 + src/test/ui/issues/issue-40510-1.stderr | 8 ++- src/test/ui/issues/issue-40510-3.stderr | 4 ++ src/test/ui/issues/issue-49824.stderr | 3 + src/test/ui/nll/issue-53040.stderr | 8 ++- ...ons-return-ref-to-upvar-issue-17403.stderr | 8 ++- 17 files changed, 203 insertions(+), 58 deletions(-) create mode 100644 src/test/ui/async-await/issue-69446-fnmut-capture.rs create mode 100644 src/test/ui/async-await/issue-69446-fnmut-capture.stderr diff --git a/src/libcore/future/mod.rs b/src/libcore/future/mod.rs index 6f6009b47e672..9dbc23f5c04c5 100644 --- a/src/libcore/future/mod.rs +++ b/src/libcore/future/mod.rs @@ -56,6 +56,7 @@ pub const fn from_generator(gen: T) -> impl Future where T: Generator, { + #[rustc_diagnostic_item = "gen_future"] struct GenFuture>(T); // We rely on the fact that async/await futures are immovable in order to create diff --git a/src/librustc_middle/mir/query.rs b/src/librustc_middle/mir/query.rs index 63b8d8c8da782..aed93e86863b9 100644 --- a/src/librustc_middle/mir/query.rs +++ b/src/librustc_middle/mir/query.rs @@ -171,7 +171,7 @@ pub struct ClosureOutlivesRequirement<'tcx> { #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)] #[derive(RustcEncodable, RustcDecodable, HashStable)] pub enum ConstraintCategory { - Return, + Return(ReturnConstraint), Yield, UseAsConst, UseAsStatic, @@ -187,6 +187,7 @@ pub enum ConstraintCategory { SizedBound, Assignment, OpaqueType, + ClosureUpvar(hir::HirId), /// A "boring" constraint (caused by the given location) is one that /// the user probably doesn't want to see described in diagnostics, @@ -204,6 +205,13 @@ pub enum ConstraintCategory { Internal, } +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)] +#[derive(RustcEncodable, RustcDecodable, HashStable)] +pub enum ReturnConstraint { + Normal, + ClosureUpvar(hir::HirId), +} + /// The subject of a `ClosureOutlivesRequirement` -- that is, the thing /// that must outlive some region. #[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable, HashStable)] diff --git a/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs b/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs index 5f1c0911da2bf..b0b5a819983a8 100644 --- a/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs @@ -766,7 +766,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { category: category @ - (ConstraintCategory::Return + (ConstraintCategory::Return(_) | ConstraintCategory::CallArgument | ConstraintCategory::OpaqueType), from_closure: false, @@ -1096,7 +1096,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { opt_place_desc: Option<&String>, ) -> Option> { let return_kind = match category { - ConstraintCategory::Return => "return", + ConstraintCategory::Return(_) => "return", ConstraintCategory::Yield => "yield", _ => return None, }; @@ -1210,7 +1210,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); let msg = match category { - ConstraintCategory::Return | ConstraintCategory::OpaqueType => { + ConstraintCategory::Return(_) | ConstraintCategory::OpaqueType => { format!("{} is returned here", kind) } ConstraintCategory::CallArgument => { diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index e19fab89eabfe..681f999e048c1 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -5,9 +5,9 @@ use rustc_infer::infer::{ error_reporting::nice_region_error::NiceRegionError, error_reporting::unexpected_hidden_region_diagnostic, NLLRegionVariableOrigin, }; -use rustc_middle::mir::ConstraintCategory; +use rustc_middle::mir::{ConstraintCategory, ReturnConstraint}; use rustc_middle::ty::{self, RegionVid, Ty}; -use rustc_span::symbol::kw; +use rustc_span::symbol::{kw, sym}; use rustc_span::Span; use crate::util::borrowck_errors; @@ -26,7 +26,7 @@ impl ConstraintDescription for ConstraintCategory { // Must end with a space. Allows for empty names to be provided. match self { ConstraintCategory::Assignment => "assignment ", - ConstraintCategory::Return => "returning this value ", + ConstraintCategory::Return(_) => "returning this value ", ConstraintCategory::Yield => "yielding this value ", ConstraintCategory::UseAsConst => "using this value as a constant ", ConstraintCategory::UseAsStatic => "using this value as a static ", @@ -37,6 +37,7 @@ impl ConstraintDescription for ConstraintCategory { ConstraintCategory::SizedBound => "proving this value is `Sized` ", ConstraintCategory::CopyBound => "copying this value ", ConstraintCategory::OpaqueType => "opaque type ", + ConstraintCategory::ClosureUpvar(_) => "closure capture ", ConstraintCategory::Boring | ConstraintCategory::BoringNoLocation | ConstraintCategory::Internal => "", @@ -306,8 +307,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { }; let diag = match (category, fr_is_local, outlived_fr_is_local) { - (ConstraintCategory::Return, true, false) if self.is_closure_fn_mut(fr) => { - self.report_fnmut_error(&errci) + (ConstraintCategory::Return(kind), true, false) if self.is_closure_fn_mut(fr) => { + self.report_fnmut_error(&errci, kind) } (ConstraintCategory::Assignment, true, false) | (ConstraintCategory::CallArgument, true, false) => { @@ -347,7 +348,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { /// executing... /// = note: ...therefore, returned references to captured variables will escape the closure /// ``` - fn report_fnmut_error(&self, errci: &ErrorConstraintInfo) -> DiagnosticBuilder<'tcx> { + fn report_fnmut_error( + &self, + errci: &ErrorConstraintInfo, + kind: ReturnConstraint, + ) -> DiagnosticBuilder<'tcx> { let ErrorConstraintInfo { outlived_fr, span, .. } = errci; let mut diag = self @@ -356,19 +361,39 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { .sess .struct_span_err(*span, "captured variable cannot escape `FnMut` closure body"); - // We should check if the return type of this closure is in fact a closure - in that - // case, we can special case the error further. - let return_type_is_closure = - self.regioncx.universal_regions().unnormalized_output_ty.is_closure(); - let message = if return_type_is_closure { - "returns a closure that contains a reference to a captured variable, which then \ - escapes the closure body" - } else { - "returns a reference to a captured variable which escapes the closure body" + let mut output_ty = self.regioncx.universal_regions().unnormalized_output_ty; + if let ty::Opaque(def_id, _) = output_ty.kind { + output_ty = self.infcx.tcx.type_of(def_id) + }; + + debug!("report_fnmut_error: output_ty={:?}", output_ty); + + let message = match output_ty.kind { + ty::Closure(_, _) => { + "returns a closure that contains a reference to a captured variable, which then \ + escapes the closure body" + } + ty::Adt(def, _) if self.infcx.tcx.is_diagnostic_item(sym::gen_future, def.did) => { + "returns an `async` block that contains a reference to a captured variable, which then \ + escapes the closure body" + } + _ => "returns a reference to a captured variable which escapes the closure body", }; diag.span_label(*span, message); + if let ReturnConstraint::ClosureUpvar(upvar) = kind { + let def_id = match self.regioncx.universal_regions().defining_ty { + DefiningTy::Closure(def_id, _) => def_id, + ty @ _ => bug!("unexpected DefiningTy {:?}", ty), + }; + + let upvar_def_span = self.infcx.tcx.hir().span(upvar); + let upvar_span = self.infcx.tcx.upvars_mentioned(def_id).unwrap()[&upvar].span; + diag.span_label(upvar_def_span, "variable defined here"); + diag.span_label(upvar_span, "variable captured here"); + } + match self.give_region_a_name(*outlived_fr).unwrap().source { RegionNameSource::NamedEarlyBoundRegion(fr_span) | RegionNameSource::NamedFreeRegion(fr_span) @@ -506,7 +531,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { outlived_fr_name.highlight_region_name(&mut diag); match (category, outlived_fr_is_local, fr_is_local) { - (ConstraintCategory::Return, true, _) => { + (ConstraintCategory::Return(_), true, _) => { diag.span_label( *span, format!( diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index a0c1d96bb4743..78d88ac81aaf8 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -218,6 +218,7 @@ fn do_mir_borrowck<'a, 'tcx>( &mut flow_inits, &mdpe.move_data, &borrow_set, + &upvars, ); // Dump MIR results into a file, if that is enabled. This let us @@ -2301,30 +2302,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { /// be `self` in the current MIR, because that is the only time we directly access the fields /// of a closure type. pub fn is_upvar_field_projection(&self, place_ref: PlaceRef<'tcx>) -> Option { - let mut place_projection = place_ref.projection; - let mut by_ref = false; - - if let [proj_base @ .., ProjectionElem::Deref] = place_projection { - place_projection = proj_base; - by_ref = true; - } - - match place_projection { - [base @ .., ProjectionElem::Field(field, _ty)] => { - let tcx = self.infcx.tcx; - let base_ty = Place::ty_from(place_ref.local, base, self.body(), tcx).ty; - - if (base_ty.is_closure() || base_ty.is_generator()) - && (!by_ref || self.upvars[field.index()].by_ref) - { - Some(*field) - } else { - None - } - } - - _ => None, - } + path_utils::is_upvar_field_projection(self.infcx.tcx, &self.upvars, place_ref, self.body()) } } diff --git a/src/librustc_mir/borrow_check/nll.rs b/src/librustc_mir/borrow_check/nll.rs index b820b79c47fe8..5bae31cafb953 100644 --- a/src/librustc_mir/borrow_check/nll.rs +++ b/src/librustc_mir/borrow_check/nll.rs @@ -39,6 +39,7 @@ use crate::borrow_check::{ renumber, type_check::{self, MirTypeckRegionConstraints, MirTypeckResults}, universal_regions::UniversalRegions, + Upvar, }; crate type PoloniusOutput = Output; @@ -166,6 +167,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( flow_inits: &mut ResultsCursor<'cx, 'tcx, MaybeInitializedPlaces<'cx, 'tcx>>, move_data: &MoveData<'tcx>, borrow_set: &BorrowSet<'tcx>, + upvars: &[Upvar], ) -> NllOutput<'tcx> { let mut all_facts = AllFacts::enabled(infcx.tcx).then_some(AllFacts::default()); @@ -188,6 +190,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( flow_inits, move_data, elements, + upvars, ); if let Some(all_facts) = &mut all_facts { diff --git a/src/librustc_mir/borrow_check/path_utils.rs b/src/librustc_mir/borrow_check/path_utils.rs index f5238e7b7bedc..934729553a73b 100644 --- a/src/librustc_mir/borrow_check/path_utils.rs +++ b/src/librustc_mir/borrow_check/path_utils.rs @@ -1,10 +1,11 @@ use crate::borrow_check::borrow_set::{BorrowData, BorrowSet, TwoPhaseActivation}; use crate::borrow_check::places_conflict; use crate::borrow_check::AccessDepth; +use crate::borrow_check::Upvar; use crate::dataflow::indexes::BorrowIndex; use rustc_data_structures::graph::dominators::Dominators; use rustc_middle::mir::BorrowKind; -use rustc_middle::mir::{BasicBlock, Body, Location, Place}; +use rustc_middle::mir::{BasicBlock, Body, Field, Location, Place, PlaceRef, ProjectionElem}; use rustc_middle::ty::TyCtxt; /// Returns `true` if the borrow represented by `kind` is @@ -135,3 +136,38 @@ pub(super) fn borrow_of_local_data(place: Place<'_>) -> bool { // Any errors will be caught on the initial borrow !place.is_indirect() } + +/// If `place` is a field projection, and the field is being projected from a closure type, +/// then returns the index of the field being projected. Note that this closure will always +/// be `self` in the current MIR, because that is the only time we directly access the fields +/// of a closure type. +pub(crate) fn is_upvar_field_projection( + tcx: TyCtxt<'tcx>, + upvars: &[Upvar], + place_ref: PlaceRef<'tcx>, + body: &Body<'tcx>, +) -> Option { + let mut place_projection = place_ref.projection; + let mut by_ref = false; + + if let [proj_base @ .., ProjectionElem::Deref] = place_projection { + place_projection = proj_base; + by_ref = true; + } + + match place_projection { + [base @ .., ProjectionElem::Field(field, _ty)] => { + let base_ty = Place::ty_from(place_ref.local, base, body, tcx).ty; + + if (base_ty.is_closure() || base_ty.is_generator()) + && (!by_ref || upvars[field.index()].by_ref) + { + Some(*field) + } else { + None + } + } + + _ => None, + } +} diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index fe11384380086..3e459bd52f757 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -12,7 +12,7 @@ use rustc_infer::infer::region_constraints::{GenericKind, VarInfos, VerifyBound} use rustc_infer::infer::{InferCtxt, NLLRegionVariableOrigin, RegionVariableOrigin}; use rustc_middle::mir::{ Body, ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureRegionRequirements, - ConstraintCategory, Local, Location, + ConstraintCategory, Local, Location, ReturnConstraint, }; use rustc_middle::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable}; use rustc_span::Span; @@ -2017,7 +2017,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { | ConstraintCategory::BoringNoLocation | ConstraintCategory::Internal => false, ConstraintCategory::TypeAnnotation - | ConstraintCategory::Return + | ConstraintCategory::Return(_) | ConstraintCategory::Yield => true, _ => constraint_sup_scc != target_scc, } @@ -2042,7 +2042,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { if let Some(i) = best_choice { if let Some(next) = categorized_path.get(i + 1) { - if categorized_path[i].0 == ConstraintCategory::Return + if matches!(categorized_path[i].0, ConstraintCategory::Return(_)) && next.0 == ConstraintCategory::OpaqueType { // The return expression is being influenced by the return type being @@ -2050,6 +2050,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { return *next; } } + + if categorized_path[i].0 == ConstraintCategory::Return(ReturnConstraint::Normal) { + let field = categorized_path.iter().find_map(|p| { + if let ConstraintCategory::ClosureUpvar(f) = p.0 { Some(f) } else { None } + }); + + if let Some(field) = field { + categorized_path[i].0 = + ConstraintCategory::Return(ReturnConstraint::ClosureUpvar(field)); + } + } + return categorized_path[i]; } diff --git a/src/librustc_mir/borrow_check/type_check/mod.rs b/src/librustc_mir/borrow_check/type_check/mod.rs index bdbce1de745ad..d6b06bc1dfa64 100644 --- a/src/librustc_mir/borrow_check/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/type_check/mod.rs @@ -55,6 +55,7 @@ use crate::borrow_check::{ location::LocationTable, member_constraints::MemberConstraintSet, nll::ToRegionVid, + path_utils, region_infer::values::{ LivenessValues, PlaceholderIndex, PlaceholderIndices, RegionValueElements, }, @@ -62,6 +63,7 @@ use crate::borrow_check::{ renumber, type_check::free_region_relations::{CreateResult, UniversalRegionRelations}, universal_regions::{DefiningTy, UniversalRegions}, + Upvar, }; macro_rules! span_mirbug { @@ -132,6 +134,7 @@ pub(crate) fn type_check<'mir, 'tcx>( flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, move_data: &MoveData<'tcx>, elements: &Rc, + upvars: &[Upvar], ) -> MirTypeckResults<'tcx> { let implicit_region_bound = infcx.tcx.mk_region(ty::ReVar(universal_regions.fr_fn_body)); let mut constraints = MirTypeckRegionConstraints { @@ -162,6 +165,7 @@ pub(crate) fn type_check<'mir, 'tcx>( borrow_set, all_facts, constraints: &mut constraints, + upvars, }; let opaque_type_values = type_check_internal( @@ -577,7 +581,7 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> { for constraint in constraints.outlives().iter() { let mut constraint = *constraint; constraint.locations = locations; - if let ConstraintCategory::Return + if let ConstraintCategory::Return(_) | ConstraintCategory::UseAsConst | ConstraintCategory::UseAsStatic = constraint.category { @@ -827,6 +831,7 @@ struct BorrowCheckContext<'a, 'tcx> { all_facts: &'a mut Option, borrow_set: &'a BorrowSet<'tcx>, constraints: &'a mut MirTypeckRegionConstraints<'tcx>, + upvars: &'a [Upvar], } crate struct MirTypeckResults<'tcx> { @@ -1420,7 +1425,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ConstraintCategory::UseAsConst } } else { - ConstraintCategory::Return + ConstraintCategory::Return(ReturnConstraint::Normal) } } Some(l) if !body.local_decls[l].is_user_variable() => { @@ -1703,7 +1708,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ConstraintCategory::UseAsConst } } else { - ConstraintCategory::Return + ConstraintCategory::Return(ReturnConstraint::Normal) } } Some(l) if !body.local_decls[l].is_user_variable() => { @@ -2487,6 +2492,19 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ); let mut cursor = borrowed_place.projection.as_ref(); + let tcx = self.infcx.tcx; + let field = path_utils::is_upvar_field_projection( + tcx, + &self.borrowck_context.upvars, + borrowed_place.as_ref(), + body, + ); + let category = if let Some(field) = field { + ConstraintCategory::ClosureUpvar(self.borrowck_context.upvars[field.index()].var_hir_id) + } else { + ConstraintCategory::Boring + }; + while let [proj_base @ .., elem] = cursor { cursor = proj_base; @@ -2494,7 +2512,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { match elem { ProjectionElem::Deref => { - let tcx = self.infcx.tcx; let base_ty = Place::ty_from(borrowed_place.local, proj_base, body, tcx).ty; debug!("add_reborrow_constraint - base_ty = {:?}", base_ty); @@ -2504,7 +2521,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { sup: ref_region.to_region_vid(), sub: borrow_region.to_region_vid(), locations: location.to_locations(), - category: ConstraintCategory::Boring, + category, }); match mutbl { diff --git a/src/test/ui/async-await/issue-69446-fnmut-capture.rs b/src/test/ui/async-await/issue-69446-fnmut-capture.rs new file mode 100644 index 0000000000000..842115538c9d8 --- /dev/null +++ b/src/test/ui/async-await/issue-69446-fnmut-capture.rs @@ -0,0 +1,22 @@ +// Regression test for issue #69446 - we should display +// which variable is captured +// edition:2018 + +use core::future::Future; + +struct Foo; +impl Foo { + fn foo(&mut self) {} +} + +async fn bar(_: impl FnMut() -> T) +where + T: Future, +{} + +fn main() { + let mut x = Foo; + bar(move || async { //~ ERROR captured + x.foo(); + }); +} diff --git a/src/test/ui/async-await/issue-69446-fnmut-capture.stderr b/src/test/ui/async-await/issue-69446-fnmut-capture.stderr new file mode 100644 index 0000000000000..3d2b0402bc52c --- /dev/null +++ b/src/test/ui/async-await/issue-69446-fnmut-capture.stderr @@ -0,0 +1,19 @@ +error: captured variable cannot escape `FnMut` closure body + --> $DIR/issue-69446-fnmut-capture.rs:19:17 + | +LL | let mut x = Foo; + | ----- variable defined here +LL | bar(move || async { + | _______________-_^ + | | | + | | inferred to be a `FnMut` closure +LL | | x.foo(); + | | - variable captured here +LL | | }); + | |_____^ returns an `async` block that contains a reference to a captured variable, which then escapes the closure body + | + = note: `FnMut` closures only have access to their captured variables while they are executing... + = note: ...therefore, they cannot allow references to captured variables to escape + +error: aborting due to previous error + diff --git a/src/test/ui/borrowck/borrowck-describe-lvalue.stderr b/src/test/ui/borrowck/borrowck-describe-lvalue.stderr index 075e0e2e4515e..4144d70cc1601 100644 --- a/src/test/ui/borrowck/borrowck-describe-lvalue.stderr +++ b/src/test/ui/borrowck/borrowck-describe-lvalue.stderr @@ -21,10 +21,13 @@ LL | *y = 1; error: captured variable cannot escape `FnMut` closure body --> $DIR/borrowck-describe-lvalue.rs:264:16 | +LL | let mut x = 0; + | ----- variable defined here LL | || { | - inferred to be a `FnMut` closure LL | / || { LL | | let y = &mut x; + | | - variable captured here LL | | &mut x; LL | | *y = 1; LL | | drop(y); diff --git a/src/test/ui/issues/issue-40510-1.stderr b/src/test/ui/issues/issue-40510-1.stderr index f4fda0abc2049..54df40b6e3d05 100644 --- a/src/test/ui/issues/issue-40510-1.stderr +++ b/src/test/ui/issues/issue-40510-1.stderr @@ -1,10 +1,16 @@ error: captured variable cannot escape `FnMut` closure body --> $DIR/issue-40510-1.rs:7:9 | +LL | let mut x: Box<()> = Box::new(()); + | ----- variable defined here +LL | LL | || { | - inferred to be a `FnMut` closure LL | &mut x - | ^^^^^^ returns a reference to a captured variable which escapes the closure body + | ^^^^^- + | | | + | | variable captured here + | returns a reference to a captured variable which escapes the closure body | = note: `FnMut` closures only have access to their captured variables while they are executing... = note: ...therefore, they cannot allow references to captured variables to escape diff --git a/src/test/ui/issues/issue-40510-3.stderr b/src/test/ui/issues/issue-40510-3.stderr index 4bc7d0f5deac5..cb885ec7d952a 100644 --- a/src/test/ui/issues/issue-40510-3.stderr +++ b/src/test/ui/issues/issue-40510-3.stderr @@ -1,10 +1,14 @@ error: captured variable cannot escape `FnMut` closure body --> $DIR/issue-40510-3.rs:7:9 | +LL | let mut x: Vec<()> = Vec::new(); + | ----- variable defined here +LL | LL | || { | - inferred to be a `FnMut` closure LL | / || { LL | | x.push(()) + | | - variable captured here LL | | } | |_________^ returns a closure that contains a reference to a captured variable, which then escapes the closure body | diff --git a/src/test/ui/issues/issue-49824.stderr b/src/test/ui/issues/issue-49824.stderr index 6b486aafcdf40..2fec482543d7b 100644 --- a/src/test/ui/issues/issue-49824.stderr +++ b/src/test/ui/issues/issue-49824.stderr @@ -1,11 +1,14 @@ error: captured variable cannot escape `FnMut` closure body --> $DIR/issue-49824.rs:4:9 | +LL | let mut x = 0; + | ----- variable defined here LL | || { | - inferred to be a `FnMut` closure LL | / || { LL | | LL | | let _y = &mut x; + | | - variable captured here LL | | } | |_________^ returns a closure that contains a reference to a captured variable, which then escapes the closure body | diff --git a/src/test/ui/nll/issue-53040.stderr b/src/test/ui/nll/issue-53040.stderr index 7cba32c67432c..87ffe9b1abf45 100644 --- a/src/test/ui/nll/issue-53040.stderr +++ b/src/test/ui/nll/issue-53040.stderr @@ -1,9 +1,13 @@ error: captured variable cannot escape `FnMut` closure body --> $DIR/issue-53040.rs:3:8 | +LL | let mut v: Vec<()> = Vec::new(); + | ----- variable defined here LL | || &mut v; - | - ^^^^^^ returns a reference to a captured variable which escapes the closure body - | | + | - ^^^^^- + | | | | + | | | variable captured here + | | returns a reference to a captured variable which escapes the closure body | inferred to be a `FnMut` closure | = note: `FnMut` closures only have access to their captured variables while they are executing... diff --git a/src/test/ui/regions/regions-return-ref-to-upvar-issue-17403.stderr b/src/test/ui/regions/regions-return-ref-to-upvar-issue-17403.stderr index 4c275b19492c6..b087e03b464b3 100644 --- a/src/test/ui/regions/regions-return-ref-to-upvar-issue-17403.stderr +++ b/src/test/ui/regions/regions-return-ref-to-upvar-issue-17403.stderr @@ -1,9 +1,13 @@ error: captured variable cannot escape `FnMut` closure body --> $DIR/regions-return-ref-to-upvar-issue-17403.rs:7:24 | +LL | let mut x = 0; + | ----- variable defined here LL | let mut f = || &mut x; - | - ^^^^^^ returns a reference to a captured variable which escapes the closure body - | | + | - ^^^^^- + | | | | + | | | variable captured here + | | returns a reference to a captured variable which escapes the closure body | inferred to be a `FnMut` closure | = note: `FnMut` closures only have access to their captured variables while they are executing... From 754da8849c18c45ff2fd2e77213f8371de488f80 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 11 Jun 2020 13:42:22 -0400 Subject: [PATCH 04/20] Make `fn_arg_names` return `Ident` instead of symbol Also, implement this query for the local crate, not just foreign crates. --- src/librustc_metadata/rmeta/decoder.rs | 4 ++-- src/librustc_metadata/rmeta/encoder.rs | 16 +++++----------- src/librustc_metadata/rmeta/mod.rs | 4 ++-- src/librustc_middle/hir/map/mod.rs | 9 ++++++++- src/librustc_middle/hir/mod.rs | 20 ++++++++++++++++---- src/librustc_middle/query/mod.rs | 2 +- 6 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/librustc_metadata/rmeta/decoder.rs b/src/librustc_metadata/rmeta/decoder.rs index f5a9dceb78295..b2afdd0b4e13f 100644 --- a/src/librustc_metadata/rmeta/decoder.rs +++ b/src/librustc_metadata/rmeta/decoder.rs @@ -1317,13 +1317,13 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { } } - fn get_fn_param_names(&self, tcx: TyCtxt<'tcx>, id: DefIndex) -> &'tcx [Symbol] { + fn get_fn_param_names(&self, tcx: TyCtxt<'tcx>, id: DefIndex) -> &'tcx [Ident] { let param_names = match self.kind(id) { EntryKind::Fn(data) | EntryKind::ForeignFn(data) => data.decode(self).param_names, EntryKind::AssocFn(data) => data.decode(self).fn_data.param_names, _ => Lazy::empty(), }; - tcx.arena.alloc_from_iter(param_names.decode(self)) + tcx.arena.alloc_from_iter(param_names.decode((self, tcx))) } fn exported_symbols( diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 64ccd46a744f5..60a5bb4340513 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -30,7 +30,7 @@ use rustc_middle::ty::{self, SymbolName, Ty, TyCtxt}; use rustc_serialize::{opaque, Encodable, Encoder, SpecializedEncoder}; use rustc_session::config::CrateType; use rustc_span::source_map::Spanned; -use rustc_span::symbol::{kw, sym, Ident, Symbol}; +use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_span::{self, ExternalSource, FileName, SourceFile, Span}; use rustc_target::abi::VariantIdx; use std::hash::Hash; @@ -997,18 +997,12 @@ impl EncodeContext<'tcx> { } } - fn encode_fn_param_names_for_body(&mut self, body_id: hir::BodyId) -> Lazy<[Symbol]> { - self.tcx.dep_graph.with_ignore(|| { - let body = self.tcx.hir().body(body_id); - self.lazy(body.params.iter().map(|arg| match arg.pat.kind { - hir::PatKind::Binding(_, _, ident, _) => ident.name, - _ => kw::Invalid, - })) - }) + fn encode_fn_param_names_for_body(&mut self, body_id: hir::BodyId) -> Lazy<[Ident]> { + self.tcx.dep_graph.with_ignore(|| self.lazy(self.tcx.hir().body_param_names(body_id))) } - fn encode_fn_param_names(&mut self, param_names: &[Ident]) -> Lazy<[Symbol]> { - self.lazy(param_names.iter().map(|ident| ident.name)) + fn encode_fn_param_names(&mut self, param_names: &[Ident]) -> Lazy<[Ident]> { + self.lazy(param_names.iter()) } fn encode_optimized_mir(&mut self, def_id: LocalDefId) { diff --git a/src/librustc_metadata/rmeta/mod.rs b/src/librustc_metadata/rmeta/mod.rs index 89d525eb80b8c..1cf86c7660ee4 100644 --- a/src/librustc_metadata/rmeta/mod.rs +++ b/src/librustc_metadata/rmeta/mod.rs @@ -19,7 +19,7 @@ use rustc_serialize::opaque::Encoder; use rustc_session::config::SymbolManglingVersion; use rustc_session::CrateDisambiguator; use rustc_span::edition::Edition; -use rustc_span::symbol::Symbol; +use rustc_span::symbol::{Ident, Symbol}; use rustc_span::{self, Span}; use rustc_target::spec::{PanicStrategy, TargetTriple}; @@ -327,7 +327,7 @@ struct ModData { struct FnData { asyncness: hir::IsAsync, constness: hir::Constness, - param_names: Lazy<[Symbol]>, + param_names: Lazy<[Ident]>, } #[derive(RustcEncodable, RustcDecodable)] diff --git a/src/librustc_middle/hir/map/mod.rs b/src/librustc_middle/hir/map/mod.rs index b1dafb3c88585..12c7b18d06988 100644 --- a/src/librustc_middle/hir/map/mod.rs +++ b/src/librustc_middle/hir/map/mod.rs @@ -14,7 +14,7 @@ use rustc_hir::*; use rustc_index::vec::IndexVec; use rustc_span::hygiene::MacroKind; use rustc_span::source_map::Spanned; -use rustc_span::symbol::{kw, Symbol}; +use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::Span; use rustc_target::spec::abi::Abi; @@ -375,6 +375,13 @@ impl<'hir> Map<'hir> { }) } + pub fn body_param_names(&self, id: BodyId) -> impl Iterator + 'hir { + self.body(id).params.iter().map(|arg| match arg.pat.kind { + PatKind::Binding(_, _, ident, _) => ident, + _ => Ident::new(kw::Invalid, rustc_span::DUMMY_SP), + }) + } + /// Returns the `BodyOwnerKind` of this `LocalDefId`. /// /// Panics if `LocalDefId` does not have an associated body. diff --git a/src/librustc_middle/hir/mod.rs b/src/librustc_middle/hir/mod.rs index 1e3676496ce39..e152d11c081a1 100644 --- a/src/librustc_middle/hir/mod.rs +++ b/src/librustc_middle/hir/mod.rs @@ -12,10 +12,7 @@ use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::def_id::{LocalDefId, LOCAL_CRATE}; -use rustc_hir::Body; -use rustc_hir::HirId; -use rustc_hir::ItemLocalId; -use rustc_hir::Node; +use rustc_hir::*; use rustc_index::vec::IndexVec; pub struct Owner<'tcx> { @@ -79,5 +76,20 @@ pub fn provide(providers: &mut Providers<'_>) { }; providers.hir_owner = |tcx, id| tcx.index_hir(LOCAL_CRATE).map[id].signature; providers.hir_owner_nodes = |tcx, id| tcx.index_hir(LOCAL_CRATE).map[id].with_bodies.as_deref(); + providers.fn_arg_names = |tcx, id| { + let hir = tcx.hir(); + let hir_id = hir.as_local_hir_id(id.expect_local()); + if let Some(body_id) = hir.maybe_body_owned_by(hir_id) { + tcx.arena.alloc_from_iter(hir.body_param_names(body_id)) + } else if let Node::TraitItem(&TraitItem { + kind: TraitItemKind::Fn(_, TraitFn::Required(idents)), + .. + }) = hir.get(hir_id) + { + tcx.arena.alloc_slice(idents) + } else { + span_bug!(hir.span(hir_id), "fn_arg_names: unexpected item {:?}", id); + } + }; map::provide(providers); } diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 4d7e7882e426c..d65e01b145ac6 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -700,7 +700,7 @@ rustc_queries! { } Other { - query fn_arg_names(def_id: DefId) -> &'tcx [Symbol] { + query fn_arg_names(def_id: DefId) -> &'tcx [rustc_span::symbol::Ident] { desc { |tcx| "looking up function parameter names for `{}`", tcx.def_path_str(def_id) } } /// Gets the rendered value of the specified constant or associated constant. From 2c11c35f8982a9a5fd654f0499cc72b70208d62f Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 11 Jun 2020 13:48:46 -0400 Subject: [PATCH 05/20] Explain move errors that occur due to method calls involving `self` --- src/librustc_ast_lowering/expr.rs | 24 ++- .../infer/error_reporting/need_type_info.rs | 2 +- src/librustc_middle/lint.rs | 4 +- .../diagnostics/conflict_errors.rs | 72 +++++++- .../diagnostics/explain_borrow.rs | 2 +- .../borrow_check/diagnostics/mod.rs | 152 ++++++++++++++--- .../borrow_check/diagnostics/move_errors.rs | 2 +- .../diagnostics/mutability_errors.rs | 2 +- src/librustc_mir/borrow_check/mod.rs | 6 + src/librustc_mir/transform/const_prop.rs | 1 + src/librustc_span/hygiene.rs | 13 +- src/librustc_span/lib.rs | 4 +- src/test/ui/binop/binop-consume-args.stderr | 70 ++++++-- src/test/ui/binop/binop-move-semantics.stderr | 21 ++- .../borrowck/borrowck-unboxed-closures.stderr | 7 +- .../ui/closure_context/issue-42065.stderr | 7 +- src/test/ui/codemap_tests/tab_3.stderr | 8 +- .../ui/consts/miri_unleashed/ptr_arith.rs | 5 +- .../ui/consts/miri_unleashed/ptr_arith.stderr | 4 +- src/test/ui/hygiene/unpretty-debug.stdout | 4 + src/test/ui/issues/issue-12127.stderr | 7 +- src/test/ui/issues/issue-33941.rs | 1 + src/test/ui/issues/issue-33941.stderr | 12 +- src/test/ui/issues/issue-34721.stderr | 9 +- src/test/ui/issues/issue-61108.stderr | 8 +- src/test/ui/issues/issue-64559.stderr | 8 +- src/test/ui/moves/move-fn-self-receiver.rs | 74 ++++++++ .../ui/moves/move-fn-self-receiver.stderr | 158 ++++++++++++++++++ ...moves-based-on-type-access-to-field.stderr | 8 +- .../ui/moves/moves-based-on-type-exprs.stderr | 16 +- .../ui/once-cant-call-twice-on-heap.stderr | 7 +- ...ed-closures-infer-fnonce-call-twice.stderr | 7 +- ...osures-infer-fnonce-move-call-twice.stderr | 7 +- src/test/ui/unop-move-semantics.stderr | 7 +- .../unsized-locals/borrow-after-move.stderr | 8 +- src/test/ui/unsized-locals/double-move.stderr | 8 +- .../use-after-move-self-based-on-type.stderr | 8 +- src/test/ui/use/use-after-move-self.stderr | 8 +- src/test/ui/walk-struct-literal-with.stderr | 8 +- 39 files changed, 697 insertions(+), 82 deletions(-) create mode 100644 src/test/ui/moves/move-fn-self-receiver.rs create mode 100644 src/test/ui/moves/move-fn-self-receiver.stderr diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index b7894eb145b0a..e59cacfffc926 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -9,7 +9,7 @@ use rustc_data_structures::thin_vec::ThinVec; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::Res; -use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned}; +use rustc_span::source_map::{respan, DesugaringKind, ForLoopLoc, Span, Spanned}; use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_target::asm; use std::collections::hash_map::Entry; @@ -25,6 +25,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } pub(super) fn lower_expr_mut(&mut self, e: &Expr) -> hir::Expr<'hir> { + let mut span = e.span; ensure_sufficient_stack(|| { let kind = match e.kind { ExprKind::Box(ref inner) => hir::ExprKind::Box(self.lower_expr(inner)), @@ -53,6 +54,7 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::MethodCall(hir_seg, seg.ident.span, args, span) } ExprKind::Binary(binop, ref lhs, ref rhs) => { + span = self.mark_span_with_reason(DesugaringKind::Operator, e.span, None); let binop = self.lower_binop(binop); let lhs = self.lower_expr(lhs); let rhs = self.lower_expr(rhs); @@ -222,7 +224,7 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::Expr { hir_id: self.lower_node_id(e.id), kind, - span: e.span, + span, attrs: e.attrs.iter().map(|a| self.lower_attr(a)).collect::>().into(), } }) @@ -237,6 +239,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } fn lower_binop(&mut self, b: BinOp) -> hir::BinOp { + let span = self.mark_span_with_reason(DesugaringKind::Operator, b.span, None); Spanned { node: match b.node { BinOpKind::Add => hir::BinOpKind::Add, @@ -258,7 +261,7 @@ impl<'hir> LoweringContext<'_, 'hir> { BinOpKind::Ge => hir::BinOpKind::Ge, BinOpKind::Gt => hir::BinOpKind::Gt, }, - span: b.span, + span, } } @@ -1360,9 +1363,14 @@ impl<'hir> LoweringContext<'_, 'hir> { body: &Block, opt_label: Option