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

Better diagnostics when where clauses need on associated type in impl #92033

Closed
chubei-oppen opened this issue Dec 17, 2021 · 7 comments · Fixed by #92683
Closed

Better diagnostics when where clauses need on associated type in impl #92033

chubei-oppen opened this issue Dec 17, 2021 · 7 comments · Fixed by #92683
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-GATs Area: Generic associated types (GATs) C-bug Category: This is a bug. F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs GATs-triaged Issues using the `generic_associated_types` feature that have been triaged

Comments

@chubei-oppen
Copy link

As suggested in #87479, I'm reporting a case where the required bound breaks code.

I was writing a 3D renderer and modeling the surface and swapchain concepts using traits.

A Surface is something you can render to, it should produce a iterator of references to Textures:

struct Texture;

trait Surface {
    type TextureIter<'a>: Iterator<Item = &'a Texture>;

    fn get_texture(&self) -> Self::TextureIter<'_>;
}

A Swapchain produces a reference to a Surface:

trait Swapchain {
    type Surface<'a>: Surface;

    fn get_surface(&self) -> Self::Surface<'_>;
}

Now a Texture itself can be used as a swapchain, producing reference to itself as the Surface.

impl<'s> Surface for &'s Texture {
    type TextureIter<'a> = std::option::IntoIter<&'a Texture>;

    fn get_texture(&self) -> Self::TextureIter<'_> {
        let option: Option<&Texture> = Some(self);
        option.into_iter()
    }
}

impl Swapchain for Texture {
    type Surface<'a> = &'a Texture;

    fn get_surface(&self) -> Self::Surface<'_> {
        self
    }
}

Above code compiles under rustc 1.58.0-nightly (0d1754e8b 2021-11-05).

Now it doesn't with added where Self: 'a bound.

This is a simplified example as Surface and Swapchain have other required methods.

Meta

rustc --version --verbose:

rustc 1.59.0-nightly (efec54529 2021-12-04
Backtrace

<backtrace>
error[E0477]: the type `&'s Texture` does not fulfill the required lifetime
  --> src/main.rs:18:5
   |
18 |     type TextureIter<'a> = std::option::IntoIter<&'a Texture>;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: type must outlive the lifetime `'a` as defined here
  --> src/main.rs:18:22
   |
18 |     type TextureIter<'a> = std::option::IntoIter<&'a Texture>;
   |                      ^^

For more information about this error, try `rustc --explain E0477`.
error: could not compile `rsplayground` due to previous error

@chubei-oppen chubei-oppen added the C-bug Category: This is a bug. label Dec 17, 2021
@jackh726
Copy link
Member

I think this can be solved by just added the where Self: 'a bounds to the impl too...

@jackh726 jackh726 added the F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs label Dec 18, 2021
@jackh726
Copy link
Member

That being said, I feel like this should be fine, so there might be something more subtle going on.

@chubei-oppen
Copy link
Author

I think this can be solved by just added the where Self: 'a bounds to the impl too...

How to add where Self: 'a to the impl? Is it after impl<'s> Surface for &'s Texture or type TextureIter<'a> = std::option::IntoIter<&'a Texture>?

@jackh726
Copy link
Member

This works (with the where clause on the impl): https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=cc1325c7ca369536e63c9e436d6eacf0

Were you trying to put the where clause on the impl at the end of the line? Like type TextureIter<'a> = std::option::IntoIter<&'a Texture> where Self: 'a?

@chubei-oppen
Copy link
Author

Yes... Should have thought of the correct place. It works in real code too. Closing the issue. Thanks a lot!

@jackh726
Copy link
Member

I'm actually going to leave this open for two diagnostics improvements:

  1. Proper suggestion to move the where clause if it's placed in the wrong place on the GAT (Change location of where clause on GATs #90076 changes the location, but I'm about to open a PR that doesn't change the location but parses/errors the invalid location)
  2. Maybe try to suggest Self: 'a on the impl GAT in some way. Either through suggest copying the trait bounds, or recognizing that &'s Texture is the self type.

@jackh726
Copy link
Member

jackh726 commented Feb 7, 2022

GATs issue triage: not blocking. At this point, this is just a diagnostics improvement. 1 from above is fixed; just want to do 2.

@jackh726 jackh726 added A-diagnostics Area: Messages for errors, warnings, and lints GATs-triaged Issues using the `generic_associated_types` feature that have been triaged labels Feb 7, 2022
@jackh726 jackh726 changed the title GAT defaults to where Self: 'a code breaking report Better diagnostics when where clauses need on associated type in impl Feb 7, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 17, 2022
Suggest copying trait associated type bounds on lifetime error

Closes rust-lang#92033

Kind of the most simple suggestion to make - we don't try to be fancy. Turns out, it's still pretty useful (the couple existing tests that trigger this error end up fixed - for this error - upon applying the fix).

r? `@estebank`
cc `@nikomatsakis`
@bors bors closed this as completed in dd11126 Feb 18, 2022
@fmease fmease added the A-GATs Area: Generic associated types (GATs) label Nov 2, 2024
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 A-GATs Area: Generic associated types (GATs) C-bug Category: This is a bug. F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs GATs-triaged Issues using the `generic_associated_types` feature that have been triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants