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 7 pull requests #125393

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
6 changes: 3 additions & 3 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,9 @@ hir_analysis_pass_to_variadic_function = can't pass `{$ty}` to variadic function
.suggestion = cast the value to `{$cast_ty}`
.help = cast the value to `{$cast_ty}`

hir_analysis_pattern_type_non_const_range = "range patterns must have constant range start and end"
hir_analysis_pattern_type_wild_pat = "wildcard patterns are not permitted for pattern types"
.label = "this type is the same as the inner type without a pattern"
hir_analysis_pattern_type_non_const_range = range patterns must have constant range start and end
hir_analysis_pattern_type_wild_pat = wildcard patterns are not permitted for pattern types
.label = this type is the same as the inner type without a pattern
hir_analysis_placeholder_not_allowed_item_signatures = the placeholder `_` is not allowed within types on item signatures for {$kind}
.label = not allowed in type signatures

Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1382,7 +1382,7 @@ pub enum GenericsArgsErrExtend<'tcx> {
span: Span,
},
SelfTyParam(Span),
TyParam(DefId),
Param(DefId),
DefVariant,
None,
}
Expand Down Expand Up @@ -1498,11 +1498,11 @@ fn generics_args_err_extend<'a>(
GenericsArgsErrExtend::DefVariant => {
err.note("enum variants can't have type parameters");
}
GenericsArgsErrExtend::TyParam(def_id) => {
if let Some(span) = tcx.def_ident_span(def_id) {
let name = tcx.item_name(def_id);
err.span_note(span, format!("type parameter `{name}` defined here"));
}
GenericsArgsErrExtend::Param(def_id) => {
let span = tcx.def_ident_span(def_id).unwrap();
let kind = tcx.def_descr(def_id);
let name = tcx.item_name(def_id);
err.span_note(span, format!("{kind} `{name}` defined here"));
}
GenericsArgsErrExtend::SelfTyParam(span) => {
err.span_suggestion_verbose(
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1757,7 +1757,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
assert_eq!(opt_self_ty, None);
let _ = self.prohibit_generic_args(
path.segments.iter(),
GenericsArgsErrExtend::TyParam(def_id),
GenericsArgsErrExtend::Param(def_id),
);
self.lower_ty_param(hir_id)
}
Expand Down Expand Up @@ -2190,10 +2190,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {

hir::ExprKind::Path(hir::QPath::Resolved(
_,
&hir::Path {
res: Res::Def(DefKind::ConstParam, def_id), ..
path @ &hir::Path {
res: Res::Def(DefKind::ConstParam, def_id),
..
},
)) => {
let _ = self.prohibit_generic_args(
path.segments.iter(),
GenericsArgsErrExtend::Param(def_id),
);
let ty = tcx
.type_of(def_id)
.no_bound_vars()
Expand Down
33 changes: 27 additions & 6 deletions compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let Some(arg_ty) = self.node_ty_opt(args[idx].hir_id) else {
return false;
};
let possible_rcvr_ty = expr_finder.uses.iter().find_map(|binding| {
let possible_rcvr_ty = expr_finder.uses.iter().rev().find_map(|binding| {
let possible_rcvr_ty = self.node_ty_opt(binding.hir_id)?;
if possible_rcvr_ty.is_ty_var() {
return None;
}
// Fudge the receiver, so we can do new inference on it.
let possible_rcvr_ty = possible_rcvr_ty.fold_with(&mut fudger);
let method = self
Expand All @@ -386,6 +389,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
binding,
)
.ok()?;
// Make sure we select the same method that we started with...
if Some(method.def_id)
!= self.typeck_results.borrow().type_dependent_def_id(call_expr.hir_id)
{
return None;
}
// Unify the method signature with our incompatible arg, to
// do inference in the *opposite* direction and to find out
// what our ideal rcvr ty would look like.
Expand Down Expand Up @@ -456,6 +465,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) else {
continue;
};
// Make sure we select the same method that we started with...
if Some(method.def_id)
!= self.typeck_results.borrow().type_dependent_def_id(parent_expr.hir_id)
{
continue;
}

let ideal_rcvr_ty = rcvr_ty.fold_with(&mut fudger);
let ideal_method = self
Expand Down Expand Up @@ -505,13 +520,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// blame arg, if possible. Don't do this if we're coming from
// arg mismatch code, because we'll possibly suggest a mutually
// incompatible fix at the original mismatch site.
// HACK(compiler-errors): We don't actually consider the implications
// of our inference guesses in `emit_type_mismatch_suggestions`, so
// only suggest things when we know our type error is precisely due to
// a type mismatch, and not via some projection or something. See #116155.
if matches!(source, TypeMismatchSource::Ty(_))
&& let Some(ideal_method) = ideal_method
&& let ideal_arg_ty = self.resolve_vars_if_possible(ideal_method.sig.inputs()[idx + 1])
// HACK(compiler-errors): We don't actually consider the implications
// of our inference guesses in `emit_type_mismatch_suggestions`, so
// only suggest things when we know our type error is precisely due to
// a type mismatch, and not via some projection or something. See #116155.
&& Some(ideal_method.def_id)
== self
.typeck_results
.borrow()
.type_dependent_def_id(parent_expr.hir_id)
&& let ideal_arg_ty =
self.resolve_vars_if_possible(ideal_method.sig.inputs()[idx + 1])
&& !ideal_arg_ty.has_non_region_infer()
{
self.emit_type_mismatch_suggestions(
Expand Down
89 changes: 58 additions & 31 deletions compiler/rustc_hir_typeck/src/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,60 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fake_reads: Default::default(),
};

let _ = euv::ExprUseVisitor::new(
&FnCtxt::new(self, self.tcx.param_env(closure_def_id), closure_def_id),
&mut delegate,
)
.consume_body(body);

// There are several curious situations with coroutine-closures where
// analysis is too aggressive with borrows when the coroutine-closure is
// marked `move`. Specifically:
//
// 1. If the coroutine-closure was inferred to be `FnOnce` during signature
// inference, then it's still possible that we try to borrow upvars from
// the coroutine-closure because they are not used by the coroutine body
// in a way that forces a move. See the test:
// `async-await/async-closures/force-move-due-to-inferred-kind.rs`.
//
// 2. If the coroutine-closure is forced to be `FnOnce` due to the way it
// uses its upvars, but not *all* upvars would force the closure to `FnOnce`.
// See the test: `async-await/async-closures/force-move-due-to-actually-fnonce.rs`.
//
// This would lead to an impossible to satisfy situation, since `AsyncFnOnce`
// coroutine bodies can't borrow from their parent closure. To fix this,
// we force the inner coroutine to also be `move`. This only matters for
// coroutine-closures that are `move` since otherwise they themselves will
// be borrowing from the outer environment, so there's no self-borrows occuring.
//
// One *important* note is that we do a call to `process_collected_capture_information`
// to eagerly test whether the coroutine would end up `FnOnce`, but we do this
// *before* capturing all the closure args by-value below, since that would always
// cause the analysis to return `FnOnce`.
if let UpvarArgs::Coroutine(..) = args
&& let hir::CoroutineKind::Desugared(_, hir::CoroutineSource::Closure) =
self.tcx.coroutine_kind(closure_def_id).expect("coroutine should have kind")
&& let parent_hir_id =
self.tcx.local_def_id_to_hir_id(self.tcx.local_parent(closure_def_id))
&& let parent_ty = self.node_ty(parent_hir_id)
&& let hir::CaptureBy::Value { move_kw } =
self.tcx.hir_node(parent_hir_id).expect_closure().capture_clause
{
// (1.) Closure signature inference forced this closure to `FnOnce`.
if let Some(ty::ClosureKind::FnOnce) = self.closure_kind(parent_ty) {
capture_clause = hir::CaptureBy::Value { move_kw };
}
// (2.) The way that the closure uses its upvars means it's `FnOnce`.
else if let (_, ty::ClosureKind::FnOnce, _) = self
.process_collected_capture_information(
capture_clause,
&delegate.capture_information,
)
{
capture_clause = hir::CaptureBy::Value { move_kw };
}
}

// As noted in `lower_coroutine_body_with_moved_arguments`, we default the capture mode
// to `ByRef` for the `async {}` block internal to async fns/closure. This means
// that we would *not* be moving all of the parameters into the async block by default.
Expand Down Expand Up @@ -253,34 +307,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

let _ = euv::ExprUseVisitor::new(
&FnCtxt::new(self, self.tcx.param_env(closure_def_id), closure_def_id),
&mut delegate,
)
.consume_body(body);

// If a coroutine is comes from a coroutine-closure that is `move`, but
// the coroutine-closure was inferred to be `FnOnce` during signature
// inference, then it's still possible that we try to borrow upvars from
// the coroutine-closure because they are not used by the coroutine body
// in a way that forces a move.
//
// This would lead to an impossible to satisfy situation, since `AsyncFnOnce`
// coroutine bodies can't borrow from their parent closure. To fix this,
// we force the inner coroutine to also be `move`. This only matters for
// coroutine-closures that are `move` since otherwise they themselves will
// be borrowing from the outer environment, so there's no self-borrows occuring.
if let UpvarArgs::Coroutine(..) = args
&& let hir::CoroutineKind::Desugared(_, hir::CoroutineSource::Closure) =
self.tcx.coroutine_kind(closure_def_id).expect("coroutine should have kind")
&& let parent_hir_id =
self.tcx.local_def_id_to_hir_id(self.tcx.local_parent(closure_def_id))
&& let parent_ty = self.node_ty(parent_hir_id)
&& let Some(ty::ClosureKind::FnOnce) = self.closure_kind(parent_ty)
{
capture_clause = self.tcx.hir_node(parent_hir_id).expect_closure().capture_clause;
}

debug!(
"For closure={:?}, capture_information={:#?}",
closure_def_id, delegate.capture_information
Expand All @@ -289,7 +315,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.log_capture_analysis_first_pass(closure_def_id, &delegate.capture_information, span);

let (capture_information, closure_kind, origin) = self
.process_collected_capture_information(capture_clause, delegate.capture_information);
.process_collected_capture_information(capture_clause, &delegate.capture_information);

self.compute_min_captures(closure_def_id, capture_information, span);

Expand Down Expand Up @@ -545,13 +571,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
fn process_collected_capture_information(
&self,
capture_clause: hir::CaptureBy,
capture_information: InferredCaptureInformation<'tcx>,
capture_information: &InferredCaptureInformation<'tcx>,
) -> (InferredCaptureInformation<'tcx>, ty::ClosureKind, Option<(Span, Place<'tcx>)>) {
let mut closure_kind = ty::ClosureKind::LATTICE_BOTTOM;
let mut origin: Option<(Span, Place<'tcx>)> = None;

let processed = capture_information
.into_iter()
.iter()
.cloned()
.map(|(place, mut capture_info)| {
// Apply rules for safety before inferring closure kind
let (place, capture_kind) =
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_log/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ pub fn init_logger(cfg: LoggerConfig) -> Result<(), Error> {

let mut layer = tracing_tree::HierarchicalLayer::default()
.with_writer(io::stderr)
.with_indent_lines(true)
.with_ansi(color_logs)
.with_targets(true)
.with_verbose_exit(verbose_entry_exit)
Expand Down
39 changes: 39 additions & 0 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,45 @@ impl<'tcx> CoroutineClosureArgs<'tcx> {
pub fn coroutine_witness_ty(self) -> Ty<'tcx> {
self.split().coroutine_witness_ty
}

pub fn has_self_borrows(&self) -> bool {
match self.coroutine_captures_by_ref_ty().kind() {
ty::FnPtr(sig) => sig
.skip_binder()
.visit_with(&mut HasRegionsBoundAt { binder: ty::INNERMOST })
.is_break(),
ty::Error(_) => true,
_ => bug!(),
}
}
}
/// Unlike `has_escaping_bound_vars` or `outermost_exclusive_binder`, this will
/// detect only regions bound *at* the debruijn index.
struct HasRegionsBoundAt {
binder: ty::DebruijnIndex,
}
// FIXME: Could be optimized to not walk into components with no escaping bound vars.
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for HasRegionsBoundAt {
type Result = ControlFlow<()>;
fn visit_binder<T: TypeVisitable<TyCtxt<'tcx>>>(
&mut self,
t: &ty::Binder<'tcx, T>,
) -> Self::Result {
self.binder.shift_in(1);
t.super_visit_with(self)?;
self.binder.shift_out(1);
ControlFlow::Continue(())
}

fn visit_region(&mut self, r: ty::Region<'tcx>) -> Self::Result {
if let ty::ReBound(binder, _) = *r
&& self.binder == binder
{
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
}
}
}

#[derive(Copy, Clone, PartialEq, Eq, Debug, TypeFoldable, TypeVisitable)]
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_pattern_analysis/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ pub fn init_tracing() {
use tracing_subscriber::Layer;
let _ = tracing_tree::HierarchicalLayer::default()
.with_writer(std::io::stderr)
.with_indent_lines(true)
.with_ansi(true)
.with_targets(true)
.with_indent_amount(2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,11 @@ pub(in crate::solve) fn extract_tupled_inputs_and_output_from_callable<'tcx>(
return Err(NoSolution);
}

// If `Fn`/`FnMut`, we only implement this goal if we
// have no captures.
let no_borrows = match args.tupled_upvars_ty().kind() {
ty::Tuple(tys) => tys.is_empty(),
ty::Error(_) => false,
_ => bug!("tuple_fields called on non-tuple"),
};
if closure_kind != ty::ClosureKind::FnOnce && !no_borrows {
// A coroutine-closure implements `FnOnce` *always*, since it may
// always be called once. It additionally implements `Fn`/`FnMut`
// only if it has no upvars referencing the closure-env lifetime,
// and if the closure kind permits it.
if closure_kind != ty::ClosureKind::FnOnce && args.has_self_borrows() {
return Err(NoSolution);
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_trait_selection/src/solve/search_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ impl<'tcx> SearchGraph<TyCtxt<'tcx>> {
};

if let Some(result) = self.lookup_global_cache(tcx, input, available_depth, inspect) {
debug!("global cache hit");
return result;
}

Expand Down Expand Up @@ -360,7 +361,7 @@ impl<'tcx> SearchGraph<TyCtxt<'tcx>> {
for _ in 0..FIXPOINT_STEP_LIMIT {
match self.fixpoint_step_in_task(tcx, input, inspect, &mut prove_goal) {
StepResult::Done(final_entry, result) => return (final_entry, result),
StepResult::HasChanged => {}
StepResult::HasChanged => debug!("fixpoint changed provisional results"),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,20 +418,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// Ambiguity if upvars haven't been constrained yet
&& !args.tupled_upvars_ty().is_ty_var()
{
let no_borrows = match args.tupled_upvars_ty().kind() {
ty::Tuple(tys) => tys.is_empty(),
ty::Error(_) => false,
_ => bug!("tuple_fields called on non-tuple"),
};
// A coroutine-closure implements `FnOnce` *always*, since it may
// always be called once. It additionally implements `Fn`/`FnMut`
// only if it has no upvars (therefore no borrows from the closure
// that would need to be represented with a lifetime) and if the
// closure kind permits it.
// FIXME(async_closures): Actually, it could also implement `Fn`/`FnMut`
// if it takes all of its upvars by copy, and none by ref. This would
// require us to record a bit more information during upvar analysis.
if no_borrows && closure_kind.extends(kind) {
// only if it has no upvars referencing the closure-env lifetime,
// and if the closure kind permits it.
if closure_kind.extends(kind) && !args.has_self_borrows() {
candidates.vec.push(ClosureCandidate { is_const });
} else if kind == ty::ClosureKind::FnOnce {
candidates.vec.push(ClosureCandidate { is_const });
Expand Down
Loading
Loading