diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs index 4912d5f4582e7..8dff441bb3335 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -243,6 +243,15 @@ pub fn suggest_restriction<'tcx, G: EmissionGuarantee>( } } +/// A single layer of `&` peeled from an expression, used by +/// [`TypeErrCtxt::peel_expr_refs`]. +struct PeeledRef<'tcx> { + /// The span covering the `&` (and any whitespace/mutability keyword) to remove. + span: Span, + /// The type after peeling this layer (and all prior layers). + peeled_ty: Ty<'tcx>, +} + impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { pub fn note_field_shadowed_by_private_candidate_in_cause( &self, @@ -1818,6 +1827,85 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { ); } + /// Peel `&`-borrows from an expression, following through untyped let-bindings. + /// Returns a list of removable `&` layers (each with the span to remove and the + /// resulting type), plus an optional terminal [`hir::Param`] when the chain ends + /// at a function parameter (including async-fn desugared parameters). + fn peel_expr_refs( + &self, + mut expr: &'tcx hir::Expr<'tcx>, + mut ty: Ty<'tcx>, + ) -> (Vec>, Option<&'tcx hir::Param<'tcx>>) { + let mut refs = Vec::new(); + 'outer: loop { + while let hir::ExprKind::AddrOf(_, _, borrowed) = expr.kind { + let span = + if let Some(borrowed_span) = borrowed.span.find_ancestor_inside(expr.span) { + expr.span.until(borrowed_span) + } else { + break 'outer; + }; + + // Double check that the span actually corresponds to a borrow, + // rather than some macro garbage. + // The span may include leading parens from parenthesized expressions + // (e.g., `(&expr)` where HIR removes the Paren but keeps the span). + // In that case, trim the span to start at the `&`. + let span = match self.tcx.sess.source_map().span_to_snippet(span) { + Ok(ref snippet) if snippet.starts_with("&") => span, + Ok(ref snippet) if let Some(amp) = snippet.find('&') => { + span.with_lo(span.lo() + BytePos(amp as u32)) + } + _ => break 'outer, + }; + + let ty::Ref(_, inner_ty, _) = ty.kind() else { + break 'outer; + }; + ty = *inner_ty; + refs.push(PeeledRef { span, peeled_ty: ty }); + expr = borrowed; + } + if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind + && let Res::Local(hir_id) = path.res + && let hir::Node::Pat(binding) = self.tcx.hir_node(hir_id) + { + match self.tcx.parent_hir_node(binding.hir_id) { + // Untyped let-binding: follow to its initializer. + hir::Node::LetStmt(local) + if local.ty.is_none() + && let Some(init) = local.init => + { + expr = init; + continue; + } + // Async fn desugared parameter: `let x = __arg0;` with AsyncFn source. + // Follow to the original parameter. + hir::Node::LetStmt(local) + if matches!(local.source, hir::LocalSource::AsyncFn) + && let Some(init) = local.init + && let hir::ExprKind::Path(hir::QPath::Resolved(None, arg_path)) = + init.kind + && let Res::Local(arg_hir_id) = arg_path.res + && let hir::Node::Pat(arg_binding) = self.tcx.hir_node(arg_hir_id) + && let hir::Node::Param(param) = + self.tcx.parent_hir_node(arg_binding.hir_id) => + { + return (refs, Some(param)); + } + // Direct parameter reference. + hir::Node::Param(param) => { + return (refs, Some(param)); + } + _ => break 'outer, + } + } else { + break 'outer; + } + } + (refs, None) + } + /// Whenever references are used by mistake, like `for (i, e) in &vec.iter().enumerate()`, /// suggest removing these references until we reach a type that implements the trait. pub(super) fn suggest_remove_reference( @@ -1894,53 +1982,40 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } // Maybe suggest removal of borrows from expressions, like in `for i in &&&foo {}`. - let Some(mut expr) = expr_finder.result else { + let Some(expr) = expr_finder.result else { return false; }; - let mut count = 0; - let mut suggestions = vec![]; // Skipping binder here, remapping below - let mut suggested_ty = trait_pred.self_ty().skip_binder(); - 'outer: loop { - while let hir::ExprKind::AddrOf(_, _, borrowed) = expr.kind { - count += 1; - let span = - if let Some(borrowed_span) = borrowed.span.find_ancestor_inside(expr.span) { - expr.span.until(borrowed_span) - } else { - break 'outer; - }; - - // Double check that the span we extracted actually corresponds to a borrow, - // rather than some macro garbage. - match self.tcx.sess.source_map().span_to_snippet(span) { - Ok(snippet) if snippet.starts_with("&") => {} - _ => break 'outer, - } - - suggestions.push((span, String::new())); - - let ty::Ref(_, inner_ty, _) = suggested_ty.kind() else { - break 'outer; - }; - suggested_ty = *inner_ty; - - expr = borrowed; + let suggested_ty = trait_pred.self_ty().skip_binder(); + let (peeled_refs, _) = self.peel_expr_refs(expr, suggested_ty); + for (i, peeled) in peeled_refs.iter().enumerate() { + let suggestions: Vec<_> = + peeled_refs[..=i].iter().map(|r| (r.span, String::new())).collect(); + if maybe_suggest(peeled.peeled_ty, i + 1, suggestions) { + return true; + } + } + false + } - if maybe_suggest(suggested_ty, count, suggestions.clone()) { + /// Suggest removing `&` from a function parameter type like `&impl Future`. + fn suggest_remove_ref_from_param(&self, param: &hir::Param<'_>, err: &mut Diag<'_>) -> bool { + if let Some(decl) = self.tcx.parent_hir_node(param.hir_id).fn_decl() + && let Some(input_ty) = decl.inputs.iter().find(|t| param.ty_span.contains(t.span)) + && let hir::TyKind::Ref(_, mut_ty) = input_ty.kind + { + let ref_span = input_ty.span.until(mut_ty.ty.span); + match self.tcx.sess.source_map().span_to_snippet(ref_span) { + Ok(snippet) if snippet.starts_with("&") => { + err.span_suggestion_verbose( + ref_span, + "consider removing the `&` from the parameter type", + "", + Applicability::MaybeIncorrect, + ); return true; } - } - if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind - && let Res::Local(hir_id) = path.res - && let hir::Node::Pat(binding) = self.tcx.hir_node(hir_id) - && let hir::Node::LetStmt(local) = self.tcx.parent_hir_node(binding.hir_id) - && let None = local.ty - && let Some(binding_expr) = local.init - { - expr = binding_expr; - } else { - break 'outer; + _ => {} } } false @@ -1959,6 +2034,89 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { // could also check if it is an fn call (very likely) and suggest changing *that*, if // it is from the local crate. + // If the type is `&..&T` where `T: Future`, suggest removing `&` + // instead of removing `.await`. + if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) = + obligation.predicate.kind().skip_binder() + { + let self_ty = pred.self_ty(); + let future_trait = + self.tcx.require_lang_item(LangItem::Future, obligation.cause.span); + + // Peel through references to check if there's a Future underneath. + let has_future = { + let mut ty = self_ty; + loop { + match *ty.kind() { + ty::Ref(_, inner_ty, _) + if !matches!(inner_ty.kind(), ty::Dynamic(..)) => + { + if self + .type_implements_trait( + future_trait, + [inner_ty], + obligation.param_env, + ) + .must_apply_modulo_regions() + { + break true; + } + ty = inner_ty; + } + _ => break false, + } + } + }; + + if has_future { + let (peeled_refs, terminal_param) = self.peel_expr_refs(expr, self_ty); + + // Try removing `&`s from the expression. + for (i, peeled) in peeled_refs.iter().enumerate() { + if self + .type_implements_trait( + future_trait, + [peeled.peeled_ty], + obligation.param_env, + ) + .must_apply_modulo_regions() + { + let count = i + 1; + let msg = if count == 1 { + "consider removing the leading `&`-reference".to_string() + } else { + format!("consider removing {count} leading `&`-references") + }; + let suggestions: Vec<_> = + peeled_refs[..=i].iter().map(|r| (r.span, String::new())).collect(); + err.multipart_suggestion( + msg, + suggestions, + Applicability::MachineApplicable, + ); + return; + } + } + + // Try removing `&` from the parameter type, but only when there's + // no `&` in the expression itself (otherwise removing from the param + // alone wouldn't fix the error). + if peeled_refs.is_empty() + && let Some(param) = terminal_param + && self.suggest_remove_ref_from_param(param, err) + { + return; + } + + // Fallback: emit a help message when we can't provide a specific span. + err.help( + "a reference to a future is not a future; \ + consider removing the leading `&`-reference", + ); + return; + } + } + // use nth(1) to skip one layer of desugaring from `IntoIter::into_iter` if let Some((_, hir::Node::Expr(await_expr))) = self.tcx.hir_parent_iter(*hir_id).nth(1) && let Some(expr_span) = expr.span.find_ancestor_inside_same_ctxt(await_expr.span) diff --git a/tests/ui/async-await/await-ref-future.rs b/tests/ui/async-await/await-ref-future.rs new file mode 100644 index 0000000000000..19995e267feb3 --- /dev/null +++ b/tests/ui/async-await/await-ref-future.rs @@ -0,0 +1,43 @@ +//@ edition:2021 +// Regression test for #87211. +// Test that we suggest removing `&` from references to futures, +// including let-bindings and parameter types, but not `&dyn Future`. + +async fn my_async_fn() {} + +async fn foo() { + let fut = &my_async_fn(); + fut.await; //~ ERROR `&impl Future` is not a future +} + +async fn direct_ref_await() { + (&my_async_fn()).await; //~ ERROR `&impl Future` is not a future +} + +async fn bar(fut: &impl std::future::Future) { + fut.await; //~ ERROR is not a future +} + +async fn dyn_ref_param(fut: &dyn std::future::Future) { + fut.await; //~ ERROR is not a future +} + +async fn typed_let_binding() { + let fut: &_ = &my_async_fn(); + fut.await; //~ ERROR `&impl Future` is not a future +} + +async fn ref_param_borrowed_expr(fut: &impl std::future::Future) { + (&fut).await; //~ ERROR is not a future +} + +async fn double_ref_direct() { + (&&my_async_fn()).await; //~ ERROR is not a future +} + +async fn double_ref_let() { + let fut = &&my_async_fn(); + fut.await; //~ ERROR is not a future +} + +fn main() {} diff --git a/tests/ui/async-await/await-ref-future.stderr b/tests/ui/async-await/await-ref-future.stderr new file mode 100644 index 0000000000000..f801be1d60c26 --- /dev/null +++ b/tests/ui/async-await/await-ref-future.stderr @@ -0,0 +1,119 @@ +error[E0277]: `&impl Future` is not a future + --> $DIR/await-ref-future.rs:10:9 + | +LL | fut.await; + | ^^^^^ `&impl Future` is not a future + | + = help: the trait `Future` is not implemented for `&impl Future` + = note: &impl Future must be a future or must implement `IntoFuture` to be awaited + = note: required for `&impl Future` to implement `IntoFuture` +help: consider removing the leading `&`-reference + | +LL - let fut = &my_async_fn(); +LL + let fut = my_async_fn(); + | + +error[E0277]: `&impl Future` is not a future + --> $DIR/await-ref-future.rs:14:22 + | +LL | (&my_async_fn()).await; + | ^^^^^ `&impl Future` is not a future + | + = help: the trait `Future` is not implemented for `&impl Future` + = note: &impl Future must be a future or must implement `IntoFuture` to be awaited + = note: required for `&impl Future` to implement `IntoFuture` +help: consider removing the leading `&`-reference + | +LL - (&my_async_fn()).await; +LL + (my_async_fn()).await; + | + +error[E0277]: `&impl std::future::Future` is not a future + --> $DIR/await-ref-future.rs:18:9 + | +LL | fut.await; + | ^^^^^ `&impl std::future::Future` is not a future + | + = help: the trait `Future` is not implemented for `&impl std::future::Future` + = note: &impl std::future::Future must be a future or must implement `IntoFuture` to be awaited +help: the trait `Future` is implemented for `&mut F` + --> $SRC_DIR/core/src/future/future.rs:LL:COL + = note: required for `&impl std::future::Future` to implement `IntoFuture` +help: consider removing the `&` from the parameter type + | +LL - async fn bar(fut: &impl std::future::Future) { +LL + async fn bar(fut: impl std::future::Future) { + | + +error[E0277]: `&dyn Future` is not a future + --> $DIR/await-ref-future.rs:22:9 + | +LL | fut.await; + | ^^^^^ `&dyn Future` is not a future + | + = help: the trait `Future` is not implemented for `&dyn Future` + = note: &dyn Future must be a future or must implement `IntoFuture` to be awaited + = note: required for `&dyn Future` to implement `IntoFuture` +help: remove the `.await` + | +LL - fut.await; +LL + fut; + | + +error[E0277]: `&impl Future` is not a future + --> $DIR/await-ref-future.rs:27:9 + | +LL | fut.await; + | ^^^^^ `&impl Future` is not a future + | + = help: the trait `Future` is not implemented for `&impl Future` + = note: &impl Future must be a future or must implement `IntoFuture` to be awaited + = help: a reference to a future is not a future; consider removing the leading `&`-reference + = note: required for `&impl Future` to implement `IntoFuture` + +error[E0277]: `&&impl std::future::Future` is not a future + --> $DIR/await-ref-future.rs:31:12 + | +LL | (&fut).await; + | ^^^^^ `&&impl std::future::Future` is not a future + | + = help: the trait `Future` is not implemented for `&&impl std::future::Future` + = note: &&impl std::future::Future must be a future or must implement `IntoFuture` to be awaited + = help: a reference to a future is not a future; consider removing the leading `&`-reference +help: the trait `Future` is implemented for `&mut F` + --> $SRC_DIR/core/src/future/future.rs:LL:COL + = note: required for `&&impl std::future::Future` to implement `IntoFuture` + +error[E0277]: `&&impl Future` is not a future + --> $DIR/await-ref-future.rs:35:23 + | +LL | (&&my_async_fn()).await; + | ^^^^^ `&&impl Future` is not a future + | + = help: the trait `Future` is not implemented for `&&impl Future` + = note: &&impl Future must be a future or must implement `IntoFuture` to be awaited + = note: required for `&&impl Future` to implement `IntoFuture` +help: consider removing 2 leading `&`-references + | +LL - (&&my_async_fn()).await; +LL + (my_async_fn()).await; + | + +error[E0277]: `&&impl Future` is not a future + --> $DIR/await-ref-future.rs:40:9 + | +LL | fut.await; + | ^^^^^ `&&impl Future` is not a future + | + = help: the trait `Future` is not implemented for `&&impl Future` + = note: &&impl Future must be a future or must implement `IntoFuture` to be awaited + = note: required for `&&impl Future` to implement `IntoFuture` +help: consider removing 2 leading `&`-references + | +LL - let fut = &&my_async_fn(); +LL + let fut = my_async_fn(); + | + +error: aborting due to 8 previous errors + +For more information about this error, try `rustc --explain E0277`.