Skip to content

Commit 414b083

Browse files
estebankestebank
committed
Rework impl Trait and Box<dyn Trait> return type suggestions in E0746
* If the return value types diverge, mention it and do not suggest `impl Trait` * If the returned `dyn Trait` is not object safe, mention it and do not suggest `Box<dyn Trait>` and boxing the returned values * If the function has arguments with borrowed types, suggest `&dyn Trait` * Tweak wording of `E0782` ``` error[E0746]: return type cannot have an unboxed trait object --> $DIR/dyn-trait-return-should-be-impl-trait.rs:13:13 | LL | fn bap() -> Trait { Struct } | ^^^^^ doesn't have a size known at compile-time | help: return an `impl Trait` instead of a `dyn Trait` | LL | fn bap() -> impl Trait { Struct } | ++++ help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new` | LL | fn bap() -> Box<dyn Trait> { Box::new(Struct) } | +++++++ + +++++++++ + ``` ``` error[E0746]: return type cannot have an unboxed trait object --> $DIR/object-unsafe-trait-in-return-position-dyn-trait.rs:36:18 | LL | fn can(x: &A) -> dyn NotObjectSafe { | ^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time | help: trait `NotObjectSafe` is not object safe, so you can't return a boxed trait object `Box<dyn NotObjectSafe>` --> $DIR/object-unsafe-trait-in-return-position-dyn-trait.rs:36:22 | LL | fn can(x: &A) -> dyn NotObjectSafe { | ^^^^^^^^^^^^^ help: return an `impl Trait` instead of a `dyn Trait` | LL | fn can(x: &A) -> impl NotObjectSafe { | ~~~~ help: alternatively, you might be able to borrow from the function's argument | LL | fn can(x: &A) -> &dyn NotObjectSafe { | + ``` ``` error[E0782]: trait objects must include the `dyn` keyword --> $DIR/not-on-bare-trait-2021.rs:12:19 | LL | fn bar(x: Foo) -> Foo { | ^^^ | help: use `impl Foo` to return an opaque type, as long as you return a single underlying type | LL | fn bar(x: Foo) -> impl Foo { | ++++ help: alternatively, you can return a boxed trait object | LL | fn bar(x: Foo) -> Box<dyn Foo> { | +++++++ + ```
1 parent d6eb0f5 commit 414b083

File tree

12 files changed

+281
-75
lines changed

12 files changed

+281
-75
lines changed

‎compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
125125
diag.multipart_suggestion_verbose(msg, impl_sugg, Applicability::MachineApplicable);
126126
if is_object_safe {
127127
diag.multipart_suggestion_verbose(
128-
"alternatively, you can return an owned trait object",
128+
"alternatively, you can return a boxed trait object",
129129
vec![
130130
(ty.span.shrink_to_lo(), "Box<dyn ".to_string()),
131131
(ty.span.shrink_to_hi(), ">".to_string()),

‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

Lines changed: 151 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::infer::InferCtxt;
1010
use crate::traits::{ImplDerivedObligationCause, NormalizeExt, ObligationCtxt};
1111

1212
use hir::def::CtorOf;
13+
use rustc_ast::TraitObjectSyntax;
1314
use rustc_data_structures::fx::FxHashSet;
1415
use rustc_data_structures::stack::ensure_sufficient_stack;
1516
use rustc_errors::{
@@ -1791,27 +1792,90 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
17911792
err.primary_message("return type cannot have an unboxed trait object");
17921793
err.children.clear();
17931794

1794-
let span = obligation.cause.span;
1795-
if let Ok(snip) = self.tcx.sess.source_map().span_to_snippet(span)
1796-
&& snip.starts_with("dyn ")
1797-
{
1798-
err.span_suggestion(
1799-
span.with_hi(span.lo() + BytePos(4)),
1800-
"return an `impl Trait` instead of a `dyn Trait`, \
1801-
if all returned values are the same type",
1802-
"impl ",
1803-
Applicability::MaybeIncorrect,
1804-
);
1805-
}
1806-
18071795
let body = self.tcx.hir().body(self.tcx.hir().body_owned_by(obligation.cause.body_id));
18081796

18091797
let mut visitor = ReturnsVisitor::default();
18101798
visitor.visit_body(body);
18111799

1812-
let mut sugg =
1813-
vec![(span.shrink_to_lo(), "Box<".to_string()), (span.shrink_to_hi(), ">".to_string())];
1814-
sugg.extend(visitor.returns.into_iter().flat_map(|expr| {
1800+
let fn_id = self.tcx.hir().body_owner(body.id());
1801+
let Some(fn_decl) = self.tcx.hir_node(fn_id).fn_decl() else {
1802+
return false;
1803+
};
1804+
let hir::FnRetTy::Return(hir::Ty {
1805+
kind: hir::TyKind::TraitObject(traits @ [tr, ..], _lt, syn),
1806+
span,
1807+
..
1808+
}) = fn_decl.output
1809+
else {
1810+
return false;
1811+
};
1812+
1813+
// Suggest `-> impl Trait`
1814+
let ret_tys = if let Some(typeck) = &self.typeck_results {
1815+
visitor
1816+
.returns
1817+
.iter()
1818+
.filter_map(|expr| typeck.node_type_opt(expr.hir_id).map(|ty| (expr, ty)))
1819+
.collect()
1820+
} else {
1821+
vec![]
1822+
};
1823+
1824+
// FIXME: account for `impl` assoc `fn`, where suggesting `Box<dyn Trait>` if `impl Trait`
1825+
// is expected is ok, but suggesting `impl Trait` where `Box<dyn Trait>` is expected is not.
1826+
// FIXME: account for the case where `-> dyn Trait` but the returned value is
1827+
// `Box<dyn Trait>`, like in `tests/ui/unsized/issue-91801.rs`.
1828+
1829+
let mut all_same_ty = true;
1830+
let mut prev_ty = None;
1831+
for (_, ty) in &ret_tys {
1832+
if let Some(prev_ty) = prev_ty {
1833+
if prev_ty != ty {
1834+
all_same_ty = false;
1835+
break;
1836+
}
1837+
} else {
1838+
prev_ty = Some(ty);
1839+
}
1840+
}
1841+
let mut alternatively = "";
1842+
if all_same_ty || visitor.returns.len() == 1 {
1843+
// We're certain that `impl Trait` will work.
1844+
err.span_suggestion_verbose(
1845+
// This will work for both `Dyn` and `None` object syntax.
1846+
span.until(tr.span),
1847+
"return an `impl Trait` instead of a `dyn Trait`",
1848+
"impl ",
1849+
Applicability::MachineApplicable,
1850+
);
1851+
alternatively = "alternatively, ";
1852+
} else {
1853+
let mut span: MultiSpan =
1854+
ret_tys.iter().map(|(expr, _)| expr.span).collect::<Vec<Span>>().into();
1855+
for (expr, ty) in &ret_tys {
1856+
span.push_span_label(expr.span, format!("this returned value has type `{ty}`"));
1857+
}
1858+
1859+
err.span_help(
1860+
span,
1861+
format!(
1862+
"the returned values do not all have the same type, so `impl Trait` can't be \
1863+
used",
1864+
),
1865+
);
1866+
}
1867+
1868+
// Suggest `-> Box<dyn Trait>`
1869+
let pre = match syn {
1870+
TraitObjectSyntax::None => "dyn ",
1871+
_ => "",
1872+
};
1873+
let mut sugg = vec![
1874+
(span.shrink_to_lo(), format!("Box<{pre}")),
1875+
(span.shrink_to_hi(), ">".to_string()),
1876+
];
1877+
1878+
sugg.extend(visitor.returns.iter().flat_map(|expr| {
18151879
let span =
18161880
expr.span.find_ancestor_in_same_ctxt(obligation.cause.span).unwrap_or(expr.span);
18171881
if !span.can_be_used_for_suggestions() {
@@ -1835,11 +1899,77 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
18351899
}
18361900
}));
18371901

1838-
err.multipart_suggestion(
1839-
"box the return type, and wrap all of the returned values in `Box::new`",
1840-
sugg,
1841-
Applicability::MaybeIncorrect,
1842-
);
1902+
let object_unsafe: Vec<_> = traits
1903+
.iter()
1904+
.filter_map(|tr| {
1905+
tr.trait_ref
1906+
.path
1907+
.res
1908+
.opt_def_id()
1909+
.filter(|def_id| !self.tcx.object_safety_violations(def_id).is_empty())
1910+
.map(|def_id| (def_id, tr.span))
1911+
})
1912+
.collect();
1913+
if !object_unsafe.is_empty() {
1914+
let span: MultiSpan =
1915+
object_unsafe.iter().map(|(_, sp)| *sp).collect::<Vec<Span>>().into();
1916+
err.span_help(
1917+
span,
1918+
format!(
1919+
"{} not object safe, so you can't return a boxed trait object `{}`",
1920+
if let [(def_id, _)] = &object_unsafe[..] {
1921+
format!("trait `{}` is", self.tcx.def_path_str(*def_id))
1922+
} else {
1923+
"the following traits are".to_string()
1924+
},
1925+
format!(
1926+
"Box<dyn {}>",
1927+
object_unsafe
1928+
.iter()
1929+
.map(|(def_id, _)| self.tcx.def_path_str(*def_id))
1930+
.collect::<Vec<_>>()
1931+
.join(" + ")
1932+
),
1933+
),
1934+
);
1935+
} else {
1936+
err.multipart_suggestion(
1937+
format!(
1938+
"{alternatively}box the return type to make a boxed trait object, and wrap \
1939+
all of the returned values in `Box::new`",
1940+
),
1941+
sugg,
1942+
Applicability::MaybeIncorrect,
1943+
);
1944+
alternatively = if alternatively.is_empty() { "alternatively, " } else { "finally, " };
1945+
}
1946+
1947+
// Suggest `-> &dyn Trait`
1948+
let borrow_input_tys: Vec<_> = fn_decl
1949+
.inputs
1950+
.iter()
1951+
.filter(|hir_ty| matches!(hir_ty.kind, hir::TyKind::Ref(..)))
1952+
.collect();
1953+
if !borrow_input_tys.is_empty() {
1954+
// Maybe returning a `&dyn Trait` borrowing from an argument might be ok?
1955+
err.span_suggestion_verbose(
1956+
// This will work for both `Dyn` and `None` object syntax.
1957+
span.shrink_to_lo(),
1958+
format!(
1959+
"{alternatively}you might be able to borrow from {}",
1960+
if borrow_input_tys.len() == 1 {
1961+
"the function's argument"
1962+
} else {
1963+
"one of the function's arguments"
1964+
},
1965+
),
1966+
match syn {
1967+
TraitObjectSyntax::None => "&dyn ",
1968+
_ => "&",
1969+
},
1970+
Applicability::MachineApplicable,
1971+
);
1972+
}
18431973

18441974
true
18451975
}

‎tests/ui/error-codes/E0746.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ error[E0746]: return type cannot have an unboxed trait object
44
LL | fn foo() -> dyn Trait { Struct }
55
| ^^^^^^^^^ doesn't have a size known at compile-time
66
|
7-
help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
7+
help: return an `impl Trait` instead of a `dyn Trait`
88
|
99
LL | fn foo() -> impl Trait { Struct }
1010
| ~~~~
11-
help: box the return type, and wrap all of the returned values in `Box::new`
11+
help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
1212
|
1313
LL | fn foo() -> Box<dyn Trait> { Box::new(Struct) }
1414
| ++++ + +++++++++ +
@@ -19,11 +19,11 @@ error[E0746]: return type cannot have an unboxed trait object
1919
LL | fn bar() -> dyn Trait {
2020
| ^^^^^^^^^ doesn't have a size known at compile-time
2121
|
22-
help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
22+
help: return an `impl Trait` instead of a `dyn Trait`
2323
|
2424
LL | fn bar() -> impl Trait {
2525
| ~~~~
26-
help: box the return type, and wrap all of the returned values in `Box::new`
26+
help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
2727
|
2828
LL ~ fn bar() -> Box<dyn Trait> {
2929
LL | if true {

‎tests/ui/impl-trait/dyn-trait-return-should-be-impl-trait.stderr

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,22 +48,26 @@ error[E0746]: return type cannot have an unboxed trait object
4848
LL | fn bap() -> Trait { Struct }
4949
| ^^^^^ doesn't have a size known at compile-time
5050
|
51-
help: box the return type, and wrap all of the returned values in `Box::new`
51+
help: return an `impl Trait` instead of a `dyn Trait`
5252
|
53-
LL | fn bap() -> Box<Trait> { Box::new(Struct) }
54-
| ++++ + +++++++++ +
53+
LL | fn bap() -> impl Trait { Struct }
54+
| ++++
55+
help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
56+
|
57+
LL | fn bap() -> Box<dyn Trait> { Box::new(Struct) }
58+
| +++++++ + +++++++++ +
5559

5660
error[E0746]: return type cannot have an unboxed trait object
5761
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:15:13
5862
|
5963
LL | fn ban() -> dyn Trait { Struct }
6064
| ^^^^^^^^^ doesn't have a size known at compile-time
6165
|
62-
help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
66+
help: return an `impl Trait` instead of a `dyn Trait`
6367
|
6468
LL | fn ban() -> impl Trait { Struct }
6569
| ~~~~
66-
help: box the return type, and wrap all of the returned values in `Box::new`
70+
help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
6771
|
6872
LL | fn ban() -> Box<dyn Trait> { Box::new(Struct) }
6973
| ++++ + +++++++++ +
@@ -74,11 +78,11 @@ error[E0746]: return type cannot have an unboxed trait object
7478
LL | fn bak() -> dyn Trait { unimplemented!() }
7579
| ^^^^^^^^^ doesn't have a size known at compile-time
7680
|
77-
help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
81+
help: return an `impl Trait` instead of a `dyn Trait`
7882
|
7983
LL | fn bak() -> impl Trait { unimplemented!() }
8084
| ~~~~
81-
help: box the return type, and wrap all of the returned values in `Box::new`
85+
help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
8286
|
8387
LL | fn bak() -> Box<dyn Trait> { Box::new(unimplemented!()) }
8488
| ++++ + +++++++++ +
@@ -89,11 +93,15 @@ error[E0746]: return type cannot have an unboxed trait object
8993
LL | fn bal() -> dyn Trait {
9094
| ^^^^^^^^^ doesn't have a size known at compile-time
9195
|
92-
help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
96+
help: the returned values do not all have the same type, so `impl Trait` can't be used
97+
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:21:16
9398
|
94-
LL | fn bal() -> impl Trait {
95-
| ~~~~
96-
help: box the return type, and wrap all of the returned values in `Box::new`
99+
LL | return Struct;
100+
| ^^^^^^ this returned value has type `Struct`
101+
LL | }
102+
LL | 42
103+
| ^^ this returned value has type `{integer}`
104+
help: box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
97105
|
98106
LL ~ fn bal() -> Box<dyn Trait> {
99107
LL | if true {
@@ -108,11 +116,15 @@ error[E0746]: return type cannot have an unboxed trait object
108116
LL | fn bax() -> dyn Trait {
109117
| ^^^^^^^^^ doesn't have a size known at compile-time
110118
|
111-
help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
119+
help: the returned values do not all have the same type, so `impl Trait` can't be used
120+
--> $DIR/dyn-trait-return-should-be-impl-trait.rs:27:9
112121
|
113-
LL | fn bax() -> impl Trait {
114-
| ~~~~
115-
help: box the return type, and wrap all of the returned values in `Box::new`
122+
LL | Struct
123+
| ^^^^^^ this returned value has type `Struct`
124+
LL | } else {
125+
LL | 42
126+
| ^^ this returned value has type `{integer}`
127+
help: box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
116128
|
117129
LL ~ fn bax() -> Box<dyn Trait> {
118130
LL | if true {
@@ -263,11 +275,11 @@ error[E0746]: return type cannot have an unboxed trait object
263275
LL | fn bat() -> dyn Trait {
264276
| ^^^^^^^^^ doesn't have a size known at compile-time
265277
|
266-
help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
278+
help: return an `impl Trait` instead of a `dyn Trait`
267279
|
268280
LL | fn bat() -> impl Trait {
269281
| ~~~~
270-
help: box the return type, and wrap all of the returned values in `Box::new`
282+
help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
271283
|
272284
LL ~ fn bat() -> Box<dyn Trait> {
273285
LL | if true {
@@ -282,11 +294,11 @@ error[E0746]: return type cannot have an unboxed trait object
282294
LL | fn bay() -> dyn Trait {
283295
| ^^^^^^^^^ doesn't have a size known at compile-time
284296
|
285-
help: return an `impl Trait` instead of a `dyn Trait`, if all returned values are the same type
297+
help: return an `impl Trait` instead of a `dyn Trait`
286298
|
287299
LL | fn bay() -> impl Trait {
288300
| ~~~~
289-
help: box the return type, and wrap all of the returned values in `Box::new`
301+
help: alternatively, box the return type to make a boxed trait object, and wrap all of the returned values in `Box::new`
290302
|
291303
LL ~ fn bay() -> Box<dyn Trait> {
292304
LL | if true {

‎tests/ui/impl-trait/object-unsafe-trait-in-return-position-dyn-trait.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,10 @@ fn cat() -> Box<dyn NotObjectSafe> { //~ ERROR the trait `NotObjectSafe` cannot
3333
Box::new(B) //~ ERROR cannot be made into an object
3434
}
3535

36+
fn can(x: &A) -> dyn NotObjectSafe { //~ ERROR the trait `NotObjectSafe` cannot be made into an object
37+
//~^ ERROR return type cannot have an unboxed trait object
38+
x
39+
}
40+
41+
3642
fn main() {}

0 commit comments

Comments
 (0)