Skip to content

Commit 276b74d

Browse files
estebankestebank
committed
Point at fewer methods in the chain, only those that change the E type
1 parent 2faaea9 commit 276b74d

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

1032+
let mut chain = vec![];
1033+
10481034
// The following logic is simlar to `point_at_chain`, but that's focused on associated types
10491035
let mut expr = expr;
10501036
while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind {
10511037
// Point at every method call in the chain with the `Result` type.
10521038
// let foo = bar.iter().map(mapper)?;
10531039
// ------ -----------
10541040
expr = rcvr_expr;
1055-
if !annotate_expr(span, prev_ty, self_ty) {
1056-
break;
1057-
}
1041+
chain.push((span, prev_ty));
10581042

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

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