Skip to content

Commit

Permalink
Rollup merge of rust-lang#51938 - zackmdavis:and_the_case_of_the_leak…
Browse files Browse the repository at this point in the history
…ing_desugar, r=estebank

in which we plug the crack where `?`-desugaring leaked into errors

Most of the time, it's not a problem that the types of the arm bodies in
a desugared-from-`?` match are different (that is, specifically: in `x?`
where x is a `Result<A, B>`, the `Ok` arm body is an `A`, whereas the
`Err` arm diverges to return a `Result<A, B>`), because they're being
assigned to different places. But in tail position, the types do need to
match, and our error message was explicitly referring to "match arms",
which is confusing when there's no `match` in the sweetly sugared
source.

It is not without some misgivings that we pollute the clarity-of-purpose
of `note_error_origin` with the suggestion to wrap with `Ok` (the other
branches are pointing out the odd-arm-out in the HIR that is the origin
of the error; the new branch that issues the `Ok` suggestion is serving
a different purpose), but it's the natural place to do it given that
we're already matching on `ObligationCauseCode::MatchExpressionArm {
arm_span, source }` there.

Resolves rust-lang#51632.
  • Loading branch information
Mark-Simulacrum authored Jul 2, 2018
2 parents 7cbe019 + 6cc78bf commit 898882b
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 2 deletions.
19 changes: 17 additions & 2 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,17 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
} else {
err.span_label(arm_span, msg);
}
}
},
hir::MatchSource::TryDesugar => { // Issue #51632
if let Ok(try_snippet) = self.tcx.sess.codemap().span_to_snippet(arm_span) {
err.span_suggestion_with_applicability(
arm_span,
"try wrapping with a success variant",
format!("Ok({})", try_snippet),
Applicability::MachineApplicable
);
}
},
_ => {
let msg = "match arm with an incompatible type";
if self.tcx.sess.codemap().is_multiline(arm_span) {
Expand Down Expand Up @@ -1312,7 +1322,12 @@ impl<'tcx> ObligationCause<'tcx> {
match self.code {
CompareImplMethodObligation { .. } => Error0308("method not compatible with trait"),
MatchExpressionArm { source, .. } => Error0308(match source {
hir::MatchSource::IfLetDesugar { .. } => "`if let` arms have incompatible types",
hir::MatchSource::IfLetDesugar { .. } => {
"`if let` arms have incompatible types"
},
hir::MatchSource::TryDesugar => {
"try expression alternatives have incompatible types"
},
_ => "match arms have incompatible types",
}),
IfExpression => Error0308("if and else have incompatible types"),
Expand Down
25 changes: 25 additions & 0 deletions src/test/ui/issue-51632-try-desugar-incompatible-types.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// run-rustfix

#![allow(dead_code)]

fn missing_discourses() -> Result<isize, ()> {
Ok(1)
}

fn forbidden_narratives() -> Result<isize, ()> {
Ok(missing_discourses()?)
//~^ ERROR try expression alternatives have incompatible types
//~| HELP try wrapping with a success variant
}

fn main() {}
25 changes: 25 additions & 0 deletions src/test/ui/issue-51632-try-desugar-incompatible-types.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// run-rustfix

#![allow(dead_code)]

fn missing_discourses() -> Result<isize, ()> {
Ok(1)
}

fn forbidden_narratives() -> Result<isize, ()> {
missing_discourses()?
//~^ ERROR try expression alternatives have incompatible types
//~| HELP try wrapping with a success variant
}

fn main() {}
15 changes: 15 additions & 0 deletions src/test/ui/issue-51632-try-desugar-incompatible-types.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0308]: try expression alternatives have incompatible types
--> $DIR/issue-51632-try-desugar-incompatible-types.rs:20:5
|
LL | missing_discourses()?
| ^^^^^^^^^^^^^^^^^^^^^
| |
| expected enum `std::result::Result`, found isize
| help: try wrapping with a success variant: `Ok(missing_discourses()?)`
|
= note: expected type `std::result::Result<isize, ()>`
found type `isize`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

0 comments on commit 898882b

Please sign in to comment.