Skip to content

Commit a1d06a3

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

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
@@ -1008,49 +1008,33 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
10081008
let mut prev_ty = self.resolve_vars_if_possible(
10091009
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
10101010
);
1011-
let mut annotate_expr = |span: Span, prev_ty: Ty<'tcx>, self_ty: Ty<'tcx>| -> bool {
1012-
// We always look at the `E` type, because that's the only one affected by `?`. If the
1013-
// incorrect `Result<T, E>` is because of the `T`, we'll get an E0308 on the whole
1014-
// expression, after the `?` has "unwrapped" the `T`.
1011+
1012+
// We always look at the `E` type, because that's the only one affected by `?`. If the
1013+
// incorrect `Result<T, E>` is because of the `T`, we'll get an E0308 on the whole
1014+
// expression, after the `?` has "unwrapped" the `T`.
1015+
let get_e_type = |prev_ty: Ty<'tcx>| -> Option<Ty<'tcx>> {
10151016
let ty::Adt(def, args) = prev_ty.kind() else {
1016-
return false;
1017+
return None;
10171018
};
10181019
let Some(arg) = args.get(1) else {
1019-
return false;
1020+
return None;
10201021
};
10211022
if !self.tcx.is_diagnostic_item(sym::Result, def.did()) {
1022-
return false;
1023+
return None;
10231024
}
1024-
let can = if self
1025-
.infcx
1026-
.type_implements_trait(
1027-
self.tcx.get_diagnostic_item(sym::From).unwrap(),
1028-
[self_ty.into(), *arg],
1029-
obligation.param_env,
1030-
)
1031-
.must_apply_modulo_regions()
1032-
{
1033-
"can"
1034-
} else {
1035-
"can't"
1036-
};
1037-
err.span_label(
1038-
span,
1039-
format!("this {can} be annotated with `?` because it has type `{prev_ty}`"),
1040-
);
1041-
true
1025+
Some(arg.as_type()?)
10421026
};
10431027

1028+
let mut chain = vec![];
1029+
10441030
// The following logic is simlar to `point_at_chain`, but that's focused on associated types
10451031
let mut expr = expr;
10461032
while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind {
10471033
// Point at every method call in the chain with the `Result` type.
10481034
// let foo = bar.iter().map(mapper)?;
10491035
// ------ -----------
10501036
expr = rcvr_expr;
1051-
if !annotate_expr(span, prev_ty, self_ty) {
1052-
break;
1053-
}
1037+
chain.push((span, prev_ty));
10541038

10551039
prev_ty = self.resolve_vars_if_possible(
10561040
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
@@ -1078,7 +1062,41 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
10781062
prev_ty = self.resolve_vars_if_possible(
10791063
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
10801064
);
1081-
annotate_expr(expr.span, prev_ty, self_ty);
1065+
chain.push((expr.span, prev_ty));
1066+
1067+
let mut prev = None;
1068+
for (span, err_ty) in chain.into_iter().rev() {
1069+
let err_ty = get_e_type(err_ty);
1070+
let err_ty = match (err_ty, prev) {
1071+
(Some(err_ty), Some(prev)) if !self.can_eq(obligation.param_env, err_ty, prev) => {
1072+
err_ty
1073+
}
1074+
(Some(err_ty), None) => err_ty,
1075+
_ => {
1076+
prev = err_ty;
1077+
continue;
1078+
}
1079+
};
1080+
if self
1081+
.infcx
1082+
.type_implements_trait(
1083+
self.tcx.get_diagnostic_item(sym::From).unwrap(),
1084+
[self_ty, err_ty],
1085+
obligation.param_env,
1086+
)
1087+
.must_apply_modulo_regions()
1088+
{
1089+
err.span_label(span, format!("this has type `Result<_, {err_ty}>`"));
1090+
} else {
1091+
err.span_label(
1092+
span,
1093+
format!(
1094+
"this can't be annotated with `?` because it has type `Result<_, {err_ty}>`",
1095+
),
1096+
);
1097+
}
1098+
prev = Some(err_ty);
1099+
}
10821100
}
10831101

10841102
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)