Skip to content

Commit 5381796

Browse files
estebankestebank
committed
Point at fewer methods in the chain, only those that change the E type
1 parent 6c3879d commit 5381796

File tree

3 files changed

+64
-53
lines changed

3 files changed

+64
-53
lines changed

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

+47-29
Original file line numberDiff line numberDiff line change
@@ -1049,49 +1049,33 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
10491049
let mut prev_ty = self.resolve_vars_if_possible(
10501050
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
10511051
);
1052-
let mut annotate_expr = |span: Span, prev_ty: Ty<'tcx>, self_ty: Ty<'tcx>| -> bool {
1053-
// We always look at the `E` type, because that's the only one affected by `?`. If the
1054-
// incorrect `Result<T, E>` is because of the `T`, we'll get an E0308 on the whole
1055-
// expression, after the `?` has "unwrapped" the `T`.
1052+
1053+
// We always look at the `E` type, because that's the only one affected by `?`. If the
1054+
// incorrect `Result<T, E>` is because of the `T`, we'll get an E0308 on the whole
1055+
// expression, after the `?` has "unwrapped" the `T`.
1056+
let get_e_type = |prev_ty: Ty<'tcx>| -> Option<Ty<'tcx>> {
10561057
let ty::Adt(def, args) = prev_ty.kind() else {
1057-
return false;
1058+
return None;
10581059
};
10591060
let Some(arg) = args.get(1) else {
1060-
return false;
1061+
return None;
10611062
};
10621063
if !self.tcx.is_diagnostic_item(sym::Result, def.did()) {
1063-
return false;
1064+
return None;
10641065
}
1065-
let can = if self
1066-
.infcx
1067-
.type_implements_trait(
1068-
self.tcx.get_diagnostic_item(sym::From).unwrap(),
1069-
[self_ty.into(), *arg],
1070-
obligation.param_env,
1071-
)
1072-
.must_apply_modulo_regions()
1073-
{
1074-
"can"
1075-
} else {
1076-
"can't"
1077-
};
1078-
err.span_label(
1079-
span,
1080-
format!("this {can} be annotated with `?` because it has type `{prev_ty}`"),
1081-
);
1082-
true
1066+
Some(arg.as_type()?)
10831067
};
10841068

1069+
let mut chain = vec![];
1070+
10851071
// The following logic is simlar to `point_at_chain`, but that's focused on associated types
10861072
let mut expr = expr;
10871073
while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind {
10881074
// Point at every method call in the chain with the `Result` type.
10891075
// let foo = bar.iter().map(mapper)?;
10901076
// ------ -----------
10911077
expr = rcvr_expr;
1092-
if !annotate_expr(span, prev_ty, self_ty) {
1093-
break;
1094-
}
1078+
chain.push((span, prev_ty));
10951079

10961080
prev_ty = self.resolve_vars_if_possible(
10971081
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
@@ -1121,7 +1105,41 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
11211105
prev_ty = self.resolve_vars_if_possible(
11221106
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
11231107
);
1124-
annotate_expr(expr.span, prev_ty, self_ty);
1108+
chain.push((expr.span, prev_ty));
1109+
1110+
let mut prev = None;
1111+
for (span, err_ty) in chain.into_iter().rev() {
1112+
let err_ty = get_e_type(err_ty);
1113+
let err_ty = match (err_ty, prev) {
1114+
(Some(err_ty), Some(prev)) if !self.can_eq(obligation.param_env, err_ty, prev) => {
1115+
err_ty
1116+
}
1117+
(Some(err_ty), None) => err_ty,
1118+
_ => {
1119+
prev = err_ty;
1120+
continue;
1121+
}
1122+
};
1123+
if self
1124+
.infcx
1125+
.type_implements_trait(
1126+
self.tcx.get_diagnostic_item(sym::From).unwrap(),
1127+
[self_ty, err_ty],
1128+
obligation.param_env,
1129+
)
1130+
.must_apply_modulo_regions()
1131+
{
1132+
err.span_label(span, format!("this has type `Result<_, {err_ty}>`"));
1133+
} else {
1134+
err.span_label(
1135+
span,
1136+
format!(
1137+
"this can't be annotated with `?` because it has type `Result<_, {err_ty}>`",
1138+
),
1139+
);
1140+
}
1141+
prev = Some(err_ty);
1142+
}
11251143
}
11261144

11271145
fn report_const_param_not_wf(

‎tests/ui/traits/question-mark-result-err-mismatch.rs

+7-8
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,33 @@ fn foo() -> Result<String, String> { //~ NOTE expected `String` because of this
33
let x = test
44
.split_whitespace()
55
.next()
6-
.ok_or_else(|| { //~ NOTE this can be annotated with `?` because it has type `Result<&str, &str>`
6+
.ok_or_else(|| { //~ NOTE this has type `Result<_, &str>`
77
"Couldn't split the test string"
88
});
99
let one = x
10-
.map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), &str>`
11-
.map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<(), ()>`
10+
.map(|s| ())
11+
.map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
1212
.map(|()| "")?; //~ ERROR `?` couldn't convert the error to `String`
1313
//~^ NOTE in this expansion of desugaring of operator `?`
1414
//~| NOTE in this expansion of desugaring of operator `?`
1515
//~| NOTE in this expansion of desugaring of operator `?`
1616
//~| NOTE in this expansion of desugaring of operator `?`
17-
//~| NOTE this can't be annotated with `?` because it has type `Result<&str, ()>`
1817
//~| NOTE the trait `From<()>` is not implemented for `String`
1918
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
2019
//~| NOTE required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
2120
Ok(one.to_string())
2221
}
2322

2423
fn bar() -> Result<(), String> { //~ NOTE expected `String` because of this
25-
let x = foo(); //~ NOTE this can be annotated with `?` because it has type `Result<String, String>`
24+
let x = foo(); //~ NOTE this has type `Result<_, String>`
2625
let one = x
27-
.map(|s| ()) //~ NOTE this can be annotated with `?` because it has type `Result<(), String>`
26+
.map(|s| ())
2827
.map_err(|_| ())?; //~ ERROR `?` couldn't convert the error to `String`
2928
//~^ NOTE in this expansion of desugaring of operator `?`
3029
//~| NOTE in this expansion of desugaring of operator `?`
3130
//~| NOTE in this expansion of desugaring of operator `?`
3231
//~| NOTE in this expansion of desugaring of operator `?`
33-
//~| NOTE this can't be annotated with `?` because it has type `Result<(), ()>`
32+
//~| NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
3433
//~| NOTE the trait `From<()>` is not implemented for `String`
3534
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
3635
//~| NOTE required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
@@ -42,7 +41,7 @@ fn baz() -> Result<String, String> { //~ NOTE expected `String` because of this
4241
let one = test
4342
.split_whitespace()
4443
.next()
45-
.ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<&str, ()>`
44+
.ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
4645
"Couldn't split the test string";
4746
})?;
4847
//~^ ERROR `?` couldn't convert the error to `String`

‎tests/ui/traits/question-mark-result-err-mismatch.stderr

+10-16
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,12 @@ LL | .ok_or_else(|| {
88
| __________-
99
LL | | "Couldn't split the test string"
1010
LL | | });
11-
| |__________- this can be annotated with `?` because it has type `Result<&str, &str>`
12-
LL | let one = x
13-
LL | .map(|s| ())
14-
| ----------- this can be annotated with `?` because it has type `Result<(), &str>`
11+
| |__________- this has type `Result<_, &str>`
12+
...
1513
LL | .map_err(|_| ())
16-
| --------------- this can't be annotated with `?` because it has type `Result<(), ()>`
14+
| --------------- this can't be annotated with `?` because it has type `Result<_, ()>`
1715
LL | .map(|()| "")?;
18-
| ------------^ the trait `From<()>` is not implemented for `String`
19-
| |
20-
| this can't be annotated with `?` because it has type `Result<&str, ()>`
16+
| ^ the trait `From<()>` is not implemented for `String`
2117
|
2218
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
2319
= help: the following other types implement trait `From<T>`:
@@ -30,19 +26,17 @@ LL | .map(|()| "")?;
3026
= note: required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
3127

3228
error[E0277]: `?` couldn't convert the error to `String`
33-
--> $DIR/question-mark-result-err-mismatch.rs:28:25
29+
--> $DIR/question-mark-result-err-mismatch.rs:27:25
3430
|
3531
LL | fn bar() -> Result<(), String> {
3632
| ------------------ expected `String` because of this
3733
LL | let x = foo();
38-
| ----- this can be annotated with `?` because it has type `Result<String, String>`
39-
LL | let one = x
40-
LL | .map(|s| ())
41-
| ----------- this can be annotated with `?` because it has type `Result<(), String>`
34+
| ----- this has type `Result<_, String>`
35+
...
4236
LL | .map_err(|_| ())?;
4337
| ---------------^ the trait `From<()>` is not implemented for `String`
4438
| |
45-
| this can't be annotated with `?` because it has type `Result<(), ()>`
39+
| this can't be annotated with `?` because it has type `Result<_, ()>`
4640
|
4741
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
4842
= help: the following other types implement trait `From<T>`:
@@ -55,7 +49,7 @@ LL | .map_err(|_| ())?;
5549
= note: required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
5650

5751
error[E0277]: `?` couldn't convert the error to `String`
58-
--> $DIR/question-mark-result-err-mismatch.rs:47:11
52+
--> $DIR/question-mark-result-err-mismatch.rs:46:11
5953
|
6054
LL | fn baz() -> Result<String, String> {
6155
| ---------------------- expected `String` because of this
@@ -66,7 +60,7 @@ LL | | "Couldn't split the test string";
6660
LL | | })?;
6761
| | -^ the trait `From<()>` is not implemented for `String`
6862
| |__________|
69-
| this can't be annotated with `?` because it has type `Result<&str, ()>`
63+
| this can't be annotated with `?` because it has type `Result<_, ()>`
7064
|
7165
= note: the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
7266
= help: the following other types implement trait `From<T>`:

0 commit comments

Comments
 (0)