Repository navigation
GATs: repeating not-locally-required bounds on impls? #91348
Description
See: #87479
So, I believe #89970 broke my code kas-text (nightly rustc + --feature gat).
Let's focus on the iter function. The first thing to realize is that we know that Self: 'a because of &'a self
This isn't true in general, though it may be in specific instances. My code (reduced) exemplifies this perfectly:
pub trait FormattableText: std::fmt::Debug {
type FontTokenIter<'a>: Iterator<Item = FontToken>;
fn font_tokens<'a>(&'a self, dpp: f32, pt_size: f32) -> Self::FontTokenIter<'a>;
}
impl<'t> FormattableText for &'t str {
type FontTokenIter<'a> = std::iter::Empty<FontToken>;
fn font_tokens<'a>(&'a self, _: f32, _: f32) -> Self::FontTokenIter<'a> {
std::iter::empty()
}
}Here, font_tokens may return an iterator over an instance of Self... but it also may not (&str does not have any formatting). A possible workaround in the impl is to write a custom iterator with a reference to the &str anyway, but this seems unnecessary.
Activity
dhardy commentedon Nov 29, 2021
Okay, I'm wrong that a workaround is needed: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=9c22eb9f05bc249d062e794a2f9b49ec
That said, I'm still not convinced the bound should be necessary.
dhardy commentedon Nov 29, 2021
Reading through the issue again, I would like to say a couple of things.
First, the error message for missing bound in the trait is clear about what to do (though it would be helpful if it also mentioned why / referenced extra doc), however the same is not true for implementations: here it's less clear what bound is required (
type FontTokenIter<'a> where Self: 'a = std::iter::Empty<FontToken>;is not entirely obvious syntax; I know this is #90076 but the only relevant point here is telling the user what to use in the error message).Further, putting a
where Self: 'abound on the impl works, butwhere 't: 'adoes not even though apparently this says the same thing.Finally, the point of #87479 IIUC is that
<&'t str as FormattableText>::FontTokenIter<'static>would imply't: 'static. Given awhere Self: 'abound this is true, but I still don't understand why that bound is required (though it is implicit in the usagefn font_tokens<'a>(&'a self, ..) -> Self::FontTokenIter<'a>)... aha, is it to make trait implementations whereSelf: 'ais required legal (since such animplcannot have extra bounds over the trait)?In that case, the chosen solution does not appear "rustic". I would expect, then, that (1) the bound is not required in the trait, but a lint warns that it is likely wanted (allowing opt-out) and (2) that writing an
implwithout this bound fails. (This may be over-complicated, but at least better educates the programmer about what is going on here.)jackh726 commentedon Nov 29, 2021
Okay, I'll try to respond to points individually, since there a lot here.
While it's not true to say that every type for a GAT will need the
Self: 'abound to be well-formed, the idea behind the lint is that the required bounds are those that we know based on how it's returned from the methods in the trait. Specifically, the idea is that with the required bounds, any type assigned to the GAT can be returned from the functions that return it in their signature. Put another way, fortype FontTokenIter<'a> = std::iter::Empty<FontToken>there is no'ain the type itself, but there could be, and the required bounds on the trait permit that.You're completely right here; the error messages are this aren't great (because it's hard). But that should get better in time.
This works for me.
Right, the required bounds basically boil down to the fact that impls can't have extra bounds, so we have to do our best to infer how a GAT will be used to allow the most flexibility in impls.
This was discussed quite a bit on/around #87479. The idea is to eventually make this "implied" and not make the user write these bounds at all. But in order for this to not be a breaking change, we have to require them now. If we've done the lint correctly and have the correct assumptions, then there shouldn't be any code that won't work with the required bounds but would otherwise (and vice versa) - but we can't necessarily guarantee that, so we can only be potentially more restrictive than for now.
Another alternative is to just do nothing - or "only" warn. This isn't ideal because errors around these errors can be difficult to diagnose and fix. And, moreso, if a library publishes a trait without these bounds, downstream crates can't do anything about it and it can severely limit impls.
Please let me know if I've missed anything here. Ultimately, there are tradeoffs here and I want to hear about how the outlives lint impacts different use cases.
dhardy commentedon Nov 30, 2021
No, that's a great response, thanks.
So somehow I had trouble putting the bounds on the impl. Given better error messages this wouldn't have been an issue.
The point is simply that it was not initially clear the bounds on the trait are required to cover all possible implementations:
Are the situations for GATs and trait method parameters not analogous? Why then must unused bounds be specified on the impls of GATs but not trait method parameters?
[-]GATs: `where Self: 'a` bound breaks usage with possible-iterators[/-][+]GATs: repeating not-locally-required bounds on impls?[/+]Rollup merge of rust-lang#91849 - jackh726:gats-outlives-lint-part2, …
Rollup merge of rust-lang#91849 - jackh726:gats-outlives-lint-part2, …