Skip to content

Commit

Permalink
Fix suggestions when returning a bare trait from an async fn.
Browse files Browse the repository at this point in the history
Don't assume we always return a trait object and suggest the dyn keyword.
Suggest adding impl instead.
  • Loading branch information
hirschenberger committed Oct 18, 2024
1 parent dd51276 commit d63cc44
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 45 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3365,6 +3365,7 @@ dependencies = [
"rustc_fluent_macro",
"rustc_graphviz",
"rustc_hir",
"rustc_hir_analysis",
"rustc_index",
"rustc_infer",
"rustc_lexer",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ rustc_errors = { path = "../rustc_errors" }
rustc_fluent_macro = { path = "../rustc_fluent_macro" }
rustc_graphviz = { path = "../rustc_graphviz" }
rustc_hir = { path = "../rustc_hir" }
rustc_hir_analysis = { path = "../rustc_hir_analysis" }
rustc_index = { path = "../rustc_index" }
rustc_infer = { path = "../rustc_infer" }
rustc_lexer = { path = "../rustc_lexer" }
Expand Down
44 changes: 4 additions & 40 deletions compiler/rustc_borrowck/src/diagnostics/region_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ use rustc_data_structures::fx::IndexEntry;
use rustc_errors::Diag;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
use rustc_middle::bug;
use rustc_middle::ty::print::RegionHighlightMode;
use rustc_middle::ty::{self, GenericArgKind, GenericArgsRef, RegionVid, Ty};
use rustc_middle::{bug, span_bug};
use rustc_span::symbol::{Symbol, kw, sym};
use rustc_span::symbol::{Symbol, kw};
use rustc_span::{DUMMY_SP, Span};
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
use tracing::{debug, instrument};
Expand Down Expand Up @@ -722,7 +723,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> {
.output;
span = output.span();
if let hir::FnRetTy::Return(ret) = output {
hir_ty = Some(self.get_future_inner_return_ty(ret));
hir_ty = <dyn HirTyLowerer<'_>>::get_future_inner_return_ty(ret);
}
" of async function"
}
Expand Down Expand Up @@ -816,43 +817,6 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, 'tcx> {
})
}

/// From the [`hir::Ty`] of an async function's lowered return type,
/// retrieve the `hir::Ty` representing the type the user originally wrote.
///
/// e.g. given the function:
///
/// ```
/// async fn foo() -> i32 { 2 }
/// ```
///
/// this function, given the lowered return type of `foo`, an [`OpaqueDef`] that implements `Future<Output=i32>`,
/// returns the `i32`.
///
/// [`OpaqueDef`]: hir::TyKind::OpaqueDef
fn get_future_inner_return_ty(&self, hir_ty: &'tcx hir::Ty<'tcx>) -> &'tcx hir::Ty<'tcx> {
let hir::TyKind::OpaqueDef(opaque_ty, _) = hir_ty.kind else {
span_bug!(
hir_ty.span,
"lowered return type of async fn is not OpaqueDef: {:?}",
hir_ty
);
};
if let hir::OpaqueTy { bounds: [hir::GenericBound::Trait(trait_ref)], .. } = opaque_ty
&& let Some(segment) = trait_ref.trait_ref.path.segments.last()
&& let Some(args) = segment.args
&& let [constraint] = args.constraints
&& constraint.ident.name == sym::Output
&& let Some(ty) = constraint.ty()
{
ty
} else {
span_bug!(
hir_ty.span,
"bounds from lowered return type of async fn did not match expected format: {opaque_ty:?}",
);
}
}

