Skip to content

Commit

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

Successful merges:

 - #124227 (Make sure that the method resolution matches in `note_source_of_type_mismatch_constraint`)
 - #124896 (miri: rename intrinsic_fallback_checks_ub to intrinsic_fallback_is_spec)
 - #125015 (Pattern types: Prohibit generic args on const params)
 - #125043 (reference type safety invariant docs: clarification)
 - #125259 (An async closure may implement `FnMut`/`Fn` if it has no self-borrows)
 - #125306 (Force the inner coroutine of an async closure to `move` if the outer closure is `move` and `FnOnce`)
 - #125378 (remove tracing tree indent lines)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed May 22, 2024
2 parents b54dd08 + 4c886b9 commit 81857ba
Show file tree
Hide file tree
Showing 32 changed files with 343 additions and 150 deletions.
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

0 comments on commit 81857ba

Please sign in to comment.