Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 6 pull requests #113229

Merged
merged 16 commits into from
Jul 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 74 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::iter;

use either::Either;
use hir::PatField;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxIndexSet;
use rustc_errors::{
Expand Down Expand Up @@ -28,6 +27,7 @@ use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::{BytePos, Span, Symbol};
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::ObligationCtxt;
use std::iter;

use crate::borrow_set::TwoPhaseActivation;
use crate::borrowck_errors;
Expand Down Expand Up @@ -992,6 +992,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
issued_borrow.borrowed_place,
&issued_spans,
);
self.explain_iterator_advancement_in_for_loop_if_applicable(
&mut err,
span,
&issued_spans,
);
err
}

Expand Down Expand Up @@ -1279,6 +1284,73 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

/// Suggest using `while let` for call `next` on an iterator in a for loop.
///
/// For example:
/// ```ignore (illustrative)
///
/// for x in iter {
/// ...
/// iter.next()
/// }
/// ```
pub(crate) fn explain_iterator_advancement_in_for_loop_if_applicable(
&self,
err: &mut Diagnostic,
span: Span,
issued_spans: &UseSpans<'tcx>,
) {
let issue_span = issued_spans.args_or_use();
let tcx = self.infcx.tcx;
let hir = tcx.hir();

let Some(body_id) = hir.get(self.mir_hir_id()).body_id() else { return };
let typeck_results = tcx.typeck(self.mir_def_id());

struct ExprFinder<'hir> {
issue_span: Span,
expr_span: Span,
body_expr: Option<&'hir hir::Expr<'hir>>,
loop_bind: Option<Symbol>,
}
impl<'hir> Visitor<'hir> for ExprFinder<'hir> {
fn visit_expr(&mut self, ex: &'hir hir::Expr<'hir>) {
if let hir::ExprKind::Loop(hir::Block{ stmts: [stmt, ..], ..}, _, hir::LoopSource::ForLoop, _) = ex.kind &&
let hir::StmtKind::Expr(hir::Expr{ kind: hir::ExprKind::Match(call, [_, bind, ..], _), ..}) = stmt.kind &&
let hir::ExprKind::Call(path, _args) = call.kind &&
let hir::ExprKind::Path(hir::QPath::LangItem(LangItem::IteratorNext, _, _, )) = path.kind &&
let hir::PatKind::Struct(path, [field, ..], _) = bind.pat.kind &&
let hir::QPath::LangItem(LangItem::OptionSome, _, _) = path &&
let PatField { pat: hir::Pat{ kind: hir::PatKind::Binding(_, _, ident, ..), .. }, ..} = field &&
self.issue_span.source_equal(call.span) {
self.loop_bind = Some(ident.name);
}

if let hir::ExprKind::MethodCall(body_call, _recv, ..) = ex.kind &&
body_call.ident.name == sym::next && ex.span.source_equal(self.expr_span) {
self.body_expr = Some(ex);
}

hir::intravisit::walk_expr(self, ex);
}
}
let mut finder =
ExprFinder { expr_span: span, issue_span, loop_bind: None, body_expr: None };
finder.visit_expr(hir.body(body_id).value);

if let Some(loop_bind) = finder.loop_bind &&
let Some(body_expr) = finder.body_expr &&
let Some(def_id) = typeck_results.type_dependent_def_id(body_expr.hir_id) &&
let Some(trait_did) = tcx.trait_of_item(def_id) &&
tcx.is_diagnostic_item(sym::Iterator, trait_did) {
err.note(format!(
"a for loop advances the iterator for you, the result is stored in `{}`.",
loop_bind
));
err.help("if you want to call `next` on a iterator within the loop, consider using `while let`.");
}
}

