Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mismatched types diagnostic for async fn return type trusts random returns over function signature #54326

Closed
Arnavion opened this issue Sep 18, 2018 · 8 comments
Assignees
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-frontend Area: Compiler frontend (errors, parsing and HIR) AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Arnavion
Copy link

With

#![feature(async_await, futures_api)]

async fn foo() -> i32 {
    if false {
        return Ok(6);
    }

    5
}

the errors are:

error[E0308]: mismatched types
 --> src/lib.rs:8:5
  |
8 |     5
  |     ^ expected enum `std::result::Result`, found integral variable
  |
  = note: expected type `std::result::Result<{integer}, _>`
             found type `{integer}`

error[E0271]: type mismatch resolving `<impl std::future::Future as std::future::Future>::Output == i32`
 --> src/lib.rs:3:19
  |
3 | async fn foo() -> i32 {
  |                   ^^^ expected enum `std::result::Result`, found i32
  |
  = note: expected type `std::result::Result<{integer}, _>`
             found type `i32`
  = note: the return type of a function must have a statically known size

error: aborting due to 2 previous errors

It seems the compiler prefers to trust the return Ok(6) line over the function signature, so it ends up marking the function signature and the one return that matches that signature as errors.

In comparison, the diagnostic for regular non-async fns trusts the function signature and flags the return Ok(6) as the error.

fn foo() -> i32 {
    if false {
        return Ok(6);
    }

    5
}
error[E0308]: mismatched types
 --> src/lib.rs:3:16
  |
3 |         return Ok(6);
  |                ^^^^^ expected i32, found enum `std::result::Result`
  |
  = note: expected type `i32`
             found type `std::result::Result<{integer}, _>`

error: aborting due to previous error

This latter diagnostic is much better since the user presumably wrote the function signature intentionally.


I hit this while refactoring a large async fn, and it took some time to figure out why the compiler kept insisting that the function should return a Result when I had no intention for it to do that - a stray early-return in the middle of the function that was returning an Ok().

@Centril Centril added A-frontend Area: Compiler frontend (errors, parsing and HIR) A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. WG-compiler-front A-async-await Area: Async & Await labels Sep 18, 2018
@cramertj cramertj self-assigned this Sep 19, 2018
@cramertj
Copy link
Member

cramertj commented Sep 19, 2018

The code that lowers async fn body doesn't currently get passed the expected return type of the function, instead allowing it to be inferred. I'll take this one-- should be an easy fix.

Edit: was not an easy fix-- what I had in mind would require duplicating the return type in the HIR, which is hard / not possible.

@eddyb
Copy link
Member

eddyb commented Sep 19, 2018

This shouldn't be solved during desugaring, IMO - we already handle closures nicely, e.g.:

fn foo() -> impl Fn() -> i32 {
    || {
        if false {
            return Ok(6);
        }
    
        5
    }
}

The above example gets the nicer error - but if you put the closure in a variable before returning, you get the suboptimal pair of errors. For generators, even the below example gets the worse errors:

#![feature(generators, generator_trait)]

use std::ops::Generator;

fn foo() -> impl Generator<Return = i32> {
    || {
        if false {
            return Ok(6);
        }
        
        yield ();
    
        5
    }
}

The logic for closures follows below (I think we should generalize it a bit and use it for generators):

/// Given the expected type, figures out what it can about this closure we
/// are about to type check:
fn deduce_expectations_from_expected_type(
&self,
expected_ty: Ty<'tcx>,
) -> (Option<ExpectedSig<'tcx>>, Option<ty::ClosureKind>) {
debug!(
"deduce_expectations_from_expected_type(expected_ty={:?})",
expected_ty
);
match expected_ty.sty {
ty::Dynamic(ref object_type, ..) => {
let sig = object_type
.projection_bounds()
.filter_map(|pb| {
let pb = pb.with_self_ty(self.tcx, self.tcx.types.err);
self.deduce_sig_from_projection(None, &pb)
})
.next();
let kind = object_type
.principal()
.and_then(|p| self.tcx.lang_items().fn_trait_kind(p.def_id()));
(sig, kind)
}
ty::Infer(ty::TyVar(vid)) => self.deduce_expectations_from_obligations(vid),
ty::FnPtr(sig) => {
let expected_sig = ExpectedSig {
cause_span: None,
sig: sig.skip_binder().clone(),
};
(Some(expected_sig), Some(ty::ClosureKind::Fn))
}
_ => (None, None),
}
}
fn deduce_expectations_from_obligations(
&self,
expected_vid: ty::TyVid,
) -> (Option<ExpectedSig<'tcx>>, Option<ty::ClosureKind>) {
let fulfillment_cx = self.fulfillment_cx.borrow();
// Here `expected_ty` is known to be a type inference variable.
let expected_sig = fulfillment_cx
.pending_obligations()
.iter()
.filter_map(|obligation| {
debug!(
"deduce_expectations_from_obligations: obligation.predicate={:?}",
obligation.predicate
);
match obligation.predicate {
// Given a Projection predicate, we can potentially infer
// the complete signature.
ty::Predicate::Projection(ref proj_predicate) => {
let trait_ref = proj_predicate.to_poly_trait_ref(self.tcx);
self.self_type_matches_expected_vid(trait_ref, expected_vid)
.and_then(|_| {
self.deduce_sig_from_projection(
Some(obligation.cause.span),
proj_predicate,
)
})
}
_ => None,
}
})
.next();
// Even if we can't infer the full signature, we may be able to
// infer the kind. This can occur if there is a trait-reference
// like `F : Fn<A>`. Note that due to subtyping we could encounter
// many viable options, so pick the most restrictive.
let expected_kind = fulfillment_cx
.pending_obligations()
.iter()
.filter_map(|obligation| {
let opt_trait_ref = match obligation.predicate {
ty::Predicate::Projection(ref data) => Some(data.to_poly_trait_ref(self.tcx)),
ty::Predicate::Trait(ref data) => Some(data.to_poly_trait_ref()),
ty::Predicate::Subtype(..) => None,
ty::Predicate::RegionOutlives(..) => None,
ty::Predicate::TypeOutlives(..) => None,
ty::Predicate::WellFormed(..) => None,
ty::Predicate::ObjectSafe(..) => None,
ty::Predicate::ConstEvaluatable(..) => None,
// NB: This predicate is created by breaking down a
// `ClosureType: FnFoo()` predicate, where
// `ClosureType` represents some `Closure`. It can't
// possibly be referring to the current closure,
// because we haven't produced the `Closure` for
// this closure yet; this is exactly why the other
// code is looking for a self type of a unresolved
// inference variable.
ty::Predicate::ClosureKind(..) => None,
};
opt_trait_ref
.and_then(|tr| self.self_type_matches_expected_vid(tr, expected_vid))
.and_then(|tr| self.tcx.lang_items().fn_trait_kind(tr.def_id()))
})
.fold(None, |best, cur| {
Some(best.map_or(cur, |best| cmp::min(best, cur)))
});
(expected_sig, expected_kind)
}
/// Given a projection like "<F as Fn(X)>::Result == Y", we can deduce
/// everything we need to know about a closure.
///
/// The `cause_span` should be the span that caused us to
/// have this expected signature, or `None` if we can't readily
/// know that.
fn deduce_sig_from_projection(
&self,
cause_span: Option<Span>,
projection: &ty::PolyProjectionPredicate<'tcx>,
) -> Option<ExpectedSig<'tcx>> {
let tcx = self.tcx;
debug!("deduce_sig_from_projection({:?})", projection);
let trait_ref = projection.to_poly_trait_ref(tcx);
if tcx.lang_items().fn_trait_kind(trait_ref.def_id()).is_none() {
return None;
}
let arg_param_ty = trait_ref.skip_binder().substs.type_at(1);
let arg_param_ty = self.resolve_type_vars_if_possible(&arg_param_ty);
debug!(
"deduce_sig_from_projection: arg_param_ty {:?}",
arg_param_ty
);
let input_tys = match arg_param_ty.sty {
ty::Tuple(tys) => tys.into_iter(),
_ => {
return None;
}
};
let ret_param_ty = projection.skip_binder().ty;
let ret_param_ty = self.resolve_type_vars_if_possible(&ret_param_ty);
debug!(
"deduce_sig_from_projection: ret_param_ty {:?}",
ret_param_ty
);
let sig = self.tcx.mk_fn_sig(
input_tys.cloned(),
ret_param_ty,
false,
hir::Unsafety::Normal,
Abi::Rust,
);
debug!("deduce_sig_from_projection: sig {:?}", sig);
Some(ExpectedSig { cause_span, sig })
}
fn self_type_matches_expected_vid(
&self,
trait_ref: ty::PolyTraitRef<'tcx>,
expected_vid: ty::TyVid,
) -> Option<ty::PolyTraitRef<'tcx>> {
let self_ty = self.shallow_resolve(trait_ref.self_ty());
debug!(
"self_type_matches_expected_vid(trait_ref={:?}, self_ty={:?})",
trait_ref, self_ty
);
match self_ty.sty {
ty::Infer(ty::TyVar(v)) if expected_vid == v => Some(trait_ref),
_ => None,
}
}

