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

Clippy crashes on simple method returning impl PartialOrd #10041

Closed
djkoloski opened this issue Dec 6, 2022 · 3 comments · Fixed by #10086
Closed

Clippy crashes on simple method returning impl PartialOrd #10041

djkoloski opened this issue Dec 6, 2022 · 3 comments · Fixed by #10086
Labels
C-bug Category: Clippy is not doing the correct thing I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️

Comments

@djkoloski
Copy link

djkoloski commented Dec 6, 2022

Summary

This is probably caused by #9733 per a crash analysis:

...
#30580 0x0000555c0ff6ebc0 in clippy_utils::ty::contains_ty_adt_constructor_opaque ()
#30581 0x0000555c0ff6ebc0 in clippy_utils::ty::contains_ty_adt_constructor_opaque ()
#30582 0x0000555c0ff6ebc0 in clippy_utils::ty::contains_ty_adt_constructor_opaque ()
#30583 0x0000555c0ff6ebc0 in clippy_utils::ty::contains_ty_adt_constructor_opaque ()
#30584 0x0000555c0fcbb240 in <clippy_lints::methods::Methods as rustc_lint::passes::LateLintPass>::check_impl_item ()
#30585 0x00007fe4894426dd in rustc_hir::intravisit::walk_item::<rustc_lint::late::LateContextAndPass<rustc_lint::late::LateLintPassObjects>> ()
...

Reproducer

I tried this code:

pub struct Bomb;

impl Bomb {
    pub fn explode(&self) -> impl PartialOrd {
        0i32
    }
}

I expected to see this happen:

Clippy does not crash.

Instead, this happened:

Clippy crashes with a stack overflow exception.

Version

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: x86_64-unknown-linux-gnu
release: 1.65.0
LLVM version: 15.0.0

Additional Labels

No response

@djkoloski djkoloski added the C-bug Category: Clippy is not doing the correct thing label Dec 6, 2022
@djkoloski djkoloski changed the title Clippy crashes on simple method returning impl Trait Clippy crashes on simple method returning impl PartialOrd Dec 6, 2022
@djkoloski
Copy link
Author

We believe that this is because PartialOrd has a generic parameter RHS = Self which causes this check to recurse on Self, which is once again PartialOrd<RHS = Self>.

@xFrednet xFrednet added the I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️ label Dec 6, 2022
@matthiaskrgr matthiaskrgr added the I-nominated Issue: Nominated to be discussed at the next Clippy meeting label Dec 10, 2022
@matthiaskrgr
Copy link
Member

Nominating for the next meeting; is this something we want to fix and beta backport? As far as I understand this will make it into the imminent master -> beta promition.

Nashenas88 added a commit to Nashenas88/rust-clippy that referenced this issue Dec 12, 2022
Fixes rust-lang#10041.

Prevent recusion into Self when it's a generic parameter.
Added regression test from example in rust-lang#10041.
Nashenas88 added a commit to Nashenas88/rust-clippy that referenced this issue Dec 12, 2022
@nbdd0121
Copy link
Contributor

Sorry for the trouble, just saw this today (as I wasn't cc'ed in this issue).

Yes your analysis is correct, this is caused by recursive Self which is again the same opaque type.

matthiaskrgr pushed a commit to matthiaskrgr/rust-clippy that referenced this issue Dec 15, 2022
Fixes rust-lang#10041.

Prevent recusion into Self when it's a generic parameter.
Added regression test from example in rust-lang#10041.
matthiaskrgr pushed a commit to matthiaskrgr/rust-clippy that referenced this issue Dec 15, 2022
See rust-lang#10068 (comment) for details

-- original PR description of rust-lang#10068 --

fixes rust-lang#10041

Prevent recursion into Self when it's a generic parameter. Added regression test from example in rust-lang#10041.

- \[x] Followed [lint naming conventions][lint_naming] - **N/A**
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints` - **N/A**
- \[x] Added lint documentation - **N/A**
- \[x] Run `cargo dev fmt`

changelog: [`new-ret-no-self`] Fix segmentation fault caused when generic parameter defaults to `Self` and is unspecified. For example, `fn uh_oh(&self) -> impl PartialOrd { ... }` has a hidden `Rhs=Self` as the generic parameter for `PartialOrd`.
bors added a commit that referenced this issue Dec 15, 2022
Stack overflow fix

See #10068 (comment) for details

-- original PR description of #10068 --

fixes #10041

Prevent recursion into Self when it's a generic parameter. Added regression test from example in #10041.

- \[x] Followed [lint naming conventions][lint_naming] - **N/A**
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints` - **N/A**
- \[x] Added lint documentation - **N/A**
- \[x] Run `cargo dev fmt`

changelog: [`new-ret-no-self`] Fix segmentation fault caused when generic parameter defaults to `Self` and is unspecified. For example, `fn uh_oh(&self)>
matthiaskrgr pushed a commit to matthiaskrgr/rust-clippy that referenced this issue Dec 15, 2022
See rust-lang#10068 (comment) for details

-- original PR description of rust-lang#10068 --

fixes rust-lang#10041

Prevent recursion into Self when it's a generic parameter. Added regression test from example in rust-lang#10041.

- \[x] Followed [lint naming conventions][lint_naming] - **N/A**
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints` - **N/A**
- \[x] Added lint documentation - **N/A**
- \[x] Run `cargo dev fmt`

changelog: [`new-ret-no-self`] Fix segmentation fault caused when generic parameter defaults to `Self` and is unspecified. For example, `fn uh_oh(&self) -> impl PartialOrd { ... }` has a hidden `Rhs=Self` as the generic parameter for `PartialOrd`.
bors added a commit that referenced this issue Dec 15, 2022
Stack overflow fix

See #10068 (comment) for details

-- original PR description of #10068 --

fixes #10041

Prevent recursion into Self when it's a generic parameter. Added regression test from example in #10041.

- \[x] Followed [lint naming conventions][lint_naming] - **N/A**
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints` - **N/A**
- \[x] Added lint documentation - **N/A**
- \[x] Run `cargo dev fmt`

changelog: [`new-ret-no-self`] Fix segmentation fault caused when generic parameter defaults to `Self` and is unspecified. For example, `fn uh_oh(&self)>
@bors bors closed this as completed in 02f3959 Dec 16, 2022
@Alexendoo Alexendoo removed the I-nominated Issue: Nominated to be discussed at the next Clippy meeting label Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-ICE Issue: Clippy panicked, giving an Internal Compilation Error (ICE) ❄️
Projects
None yet
5 participants