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

Bad diagnostics from RPITIT #101662

Closed
Noratrieb opened this issue Sep 10, 2022 · 5 comments
Closed

Bad diagnostics from RPITIT #101662

Noratrieb opened this issue Sep 10, 2022 · 5 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Noratrieb
Copy link
Member

Given the following code: playground

#![feature(return_position_impl_trait_in_trait)]
#![allow(incomplete_features)]

trait OnlySized<T> {}

impl<T> OnlySized<T> for () {}

trait LetsGo {
    fn uwu() -> impl OnlySized<[u8]>;
}

struct Owo;

impl LetsGo for Owo {
    fn uwu() -> () {}
}


fn main() {}

The current output is:

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
  --> src/main.rs:15:17
   |
15 |     fn uwu() -> () {}
   |                 ^^ doesn't have a size known at compile-time
   |
   = help: the trait `Sized` is not implemented for `[u8]`
   = help: the trait `OnlySized<T>` is implemented for `()`
note: required for `()` to implement `OnlySized<[u8]>`
  --> src/main.rs:6:9
   |
6  | impl<T> OnlySized<T> for () {}
   |         ^^^^^^^^^^^^     ^^
note: required by a bound in `LetsGo::uwu::{opaque#0}`
  --> src/main.rs:9:22
   |
9  |     fn uwu() -> impl OnlySized<[u8]>;
   |                      ^^^^^^^^^^^^^^^ required by this bound in `LetsGo::uwu::{opaque#0}`

The problem here is that (): OnlySized<T> where T: Sized, but [u8]: !Sized, therefore (): !OnlySized<[u8]>

The doesn't have a size known at compile-time pointing at () is quite confusing, since (): Sized.

Also the {opaque#0} name from the desugaring can be improved as well.

@Noratrieb Noratrieb added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 10, 2022
@Noratrieb
Copy link
Member Author

@rustbot label F-return_position_impl_trait_in_trait

@rustbot rustbot added the F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` label Sep 10, 2022
@compiler-errors compiler-errors self-assigned this Sep 10, 2022
@compiler-errors
Copy link
Member

The span is fixed by #101676:

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
 --> /home/gh-compiler-errors/test.rs:9:22
  |
9 |     fn uwu() -> impl OnlySized<[u8]>;
  |                      ^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
  |
  = help: the trait `Sized` is not implemented for `[u8]`
note: required by a bound in `OnlySized`
 --> /home/gh-compiler-errors/test.rs:4:17
  |
4 | trait OnlySized<T> {}
  |                 ^ required by this bound in `OnlySized`
help: consider relaxing the implicit `Sized` restriction
  |
4 | trait OnlySized<T: ?Sized> {}
  |                  ++++++++

But, for example, this code:

#![feature(return_position_impl_trait_in_trait)]

trait Foo {
    fn bar() -> impl std::fmt::Display;
}

impl Foo for () {
    fn bar() -> () {}
}

Still mentions {opaque#0}, see:

error[E0277]: `()` doesn't implement `std::fmt::Display`
 --> /home/gh-compiler-errors/test.rs:8:17
  |
8 |     fn bar() -> () {}
  |                 ^^ `()` cannot be formatted with the default formatter
  |
  = help: the trait `std::fmt::Display` is not implemented for `()`
  = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
note: required by a bound in `Foo::bar::{opaque#0}`
 --> /home/gh-compiler-errors/test.rs:4:22
  |
4 |     fn bar() -> impl std::fmt::Display;
  |                      ^^^^^^^^^^^^^^^^^ required by this bound in `Foo::bar::{opaque#0}

I'm not sure if I know a better way of explaining "required by a bound in an RPITIT in Foo::bar" -- do you have thoughts on what you would've wanted to see here @Nilstrieb, since you commented on that?

@Noratrieb
Copy link
Member Author

Noratrieb commented Sep 12, 2022

Maybe we should just say required by this bound in Foo::bar. If we find a neat and obvious way to tell it that it's the RPITIT specifically it would be great, but I think just pointing at the function would be enough.

@compiler-errors
Copy link
Member

Gonna unassign myself this issue because I don't think it's necessarily an issue, or at least I don't know exactly how to solve it in a way that makes me happy yet. I think "required by this bound in Foo::bar is also ambiguous compared to mentioning ::{opaque#0}", though someone else can take a stab at it and we can see what the diagnostic fallout looks like.

@compiler-errors compiler-errors removed their assignment Oct 5, 2022
@compiler-errors
Copy link
Member

This now says "required by a bound in OnlySized", so I'm considering this fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints F-return_position_impl_trait_in_trait `#![feature(return_position_impl_trait_in_trait)]` 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

3 participants