type_complexity: ignore opaque impl Trait bounds - #17463
Conversation
rustbot
added
S-waiting-on-community-reviews
|
Thanks for the pull request. A reviewer will take a look after it receives 2 community reviews. In the meantime, we would highly appreciate if you could try to review any of PRs waiting on community reviews. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
998635d to
e29598b
Compare
|
Lintcheck changes for f2830c4
This comment will be updated if you push new changes |
| @@ -87,3 +87,27 @@ struct D { | |||
| } | |||
|
|
|||
| fn main() {} | |||
There was a problem hiding this comment.
the "usual" approach that I have seen across the codebase is that there is fn main with the initial impl and then fn issue_xxx with the changes after this.
Can we also make this file follow this pattern or is there a reason why we are not?
|
|
||
| // The presence of an opaque type must not hide complexity in a sibling type that can be factored | ||
| // out. | ||
| #[expect(clippy::type_complexity)] |
There was a problem hiding this comment.
instead of expecting, please use the existing infra with //~^ type_complexity and the error messages
| // Opaque types cannot be extracted into type aliases on stable. Ignore their | ||
| // bounds, but keep scoring surrounding type components that can be extracted. | ||
| if matches!(ty.kind, TyKind::OpaqueDef(..)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Seems a bit blunt of a hammer doesnt it?
A pattern that I have seen is this. Coud you try and make this work this way?
msrv.is_stable(cx, item.def_id)Not sure if this a bad suggestion or if there is better infra around.
| // Opaque `impl Trait` types cannot be extracted into type aliases on stable. Skip | ||
| // their subtree only when `type_alias_impl_trait` is not enabled for this crate. | ||
| let skip_nested_type = matches!(ty.kind, TyKind::OpaqueDef(..)) && !self.type_alias_impl_trait_enabled; |
There was a problem hiding this comment.
Can't you just inline it below?
I think the code would be simpler to read for future readers given the negations of negations going on here.
| // Complexity inside an opaque type cannot be factored into a type alias either. | ||
| fn complex_opaque_bound() -> impl Fn(Vec<Vec<Box<(u32, u32, u32, u32)>>>) { |
There was a problem hiding this comment.
🤔 Why can't it?
as far as I tried this, it defitively can..
Is there maybe a typo in the comment or what do you mean here?
type Foo = Vec<Vec<Box<(u32, u32, u32, u32)>>>;
fn complex_opaque_bound() -> impl Fn(Foo) {
|_|{}
}
| // The presence of an opaque type must not hide complexity in a sibling type that can be factored | ||
| // out. | ||
| fn complex_after_opaque() -> (impl Iterator<Item = u32>, Vec<Vec<Box<(u32, u32, u32, u32)>>>) { | ||
| //~^ ERROR: very complex type used. Consider factoring parts into `type` definitions |
There was a problem hiding this comment.
see the rest of the code
| //~^ ERROR: very complex type used. Consider factoring parts into `type` definitions | |
| //~^ type_complexity |
| #![warn(clippy::type_complexity)] | ||
|
|
||
| fn complex_opaque_bound() -> impl Fn(Vec<Vec<Box<(u32, u32, u32, u32)>>>) { | ||
| //~^ ERROR: very complex type used. Consider factoring parts into `type` definitions |
There was a problem hiding this comment.
| //~^ ERROR: very complex type used. Consider factoring parts into `type` definitions | |
| //~^ type_complexity |
Inline the stable opaque-type skip at the traversal site, remove a misleading stable test case that can be factored through a normal alias, and use the existing UI shorthand lint annotations requested in review.
There was a problem hiding this comment.
Please actually answer my question in #17463 (comment) by commenting!
I did not ask you to remove the testcase, I asked you why this is the case.
A sneaky false-nagative is also not great.
3 participants
changelog: [
type_complexity]: avoid false positives for types containing opaqueimpl Traitboundsfixes #17195
Opaque
impl Traitbounds cannot be extracted into a stable type alias, so counting their internal structure can produce a warning with no usable refactoring. This change stops complexity scoring inside an opaque type while continuing to score surrounding containers and independently nameable sibling types.The UI coverage includes the nested
impl FnMutreproducer, a complex bound wholly inside an opaque return type, and an opaque-first tuple whose separate complex sibling must still trigger the lint.Tested with:
TESTNAME=type_complexity cargo uitest— both matching UI suites passedcargo fmt --all --check