#[instrument(level = "trace", skip(self))]
fn give_name_if_anonymous_region_appears_in_yield_ty(
&self,
Expand Down
58 changes: 53 additions & 5 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_lint_defs::Applicability;
use rustc_lint_defs::builtin::BARE_TRAIT_OBJECTS;
use rustc_span::Span;
use rustc_span::{Span, sym};
use rustc_trait_selection::error_reporting::traits::suggestions::NextTypeParamName;

use super::HirTyLowerer;
Expand Down Expand Up @@ -181,14 +181,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
/// Make sure that we are in the condition to suggest `impl Trait`.
fn maybe_suggest_impl_trait(&self, self_ty: &hir::Ty<'_>, diag: &mut Diag<'_>) -> bool {
let tcx = self.tcx();
let parent_id = tcx.hir().get_parent_item(self_ty.hir_id).def_id;
let parent_node = tcx.hir_node_by_def_id(tcx.hir().get_parent_item(self_ty.hir_id).def_id);
// FIXME: If `type_alias_impl_trait` is enabled, also look for `Trait0<Ty = Trait1>`
// and suggest `Trait0<Ty = impl Trait1>`.
// Functions are found in three different contexts.
// 1. Independent functions
// 2. Functions inside trait blocks
// 3. Functions inside impl blocks
let (sig, generics) = match tcx.hir_node_by_def_id(parent_id) {
let (sig, generics) = match parent_node {
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, generics, _), .. }) => {
(sig, generics)
}
Expand Down Expand Up @@ -223,10 +223,17 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
tcx.parent_hir_node(self_ty.hir_id),
hir::Node::Ty(hir::Ty { kind: hir::TyKind::Ref(..), .. })
);
let is_non_trait_object = |ty: &'tcx hir::Ty<'_>| {
if sig.header.is_async() {
Self::get_future_inner_return_ty(ty).map_or(false, |ty| ty.hir_id == self_ty.hir_id)
} else {
ty.peel_refs().hir_id == self_ty.hir_id
}
};

// Suggestions for function return type.
if let hir::FnRetTy::Return(ty) = sig.decl.output
&& ty.peel_refs().hir_id == self_ty.hir_id
&& is_non_trait_object(ty)
{
let pre = if !is_dyn_compatible {
format!("`{trait_name}` is dyn-incompatible, ")
Expand Down Expand Up @@ -311,10 +318,21 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}

fn maybe_suggest_assoc_ty_bound(&self, self_ty: &hir::Ty<'_>, diag: &mut Diag<'_>) {
let mut parents = self.tcx().hir().parent_iter(self_ty.hir_id);
let mut parents = self.tcx().hir().parent_iter(self_ty.hir_id).peekable();
let is_async_fn = if let Some((_, parent)) = parents.peek()
&& let Some(sig) = parent.fn_sig()
&& sig.header.is_async()
&& let hir::FnRetTy::Return(ty) = sig.decl.output
&& Self::get_future_inner_return_ty(ty).is_some()
{
true
} else {
false
};

if let Some((_, hir::Node::AssocItemConstraint(constraint))) = parents.next()
&& let Some(obj_ty) = constraint.ty()
&& !is_async_fn
{
if let Some((_, hir::Node::TraitRef(..))) = parents.next()
&& let Some((_, hir::Node::Ty(ty))) = parents.next()
Expand Down Expand Up @@ -343,4 +361,34 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
);
}
}

/// From the [`hir::Ty`] of an async function's lowered return type,
/// retrieve the `hir::Ty` representing the type the user originally wrote.
///
/// e.g. given the function:
///
/// ```
/// async fn foo() -> i32 { 2 }
/// ```
///
/// this function, given the lowered return type of `foo`, an [`OpaqueDef`] that implements `Future<Output=i32>`,
/// returns the `i32`.
///
/// [`OpaqueDef`]: hir::TyKind::OpaqueDef
pub fn get_future_inner_return_ty<'a>(hir_ty: &'a hir::Ty<'a>) -> Option<&'a hir::Ty<'a>> {
let hir::TyKind::OpaqueDef(opaque_ty, _) = hir_ty.kind else {
return None;
};
if let hir::OpaqueTy { bounds: [hir::GenericBound::Trait(trait_ref)], .. } = opaque_ty
&& let Some(segment) = trait_ref.trait_ref.path.segments.last()
&& let Some(args) = segment.args
&& let [constraint] = args.constraints
&& constraint.ident.name == sym::Output
&& let Some(ty) = constraint.ty()
{
Some(ty)
} else {
None
}
}
}
8 changes: 8 additions & 0 deletions tests/ui/traits/suggest-impl-on-bare-trait-return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//@ edition:2021
trait Trait {}

fn fun() -> Trait { //~ ERROR expected a type, found a trait
todo!()
}

fn main() {}
18 changes: 18 additions & 0 deletions tests/ui/traits/suggest-impl-on-bare-trait-return.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error[E0782]: expected a type, found a trait
--> $DIR/suggest-impl-on-bare-trait-return.rs:4:13
|
LL | fn fun() -> Trait {
| ^^^^^
|
help: use `impl Trait` to return an opaque type, as long as you return a single underlying type
|
LL | fn fun() -> impl Trait {
| ++++
help: alternatively, you can return an owned trait object
|
LL | fn fun() -> Box<dyn Trait> {
| +++++++ +

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0782`.

0 comments on commit d63cc44

Please sign in to comment.