@nikomatsakis nikomatsakis added AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 5, 2019
@nikomatsakis
Copy link
Contributor

We decided that this bug is not blocking stabilization; confusing though it may be, it's not a severe enough "diagnostic fail" to block stabilization.

@nikomatsakis
Copy link
Contributor

Nonetheless, I've tagged this with E-needs-mentor because it seems eminently fixable, even if not a blocker.

@davidtwco
Copy link
Member

Current status of this after #60592 is in this Zulip topic.

@richard-uk1
Copy link
Contributor

I've hit this today and was confused for a while. Here's a playground.

@jonas-schievink
Copy link
Contributor

This is not limited to closures/generators/async await but seems to occur generally with impl Trait in return position:

trait Tr {
    type A;
}

impl<A> Tr for A {
    type A = A;
}

fn f() -> impl Tr<A=u8> {
    ()
}
error[E0271]: type mismatch resolving `<() as Tr>::A == u8`
 --> src/lib.rs:9:11
  |
9 | fn f() -> impl Tr<A=u8> {
  |           ^^^^^^^^^^^^^ expected (), found u8
  |
  = note: expected type `()`
             found type `u8`
  = note: the return type of a function must have a statically known size

Additionally, the second note is wrong (() is Sized).

This does not happen when changing the code to use a boxed trait instead (expected and found are in the right order).

@nikomatsakis
Copy link
Contributor

We actually solved the initial example given (playground):

async fn foo() -> i32 {
    if false {
        return Ok(6);
    }

    5
}

fn main() { }

gives

error[E0308]: mismatched types
 --> src/main.rs:3:16
  |
3 |         return Ok(6);
  |                ^^^^^ expected i32, found enum `std::result::Result`
  |
  = note: expected type `i32`
             found type `std::result::Result<{integer}, _>`

I think the case that @jonas-schievink is describing is pretty different. I moved it to #68561 and I'm closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-frontend Area: Compiler frontend (errors, parsing and HIR) AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants