Skip to content

Commit faa6bee

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

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

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

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

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