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

todo! and unimplemented! does not work with impl Trait return types #69882

Open
neckbosov opened this issue Mar 10, 2020 · 14 comments
Open

todo! and unimplemented! does not work with impl Trait return types #69882

neckbosov opened this issue Mar 10, 2020 · 14 comments
Labels
A-diagnostics A-impl-trait C-enhancement T-compiler

Comments

@neckbosov
Copy link

neckbosov commented Mar 10, 2020

This code

trait SomeTrait {
    fn some_func();
}

fn todo_impl_trait() -> impl SomeTrait { todo!() }

does not compile because
the trait SomeTrait is not implemented for ()
But such code

trait SomeTrait {
    fn some_func();
}

fn todo_impl_trait<T: SomeTrait>() -> T { todo!() }

compiles correctly. Can this problem be resolved to use both todo!() and impl Trait?

@jonas-schievink
Copy link
Member

jonas-schievink commented Mar 10, 2020

This is expected behavior since todo!() is not constrained to any type, so it is inferred to (). In the second case it is constrained to T, which is provided by the caller.

@robinmoussu
Copy link

robinmoussu commented Jul 16, 2020

Given that 4 bugs have been opened in such a short period of time for the same reason, I don't think that a wontfix, works as intented is the right approach.

At minimum an alternative should be provided in the help message from the compiler, but I personnaly think that todo!(), unimplemented!() and the like should typecheck to ! (never), that itsel should typecheck to any impl Trait.

@jyn514
Copy link
Member

jyn514 commented Jul 17, 2020

todo!(), unimplemented!() and the like should typecheck to ! (never), that itsel should typecheck to any impl Trait.

// FIXME: Currently the `everybody_loops` transformation is not applied to:
// * `const fn`, due to issue #43636 that `loop` is not supported for const evaluation. We are
// waiting for miri to fix that.
// * `impl Trait`, due to issue #43869 that functions returning impl Trait cannot be diverging.
// Solving this may require `!` to implement every trait, which relies on the an even more
// ambitious form of the closed RFC #1637. See also [#34511].
//
// [#34511]: https://github.com/rust-lang/rust/issues/34511#issuecomment-322340401

@jonas-schievink
Copy link
Member

jonas-schievink commented Jul 23, 2020

Reopening this to have a canonical issue to direct people to. This is still working as expected for the moment though.

@jonas-schievink jonas-schievink added A-impl-trait C-feature-request T-lang labels Jul 23, 2020
@jonas-schievink
Copy link
Member

jonas-schievink commented Jul 23, 2020

Actually, let's classify this as a diagnostics enhancement

@jonas-schievink jonas-schievink added A-diagnostics C-enhancement T-compiler and removed C-feature-request T-lang labels Jul 23, 2020
@jeffs
Copy link

jeffs commented Nov 4, 2020

Is there any suggested work-around for this? It sure seems like the bottom type ought (by definition) to type-check as anything, but as a practical matter, is there any placeholder at all that can be used as a return value from an unimplemented function whose signature demands an impl trait?

@jyn514
Copy link
Member

jyn514 commented Nov 4, 2020

@jeffs in the general case, no, because there could be no type that implements the trait: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=beb24c95bf6be2a8b48f6381c2b7ad6d

@birkenfeld
Copy link
Contributor

birkenfeld commented Oct 3, 2021

@jeffs in the general case, no, because there could be no type that implements the trait: play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=beb24c95bf6be2a8b48f6381c2b7ad6d

I don't get what the example shows; you can add impl Trait for u8 or something, and still get the same error.

@ede1998
Copy link

ede1998 commented Oct 3, 2021

I don't get what the example shows; you can add impl Trait for u8 or something, and still get the same error.

It shows that it's impossible to return a placeholder type because there might not be a type that impls the trait.

@tavianator
Copy link
Contributor

tavianator commented Nov 13, 2021

rustc could make up a type that implements the trait with unreachable!() as the body for every fn.

@ede1998
Copy link

ede1998 commented Nov 13, 2021

No that wouldn't be enough. The trait might also have associated types or constants, which would have to be implemented as well.
Also the trait could be unsafe, so implementing it with unreachable might introduced undefined behavior.

@DCNick3
Copy link

DCNick3 commented Mar 26, 2022

Error message could definitely be improved. Currently this code:

trait Trait {}

fn magic() -> impl Trait {
    panic!()
}

Results in

error[E0277]: the trait bound `(): Trait` is not satisfied
 --> src/lib.rs:3:15  
  |
3 | fn magic() -> impl Trait {
  |               ^^^^^^^^^^ the trait `Trait` is not implemented for `()`

Which is really confusing.

Using Vec<impl Trait> gives a message that is much better (at least saying that it has anything to do with the ! type and the return value):

error[E0720]: cannot resolve opaque type
 --> src/lib.rs:3:19
  |
3 | fn magic() -> Vec<impl Trait> {
  |                   ^^^^^^^^^^ cannot resolve opaque type
4 |     panic!()
  |     -------- this returned value is of `!` type
  |
  = help: this error will resolve once the item's body returns a concrete type

Why is it not shown in the first case?

@oli-obk
Copy link
Contributor

oli-obk commented Mar 26, 2022

I want to fix this. Unfortunately if you had an impl Trait for () {} in your first snippet, that would compile right now and error if my fix would get accepted.

I could try to just fix the diagnostic, but it feels like just piling on hacks (the fact that this works if () implements the trait is already a huge hack imo)

My preferred solution would be to just always error like in the vec case

@oli-obk
Copy link
Contributor

oli-obk commented Mar 26, 2022

No that wouldn't be enough. The trait might also have associated types or constants, which would have to be implemented as well. Also the trait could be unsafe, so implementing it with unreachable might introduced undefined behavior.

A concrete example that would be unsound or at least ICE the compiler is https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ea0e808536f4373dc7be9f002159fa1d

If we just didn't register any hidden type, the make_me call cannot be compiled successfully.

If we used ! fallback, we would get an error because ! does not implement the trait. Of course we could say we generate the trait and panic in all methods, but what happens to associated types and constants? Do we invent random values?

This gets worse when we'll have type alias impl trait, as then you can directly name the opaque type and access associated items directly without jumping through hoops like in my playground.

We could special case todo and unimplemented to just pick some type that implements the trait, but that seems like a real footgun and we should totally lint those cases imo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics A-impl-trait C-enhancement T-compiler
Projects
None yet
Development

No branches or pull requests

10 participants