Skip to content

Commit

Permalink
Auto merge of #91799 - matthiaskrgr:rollup-b38xx6i, r=matthiaskrgr
Browse files Browse the repository at this point in the history
Rollup of 6 pull requests

Successful merges:

 - #83174 (Suggest using a temporary variable to fix borrowck errors)
 - #89734 (Point at capture points for non-`'static` reference crossing a `yield` point)
 - #90270 (Make `Borrow` and `BorrowMut` impls `const`)
 - #90741 (Const `Option::cloned`)
 - #91548 (Add spin_loop hint for RISC-V architecture)
 - #91721 (Minor improvements to `future::join!`'s implementation)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Dec 11, 2021
2 parents b8dc6aa + ed81098 commit 928783d
Show file tree
Hide file tree
Showing 56 changed files with 949 additions and 201 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ fn try_extract_error_from_fulfill_cx<'tcx>(
error_region,
cause.clone(),
placeholder_region,
vec![],
),
),
(Some(error_region), _) => NiceRegionError::new(
Expand Down
89 changes: 87 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,18 @@ use rustc_span::symbol::sym;
use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP};
use rustc_trait_selection::infer::InferCtxtExt;

use crate::borrow_set::TwoPhaseActivation;
use crate::borrowck_errors;

use crate::diagnostics::find_all_local_uses;
use crate::{
borrow_set::BorrowData, diagnostics::Instance, prefixes::IsPrefixOf,
InitializationRequiringAction, MirBorrowckCtxt, PrefixSet, WriteKind,
};

use super::{
explain_borrow::BorrowExplanation, FnSelfUseKind, IncludingDowncast, RegionName,
RegionNameSource, UseSpans,
explain_borrow::{BorrowExplanation, LaterUseKind},
FnSelfUseKind, IncludingDowncast, RegionName, RegionNameSource, UseSpans,
};

#[derive(Debug)]
Expand Down Expand Up @@ -768,9 +770,92 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
Some((issued_span, span)),
);

self.suggest_using_local_if_applicable(
&mut err,
location,
(place, span),
gen_borrow_kind,
issued_borrow,
explanation,
);

err
}

#[instrument(level = "debug", skip(self, err))]
fn suggest_using_local_if_applicable(
&self,
err: &mut DiagnosticBuilder<'_>,
location: Location,
(place, span): (Place<'tcx>, Span),
gen_borrow_kind: BorrowKind,
issued_borrow: &BorrowData<'tcx>,
explanation: BorrowExplanation,
) {
let used_in_call =
matches!(explanation, BorrowExplanation::UsedLater(LaterUseKind::Call, _call_span, _));
if !used_in_call {
debug!("not later used in call");
return;
}

let outer_call_loc =
if let TwoPhaseActivation::ActivatedAt(loc) = issued_borrow.activation_location {
loc
} else {
issued_borrow.reserve_location
};
let outer_call_stmt = self.body.stmt_at(outer_call_loc);

let inner_param_location = location;
let Some(inner_param_stmt) = self.body.stmt_at(inner_param_location).left() else {
debug!("`inner_param_location` {:?} is not for a statement", inner_param_location);
return;
};
let Some(&inner_param) = inner_param_stmt.kind.as_assign().map(|(p, _)| p) else {
debug!(
"`inner_param_location` {:?} is not for an assignment: {:?}",
inner_param_location, inner_param_stmt
);
return;
};
let inner_param_uses = find_all_local_uses::find(self.body, inner_param.local);
let Some((inner_call_loc,inner_call_term)) = inner_param_uses.into_iter().find_map(|loc| {
let Either::Right(term) = self.body.stmt_at(loc) else {
debug!("{:?} is a statement, so it can't be a call", loc);
return None;
};
let TerminatorKind::Call { args, .. } = &term.kind else {
debug!("not a call: {:?}", term);
return None;
};
debug!("checking call args for uses of inner_param: {:?}", args);
if args.contains(&Operand::Move(inner_param)) {
Some((loc,term))
} else {
None
}
}) else {
debug!("no uses of inner_param found as a by-move call arg");
return;
};
debug!("===> outer_call_loc = {:?}, inner_call_loc = {:?}", outer_call_loc, inner_call_loc);

let inner_call_span = inner_call_term.source_info.span;
let outer_call_span = outer_call_stmt.either(|s| s.source_info, |t| t.source_info).span;
if outer_call_span == inner_call_span || !outer_call_span.contains(inner_call_span) {
// FIXME: This stops the suggestion in some cases where it should be emitted.
// Fix the spans for those cases so it's emitted correctly.
debug!(
"outer span {:?} does not strictly contain inner span {:?}",
outer_call_span, inner_call_span
);
return;
}
err.span_help(inner_call_span, "try adding a local storing this argument...");
err.span_help(outer_call_span, "...and then using that local as the argument to this call");
}

fn suggest_split_at_mut_if_applicable(
&self,
err: &mut DiagnosticBuilder<'_>,
Expand Down
26 changes: 26 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/find_all_local_uses.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use std::collections::BTreeSet;

use rustc_middle::mir::visit::{PlaceContext, Visitor};
use rustc_middle::mir::{Body, Local, Location};

/// Find all uses of (including assignments to) a [`Local`].
///
/// Uses `BTreeSet` so output is deterministic.
pub(super) fn find<'tcx>(body: &Body<'tcx>, local: Local) -> BTreeSet<Location> {
let mut visitor = AllLocalUsesVisitor { for_local: local, uses: BTreeSet::default() };
visitor.visit_body(body);
visitor.uses
}

struct AllLocalUsesVisitor {
for_local: Local,
uses: BTreeSet<Location>,
}

impl<'tcx> Visitor<'tcx> for AllLocalUsesVisitor {
fn visit_local(&mut self, local: &Local, _context: PlaceContext, location: Location) {
if *local == self.for_local {
self.uses.insert(location);
}
}
}
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rustc_target::abi::VariantIdx;
use super::borrow_set::BorrowData;
use super::MirBorrowckCtxt;

mod find_all_local_uses;
mod find_use;
mod outlives_suggestion;
mod region_name;
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
sub_r,
sup_origin,
sup_r,
_,
) => {
if sub_r.is_placeholder() {
self.report_placeholder_failure(sub_origin, sub_r, sup_r).emit();
Expand Down Expand Up @@ -464,7 +465,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
errors.sort_by_key(|u| match *u {
RegionResolutionError::ConcreteFailure(ref sro, _, _) => sro.span(),
RegionResolutionError::GenericBoundFailure(ref sro, _, _) => sro.span(),
RegionResolutionError::SubSupConflict(_, ref rvo, _, _, _, _) => rvo.span(),
RegionResolutionError::SubSupConflict(_, ref rvo, _, _, _, _, _) => rvo.span(),
RegionResolutionError::UpperBoundUniverseConflict(_, ref rvo, _, _, _) => rvo.span(),
});
errors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl<'cx, 'tcx> NiceRegionError<'cx, 'tcx> {
pub fn regions(&self) -> Option<(Span, ty::Region<'tcx>, ty::Region<'tcx>)> {
match (&self.error, self.regions) {
(Some(ConcreteFailure(origin, sub, sup)), None) => Some((origin.span(), sub, sup)),
(Some(SubSupConflict(_, _, origin, sub, _, sup)), None) => {
(Some(SubSupConflict(_, _, origin, sub, _, sup, _)), None) => {
Some((origin.span(), sub, sup))
}
(None, Some((span, sub, sup))) => Some((span, sub, sup)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ impl NiceRegionError<'me, 'tcx> {
sub_placeholder @ ty::RePlaceholder(_),
_,
sup_placeholder @ ty::RePlaceholder(_),
_,
)) => self.try_report_trait_placeholder_mismatch(
Some(self.tcx().mk_region(ty::ReVar(*vid))),
cause,
Expand All @@ -49,6 +50,7 @@ impl NiceRegionError<'me, 'tcx> {
sub_placeholder @ ty::RePlaceholder(_),
_,
_,
_,
)) => self.try_report_trait_placeholder_mismatch(
Some(self.tcx().mk_region(ty::ReVar(*vid))),
cause,
Expand All @@ -64,6 +66,7 @@ impl NiceRegionError<'me, 'tcx> {
_,
_,
sup_placeholder @ ty::RePlaceholder(_),
_,
)) => self.try_report_trait_placeholder_mismatch(
Some(self.tcx().mk_region(ty::ReVar(*vid))),
cause,
Expand All @@ -79,6 +82,7 @@ impl NiceRegionError<'me, 'tcx> {
_,
SubregionOrigin::Subtype(box TypeTrace { cause, values }),
sup_placeholder @ ty::RePlaceholder(_),
_,
)) => self.try_report_trait_placeholder_mismatch(
Some(self.tcx().mk_region(ty::ReVar(*vid))),
cause,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{walk_ty, ErasedMap, NestedVisitorMap, Visitor};
use rustc_hir::{self as hir, GenericBound, Item, ItemKind, Lifetime, LifetimeName, Node, TyKind};
use rustc_middle::ty::{
self, AssocItemContainer, RegionKind, Ty, TyCtxt, TypeFoldable, TypeVisitor,
self, AssocItemContainer, RegionKind, StaticLifetimeVisitor, Ty, TyCtxt, TypeFoldable,
TypeVisitor,
};
use rustc_span::symbol::Ident;
use rustc_span::{MultiSpan, Span};
Expand All @@ -23,16 +24,17 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
pub(super) fn try_report_static_impl_trait(&self) -> Option<ErrorReported> {
debug!("try_report_static_impl_trait(error={:?})", self.error);
let tcx = self.tcx();
let (var_origin, sub_origin, sub_r, sup_origin, sup_r) = match self.error.as_ref()? {
let (var_origin, sub_origin, sub_r, sup_origin, sup_r, spans) = match self.error.as_ref()? {
RegionResolutionError::SubSupConflict(
_,
var_origin,
sub_origin,
sub_r,
sup_origin,
sup_r,
spans,
) if **sub_r == RegionKind::ReStatic => {
(var_origin, sub_origin, sub_r, sup_origin, sup_r)
(var_origin, sub_origin, sub_r, sup_origin, sup_r, spans)
}
RegionResolutionError::ConcreteFailure(
SubregionOrigin::Subtype(box TypeTrace { cause, .. }),
Expand Down Expand Up @@ -74,7 +76,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
err.span_label(
cause.span,
&format!(
"...is captured and required to live as long as `'static` here \
"...is used and required to live as long as `'static` here \
because of an implicit lifetime bound on the {}",
match ctxt.assoc_item.container {
AssocItemContainer::TraitContainer(id) =>
Expand Down Expand Up @@ -123,56 +125,101 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
param_name,
lifetime,
);
err.span_label(param.param_ty_span, &format!("this data with {}...", lifetime));
debug!("try_report_static_impl_trait: param_info={:?}", param);

// We try to make the output have fewer overlapping spans if possible.
if (sp == sup_origin.span() || !return_sp.overlaps(sup_origin.span()))
&& sup_origin.span() != return_sp
{
// FIXME: account for `async fn` like in `async-await/issues/issue-62097.rs`

// Customize the spans and labels depending on their relative order so
// that split sentences flow correctly.
if sup_origin.span().overlaps(return_sp) && sp == sup_origin.span() {
// Avoid the following:
//
// error: cannot infer an appropriate lifetime
// --> $DIR/must_outlive_least_region_or_bound.rs:18:50
// |
// LL | fn foo(x: &i32) -> Box<dyn Debug> { Box::new(x) }
// | ---- ---------^-
let (mention_influencer, influencer_point) =
if sup_origin.span().overlaps(param.param_ty_span) {
// Account for `async fn` like in `async-await/issues/issue-62097.rs`.
// The desugaring of `async `fn`s causes `sup_origin` and `param` to point at the same
// place (but with different `ctxt`, hence `overlaps` instead of `==` above).
//
// and instead show:
// This avoids the following:
//
// error: cannot infer an appropriate lifetime
// --> $DIR/must_outlive_least_region_or_bound.rs:18:50
// |
// LL | fn foo(x: &i32) -> Box<dyn Debug> { Box::new(x) }
// | ---- ^
err.span_label(
sup_origin.span(),
"...is captured here, requiring it to live as long as `'static`",
);
// LL | pub async fn run_dummy_fn(&self) {
// | ^^^^^
// | |
// | this data with an anonymous lifetime `'_`...
// | ...is captured here...
(false, sup_origin.span())
} else {
err.span_label(sup_origin.span(), "...is captured here...");
if return_sp < sup_origin.span() {
err.span_note(
return_sp,
"...and is required to live as long as `'static` here",
(!sup_origin.span().overlaps(return_sp), param.param_ty_span)
};
err.span_label(influencer_point, &format!("this data with {}...", lifetime));

debug!("try_report_static_impl_trait: param_info={:?}", param);

let mut spans = spans.clone();

if mention_influencer {
spans.push(sup_origin.span());
}
// We dedup the spans *ignoring* expansion context.
spans.sort();
spans.dedup_by_key(|span| (span.lo(), span.hi()));

// We try to make the output have fewer overlapping spans if possible.
let require_msg = if spans.is_empty() {
"...is used and required to live as long as `'static` here"
} else {
"...and is required to live as long as `'static` here"
};
let require_span =
if sup_origin.span().overlaps(return_sp) { sup_origin.span() } else { return_sp };

for span in &spans {
err.span_label(*span, "...is used here...");
}

if spans.iter().any(|sp| sp.overlaps(return_sp) || *sp > return_sp) {
// If any of the "captured here" labels appears on the same line or after
// `require_span`, we put it on a note to ensure the text flows by appearing
// always at the end.
err.span_note(require_span, require_msg);
} else {
// We don't need a note, it's already at the end, it can be shown as a `span_label`.
err.span_label(require_span, require_msg);
}

if let SubregionOrigin::RelateParamBound(_, _, Some(bound)) = sub_origin {
err.span_note(*bound, "`'static` lifetime requirement introduced by this bound");
}
if let SubregionOrigin::Subtype(box TypeTrace { cause, .. }) = sub_origin {
if let ObligationCauseCode::ReturnValue(hir_id)
| ObligationCauseCode::BlockTailExpression(hir_id) = &cause.code
{
let parent_id = tcx.hir().get_parent_item(*hir_id);
if let Some(fn_decl) = tcx.hir().fn_decl_by_hir_id(parent_id) {
let mut span: MultiSpan = fn_decl.output.span().into();
let mut add_label = true;
if let hir::FnRetTy::Return(ty) = fn_decl.output {
let mut v = StaticLifetimeVisitor(vec![], tcx.hir());
v.visit_ty(ty);
if !v.0.is_empty() {
span = v.0.clone().into();
for sp in v.0 {
span.push_span_label(
sp,
"`'static` requirement introduced here".to_string(),
);
}
add_label = false;
}
}
if add_label {
span.push_span_label(
fn_decl.output.span(),
"requirement introduced by this return type".to_string(),
);
}
span.push_span_label(
cause.span,
"because of this returned expression".to_string(),
);
} else {
err.span_label(
return_sp,
"...and is required to live as long as `'static` here",
err.span_note(
span,
"`'static` lifetime requirement introduced by the return type",
);
}
}
} else {
err.span_label(
return_sp,
"...is captured and required to live as long as `'static` here",
);
}

let fn_returns = tcx.return_type_impl_or_dyn_traits(anon_reg_sup.def_id);
Expand Down
Loading

0 comments on commit 928783d

Please sign in to comment.