/// Suggest using closure argument instead of capture.
///
/// For example:
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1146,6 +1146,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// Avoid pointing to the same function in multiple different
// error messages.
if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) {
self.explain_iterator_advancement_in_for_loop_if_applicable(
err,
span,
&move_spans,
);

let func = tcx.def_path_str(method_did);
err.subdiagnostic(CaptureReasonNote::FuncTakeSelf {
func,
Expand Down
148 changes: 116 additions & 32 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use rustc_middle::ty::{
self, InternalSubsts, Ty, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitableExt,
};
use rustc_middle::ty::{GenericParamDefKind, ToPredicate, TyCtxt};
use rustc_span::Span;
use rustc_span::{Span, DUMMY_SP};
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt;
use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _;
use rustc_trait_selection::traits::{
Expand Down Expand Up @@ -651,11 +651,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
let impl_sig = ocx.normalize(
&norm_cause,
param_env,
infcx.instantiate_binder_with_fresh_vars(
return_span,
infer::HigherRankedType,
tcx.fn_sig(impl_m.def_id).subst_identity(),
),
tcx.liberate_late_bound_regions(impl_m.def_id, tcx.fn_sig(impl_m.def_id).subst_identity()),
);
impl_sig.error_reported()?;
let impl_return_ty = impl_sig.output();
Expand All @@ -665,9 +661,10 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
// them with inference variables.
// We will use these inference variables to collect the hidden types of RPITITs.
let mut collector = ImplTraitInTraitCollector::new(&ocx, return_span, param_env, impl_m_def_id);
let unnormalized_trait_sig = tcx
.liberate_late_bound_regions(
impl_m.def_id,
let unnormalized_trait_sig = infcx
.instantiate_binder_with_fresh_vars(
return_span,
infer::HigherRankedType,
tcx.fn_sig(trait_m.def_id).subst(tcx, trait_to_placeholder_substs),
)
.fold_with(&mut collector);
Expand Down Expand Up @@ -760,15 +757,17 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(

let mut collected_tys = FxHashMap::default();
for (def_id, (ty, substs)) in collected_types {
match infcx.fully_resolve(ty) {
Ok(ty) => {
match infcx.fully_resolve((ty, substs)) {
Ok((ty, substs)) => {
// `ty` contains free regions that we created earlier while liberating the
// trait fn signature. However, projection normalization expects `ty` to
// contains `def_id`'s early-bound regions.
let id_substs = InternalSubsts::identity_for_item(tcx, def_id);
debug!(?id_substs, ?substs);
let map: FxHashMap<ty::GenericArg<'tcx>, ty::GenericArg<'tcx>> =
std::iter::zip(substs, id_substs).collect();
let map: FxHashMap<_, _> = std::iter::zip(substs, id_substs)
.skip(tcx.generics_of(trait_m.def_id).count())
.filter_map(|(a, b)| Some((a.as_region()?, b.as_region()?)))
.collect();
debug!(?map);

// NOTE(compiler-errors): RPITITs, like all other RPITs, have early-bound
Expand All @@ -793,25 +792,19 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
// same generics.
let num_trait_substs = trait_to_impl_substs.len();
let num_impl_substs = tcx.generics_of(impl_m.container_id(tcx)).params.len();
let ty = tcx.fold_regions(ty, |region, _| {
match region.kind() {
// Remap all free regions, which correspond to late-bound regions in the function.
ty::ReFree(_) => {}
// Remap early-bound regions as long as they don't come from the `impl` itself.
ty::ReEarlyBound(ebr) if tcx.parent(ebr.def_id) != impl_m.container_id(tcx) => {}
_ => return region,
}
let Some(ty::ReEarlyBound(e)) = map.get(&region.into()).map(|r| r.expect_region().kind())
else {
return ty::Region::new_error_with_message(tcx, return_span, "expected ReFree to map to ReEarlyBound")
};
ty::Region::new_early_bound(tcx, ty::EarlyBoundRegion {
def_id: e.def_id,
name: e.name,
index: (e.index as usize - num_trait_substs + num_impl_substs) as u32,
})
});
debug!(%ty);
let ty = match ty.try_fold_with(&mut RemapHiddenTyRegions {
tcx,
map,
num_trait_substs,
num_impl_substs,
def_id,
impl_def_id: impl_m.container_id(tcx),
ty,
return_span,
}) {
Ok(ty) => ty,
Err(guar) => tcx.ty_error(guar),
};
collected_tys.insert(def_id, ty::EarlyBinder::bind(ty));
}
Err(err) => {
Expand Down Expand Up @@ -895,6 +888,97 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ImplTraitInTraitCollector<'_, 'tcx> {
}
}

struct RemapHiddenTyRegions<'tcx> {
tcx: TyCtxt<'tcx>,
map: FxHashMap<ty::Region<'tcx>, ty::Region<'tcx>>,
num_trait_substs: usize,
num_impl_substs: usize,
def_id: DefId,
impl_def_id: DefId,
ty: Ty<'tcx>,
return_span: Span,
}

impl<'tcx> ty::FallibleTypeFolder<TyCtxt<'tcx>> for RemapHiddenTyRegions<'tcx> {
type Error = ErrorGuaranteed;

fn interner(&self) -> TyCtxt<'tcx> {
self.tcx
}

fn try_fold_ty(&mut self, t: Ty<'tcx>) -> Result<Ty<'tcx>, Self::Error> {
if let ty::Alias(ty::Opaque, ty::AliasTy { substs, def_id, .. }) = *t.kind() {
let mut mapped_substs = Vec::with_capacity(substs.len());
for (arg, v) in std::iter::zip(substs, self.tcx.variances_of(def_id)) {
mapped_substs.push(match (arg.unpack(), v) {
// Skip uncaptured opaque substs
(ty::GenericArgKind::Lifetime(_), ty::Bivariant) => arg,
_ => arg.try_fold_with(self)?,
});
}
Ok(self.tcx.mk_opaque(def_id, self.tcx.mk_substs(&mapped_substs)))
} else {
t.try_super_fold_with(self)
}
}

fn try_fold_region(
&mut self,
region: ty::Region<'tcx>,
) -> Result<ty::Region<'tcx>, Self::Error> {
match region.kind() {
// Remap all free regions, which correspond to late-bound regions in the function.
ty::ReFree(_) => {}
// Remap early-bound regions as long as they don't come from the `impl` itself,
// in which case we don't really need to renumber them.
ty::ReEarlyBound(ebr) if self.tcx.parent(ebr.def_id) != self.impl_def_id => {}
_ => return Ok(region),
}

let e = if let Some(region) = self.map.get(&region) {
if let ty::ReEarlyBound(e) = region.kind() { e } else { bug!() }
} else {
let guar = match region.kind() {
ty::ReEarlyBound(ty::EarlyBoundRegion { def_id, .. })
| ty::ReFree(ty::FreeRegion {
bound_region: ty::BoundRegionKind::BrNamed(def_id, _),
..
}) => {
let return_span = if let ty::Alias(ty::Opaque, opaque_ty) = self.ty.kind() {
self.tcx.def_span(opaque_ty.def_id)
} else {
self.return_span
};
self.tcx
.sess
.struct_span_err(
return_span,
"return type captures more lifetimes than trait definition",
)
.span_label(self.tcx.def_span(def_id), "this lifetime was captured")
.span_note(
self.tcx.def_span(self.def_id),
"hidden type must only reference lifetimes captured by this impl trait",
)
.note(format!("hidden type inferred to be `{}`", self.ty))
.emit()
}
_ => self.tcx.sess.delay_span_bug(DUMMY_SP, "should've been able to remap region"),
};
return Err(guar);
};

Ok(ty::Region::new_early_bound(
self.tcx,
ty::EarlyBoundRegion {
def_id: e.def_id,
name: e.name,
index: (e.index as usize - self.num_trait_substs + self.num_impl_substs) as u32,
},
))
}
}

fn report_trait_method_mismatch<'tcx>(
infcx: &InferCtxt<'tcx>,
mut cause: ObligationCause<'tcx>,
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2135,16 +2135,18 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> {
// lints shouldn't be emmited even if `from` effective visibility
// is larger than `Priv` nominal visibility and if `Priv` can leak
// in some scenarios due to type inference.
let impl_ev = Some(EffectiveVisibility::of_impl::<false>(
let impl_ev = EffectiveVisibility::of_impl::<false>(
item.owner_id.def_id,
tcx,
self.effective_visibilities,
));
);

// check that private components do not appear in the generics or predicates of inherent impls
// this check is intentionally NOT performed for impls of traits, per #90586
if impl_.of_trait.is_none() {
self.check(item.owner_id.def_id, impl_vis, impl_ev).generics().predicates();
self.check(item.owner_id.def_id, impl_vis, Some(impl_ev))
.generics()
.predicates();
}
for impl_item_ref in impl_.items {
let impl_item_vis = if impl_.of_trait.is_none() {
Expand All @@ -2159,8 +2161,9 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> {

let impl_item_ev = if impl_.of_trait.is_none() {
self.get(impl_item_ref.id.owner_id.def_id)
.map(|ev| ev.min(impl_ev, self.tcx))
} else {
impl_ev
Some(impl_ev)
};

self.check_assoc_item(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
/// expansion and import resolution (perhaps they can be merged in the future).
/// The function is used for resolving initial segments of macro paths (e.g., `foo` in
/// `foo::bar!();` or `foo!();`) and also for import paths on 2018 edition.
#[instrument(level = "debug", skip(self, scope_set))]
#[instrument(level = "debug", skip(self))]
pub(crate) fn early_resolve_ident_in_lexical_scope(
&mut self,
orig_ident: Ident,
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -894,8 +894,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
return None;
}
PathResult::NonModule(_) => {
if no_ambiguity {
PathResult::NonModule(partial_res) => {
if no_ambiguity && partial_res.full_res() != Some(Res::Err) {
// Check if there are no ambiguities and the result is not dummy.
assert!(import.imported_module.get().is_none());
}
// The error was already reported earlier.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ enum Scope<'a> {
/// with different restrictions when looking up the resolution.
/// This enum is currently used only for early resolution (imports and macros),
/// but not for late resolution yet.
#[derive(Clone, Copy)]
#[derive(Clone, Copy, Debug)]
enum ScopeSet<'a> {
/// All scopes with the given namespace.
All(Namespace),
Expand Down
Loading