[ui] Incorrect hint: Ok(wrap) instead *Deref

[boozook]

Reproduced on versions:

• rustc 1.29.0-nightly (12ed235ad 2018-07-18)
• rustc 1.29.0-nightly (9fd3d7899 2018-07-07)
• rustc 1.27.0 (3eda71b00 2018-06-19)

Isolated example to reproduce:

use std::ops::Deref;

struct Error {}
struct State {}
struct StateDeref { inner: State }

impl Deref for StateDeref {
	type Target = State;
	fn deref(&self) -> &Self::Target { &self.inner }
}

fn get_state() -> Result<StateDeref, Error> { Err(Error {}) }

fn need_ref_state_from(state: &State) -> bool { true }

fn state_info() -> Result<bool, Error> { Ok(need_ref_state_from(&get_state()?)) }

Actual error:

error[E0308]: try expression alternatives have incompatible types
   --> src/main.rs:16:66
    |
16  | fn state_info() -> Result<bool, Error> { Ok(need_ref_state_from(&get_state()?)) }
    |                                                                  ^^^^^^^^^^^^
    |                                                                  |
    |                                                                  expected struct `State`, found struct `StateDeref`
    |                                                                  help: try wrapping with a success variant: `Ok(get_state()?)`
    |
    = note: expected type `State`
               found type `StateDeref`

error: aborting due to previous error

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

Mentioned suggestion about "wrap into Ok" is incorrect at all. There we should deref the value from the get_state()?.

---

Expected error: (something like this)

...
expected struct `State`, found struct `StateDeref`
help: try re-dereference(?) with a success variant: `&*get_state()?`

[zackmdavis]

Here's where the suggestion gets issued:

rust/src/librustc/infer/error_reporting/mod.rs

Lines 504 to 513 in 6cc78bf

	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
	);
	}
	},

The motivation was to solve #51632 (where a ?-expression appears in tail position, which seemed likely to be a common mistake), but it turns out that there are other situations where we run into a type mismatch between match arms of a desugared ? expression. We regret the error.

[kornelski]

Here's another case of this:

struct Foo;

fn takes_ref(_r: &Foo) {}

fn returns_result() -> Result<Foo,()> {}

fn main() -> Result<(),()> {
    takes_ref(returns_result()?);
    Ok(())
}

8 |     takes_ref(returns_result()?);
  |               ^^^^^^^^^^^^^^^^^
  |               |
  |               expected &Foo, found struct `Foo`
  |               help: try wrapping with a success variant: `Ok(returns_result()?)`
  |
  = note: expected type `&Foo`
             found type `Foo`

[estebank]

@zackmdavis I see what's happening, we're only checking wether the error came from a ? desugaring, disregarding the types involved. It should be possible to check that information as well, even if the suggestion is only provided when it's a Result where everything but the error type matches. (I think it's ok if the supplied error cannot be converted to the expected error, as this suggestion exposes newcomers to a non-obvious construct at least.)

[zackmdavis]

even if the suggestion is only provided when it's a Result

Clippy has helper functions to detect this that we might want to uplift ...

[zackmdavis]

(But for now I'm just going to remove the suggestion.)