Ignore associated types in traits when considering type complexity #8030
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
tests/ui/type_complexity.stderr
Outdated
| --> $DIR/type_complexity.rs:34:14 | ||
| | | ||
| LL | const A: Vec<Vec<Box<(u32, u32, u32, u32)>>> = vec![]; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| error: very complex type used. Consider factoring parts into `type` definitions | ||
| --> $DIR/type_complexity.rs:38:25 | ||
| | | ||
| LL | fn method(&self, p: Vec<Vec<Box<(u32, u32, u32, u32)>>>) {} | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
There was a problem hiding this comment.
Should these warnings also not be produced? These types are set by the trait definition (and warnings are produced for them there), and they can use local generics/assoc types which makes them hard to simplify.
There was a problem hiding this comment.
So you suggest allowing the lint on trait impls? Yes, that likely makes sense. If the trait is local to the crate, it will already get a warning, so the additional warnings don't have much value.
There was a problem hiding this comment.
Yes not linting in trait implementations makes sense here, I think. There's a small exception though:
trait T {
type A;
fn f(a: Self::A); // type of `a` defined with the assoc type
}
impl T for _ {
type A = Really<Complex<Type<Thingy>>>; // should *not* get linted as assoc type def
fn f(a: Really<Complex<Type<Thingy>>>) { ... } // should get linted, since Self::A could be used like in the trait definition
}So if a complex type is used in the impl, where the trait is using an assoc type (or more general, a type that wouldn't get linted), then this lint should trigger in the trait impl and suggest to use the type that is used in the trait def.
But I know from the use_self lint that this is hard to detect and even harder to produce a suggestion for. So nothing that would've to be done in this PR, but maybe worth an issue, if we stop linting on trait impls.
|
Any updates on this? Should I have a look now or are you still trying to offer suggestions for associated types? |
|
Ah, sorry, I forget that I had this PR 😅 I'll remove warnings for trait impls leaving a |
|
I'm not sure if I did everything right, but it seems to work :D This is ready for review. |
|
Thank you! @bors r+ |
|
📌 Commit c176568 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: Ignore associated types in traits when checking
[`type_complexity`]lint.fixes #1013