Skip to content

Commit

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

Successful merges:

 - #113168 (fix(resolve): skip assertion judgment when NonModule is dummy)
 - #113174 (Better messages for next on a iterator inside for loops)
 - #113182 (Error when RPITITs' hidden types capture more lifetimes than their trait definitions)
 - #113196 (Fix associated items effective visibility calculation for type privacy lints)
 - #113226 (Fix try builds on the msvc builder)
 - #113227 (Update cargo)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jul 1, 2023
2 parents 7905eff + f67d59b commit 5633798
Show file tree
Hide file tree
Showing 19 changed files with 500 additions and 107 deletions.
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

0 comments on commit 5633798

Please sign in to comment.