Skip to content

Confusing/incorrect error message with incoherent implementations and async blocks #67651

Closed

Description

@syntacticsugarglider
use std::future::Future;

pub trait Bound {}

pub struct Error;

impl Bound for Error {}

pub struct Wrap;

impl<T: Bound> From<T> for Wrap {
    fn from(input: T) -> Self {
        Wrap
    }
}

impl<T: Into<Wrap>> From<T> for Error {
    fn from(input: T) -> Self {
        Error
    }
}

fn fail() -> impl Future<Output = Result<(), Error>> {
    async move {
        let a: Result<(), Error> = Ok(());
        let a: () = a?;
        Ok(())
    }
}

This fails with type annotations required: cannot resolve Error: std::convert::From<Error> (E0283) instead of the expected conflicting implementations of trait std::convert::From<Error> for type Error (E0119). Changing fail to

fn fail() -> Result<(), Error> {
    let a: Result<(), Error> = Ok(());
    let a: () = a?;
    Ok(())
}

i.e. eliminating the async block leads to the expected error message being produced.

playground demonstration

Activity

  1. added
    A-diagnosticsArea: Messages for errors, warnings, and lints
    C-enhancementCategory: An issue proposing an enhancement or a PR with one.
    D-confusingDiagnostics: Confusing error or lint that should be reworked.
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    on Dec 27, 2019
  2. added
    AsyncAwait-PolishAsync-await issues that are part of the "polish" area
    AsyncAwait-TriagedAsync-await issues that have been triaged during a working group meeting.
    on Jan 7, 2020
  3. tmandry commented on Jan 7, 2020

    @tmandry
    Member

    @Centril said they'd look into minimizing this further.

  4. tmandry commented on Jan 7, 2020

    @tmandry
    Member

    Also assigning @nikomatsakis to look into this along with #66312.

  5. Centril commented on Jan 7, 2020

    @Centril
    Contributor

    Reduced:

    trait From {
        fn from();
    }
    
    impl From for () {
        fn from() {}
    }
    
    impl From for () {
        fn from() {}
    }
    
    fn af() -> impl core::future::Future<Output = ()> {
        async move { From::from() }
    }
    
    /*
    fn f() -> () {
        From::from()
    }
    */
  6. removed their assignment
    on Jan 7, 2020
  7. nikomatsakis commented on Jan 8, 2020

    @nikomatsakis
    Contributor

    I've been investigating this. I'm not sure I understand yet what is going on, but I'm going to leave a few breadcrumbs as I go. To start, I think part of the problem lies on this line:

    return tcx.typeck_tables_of(def_id).node_type(hir_id);

    Specifically what is happening here is that we are executing the type_of query on a generator, and this is triggering us to do a full typeck. There is probably a good reason for this, but it's not generally how things are meant to work. This query is meant to be "modular", in that we can create the type for all items without type checking their bodies (this is possible because, for example, Rust requires us to fully specify the type signature for functions). I'm not quite sure why generators behave differently -- closures, for example, do not, as you can see if I give a bit more context:

    Node::Expr(&hir::Expr { kind: hir::ExprKind::Closure(.., gen), .. }) => {
    if gen.is_some() {
    return tcx.typeck_tables_of(def_id).node_type(hir_id);
    }
    let substs = InternalSubsts::identity_for_item(tcx, def_id);
    tcx.mk_closure(def_id, substs)
    }

    Here, on lines 1398-1399, you can see that for a closure we create a type with "identity substitutions", which means a closure type like Closure<P0...Pn> where Pi is a reference to a type parameter.

    (I see that opaque types also behave differently; it seems like tcx.type_of(opaque) is used to fetch the concrete type of an opaque type. I actually think we should make that a distinct query, but I think it's neither here nor there.)

    (I'm going to look a bit more at why generators behave this way now, not obvious to me that they must -- @Zoxc may have some details here?)

    UPDATE: Based on some exploration of git history, this line goes way back to the original gen branch at least.

  8. nikomatsakis commented on Jan 8, 2020

    @nikomatsakis
    Contributor

    OK, I can confirm that the "naive fix" causes downstream errors, I haven't investigated those much yet:

    @@ -1392,12 +1392,12 @@ fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
             Node::Field(field) => icx.to_ty(&field.ty),
    
             Node::Expr(&hir::Expr { kind: hir::ExprKind::Closure(.., gen), .. }) => {
    -            if gen.is_some() {
    -                return tcx.typeck_tables_of(def_id).node_type(hir_id);
    -            }
    -
                 let substs = InternalSubsts::identity_for_item(tcx, def_id);
    -            tcx.mk_closure(def_id, substs)
    +            if let Some(movability) = gen {
    +                tcx.mk_generator(def_id, substs, movability)
    +            } else {
    +                tcx.mk_closure(def_id, substs)
    +            }
             }
    
             Node::AnonConst(_) => {

    I suspect they can be fixed though

  9. Zoxc commented on Jan 8, 2020

    @Zoxc
    Contributor

    @nikomatsakis Generator types contain the types of the values inside which are live across a yield statement, that is why typeck_tables_of is required there. We could move getting the interior type of the generator into a separate query, but that would only help if the problematic caller of type_of does not actually look at these.

  10. nikomatsakis commented on Jan 13, 2020

    @nikomatsakis
    Contributor

    @Zoxc I don't think that's sufficient reason by itself. Closure types also contain details that result from inference (e.g., the types that their upvars use), but type_of applied to a closure just gives you a fully generic type.

  11. tmandry commented on Feb 4, 2020

    @tmandry
    Member

    We hit an instance of this in Fuchsia. Minimal example: (playground)

    mod queue {
        pub trait TryMerge {}
        
        impl TryMerge for () {}
        
        pub fn work_queue<W, C, WF>(work_fn: W)
        where
            W: Fn(C) -> WF,
            C: TryMerge,
        {}
    }
    
    impl queue::TryMerge for () {}
    
    fn main() {
        queue::work_queue(move |_: ()| {
            async {}
        })
    }

    Gives

    error[E0283]: type annotations needed for `()`
    

    instead of the expected error, which is what you get if you remove the async block:

    error[E0119]: conflicting implementations of trait `queue::TryMerge` for type `()`
    

    cc @JakeEhrlich

  12. added 2 commits that reference this issue on Feb 7, 2020
  13. added a commit that references this issue on Mar 20, 2020
  14. added a commit that references this issue on Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

A-async-awaitArea: Async & AwaitA-diagnosticsArea: Messages for errors, warnings, and lintsAsyncAwait-PolishAsync-await issues that are part of the "polish" areaAsyncAwait-TriagedAsync-await issues that have been triaged during a working group meeting.C-enhancementCategory: An issue proposing an enhancement or a PR with one.D-confusingDiagnostics: Confusing error or lint that should be reworked.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions