Skip to content

[beta] backport clippy stack overflow fix #106326

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 31, 2022
+105 −50
Merged

[beta] backport clippy stack overflow fix #106326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 53 additions & 37 deletions src/tools/clippy/clippy_utils/src/ty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,50 +69,66 @@ pub fn contains_adt_constructor<'tcx>(ty: Ty<'tcx>, adt: AdtDef<'tcx>) -> bool {
/// This method also recurses into opaque type predicates, so call it with `impl Trait<U>` and `U`
/// will also return `true`.
pub fn contains_ty_adt_constructor_opaque<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, needle: Ty<'tcx>) -> bool {
ty.walk().any(|inner| match inner.unpack() {
GenericArgKind::Type(inner_ty) => {
if inner_ty == needle {
return true;
}
fn contains_ty_adt_constructor_opaque_inner<'tcx>(
cx: &LateContext<'tcx>,
ty: Ty<'tcx>,
needle: Ty<'tcx>,
seen: &mut FxHashSet<DefId>,
) -> bool {
ty.walk().any(|inner| match inner.unpack() {
GenericArgKind::Type(inner_ty) => {
if inner_ty == needle {
return true;
}

if inner_ty.ty_adt_def() == needle.ty_adt_def() {
return true;
}
if inner_ty.ty_adt_def() == needle.ty_adt_def() {
return true;
}

if let ty::Opaque(def_id, _) = *inner_ty.kind() {
if !seen.insert(def_id) {
return false;
}

if let ty::Opaque(def_id, _) = *inner_ty.kind() {
for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) {
match predicate.kind().skip_binder() {
// For `impl Trait<U>`, it will register a predicate of `T: Trait<U>`, so we go through
// and check substituions to find `U`.
ty::PredicateKind::Clause(ty::Clause::Trait(trait_predicate)) => {
if trait_predicate
.trait_ref
.substs
.types()
.skip(1) // Skip the implicit `Self` generic parameter
.any(|ty| contains_ty_adt_constructor_opaque(cx, ty, needle))
{
return true;
}
},
// For `impl Trait<Assoc=U>`, it will register a predicate of `<T as Trait>::Assoc = U`,
// so we check the term for `U`.
ty::PredicateKind::Clause(ty::Clause::Projection(projection_predicate)) => {
if let ty::TermKind::Ty(ty) = projection_predicate.term.unpack() {
if contains_ty_adt_constructor_opaque(cx, ty, needle) {
for &(predicate, _span) in cx.tcx.explicit_item_bounds(def_id) {
match predicate.kind().skip_binder() {
// For `impl Trait<U>`, it will register a predicate of `T: Trait<U>`, so we go through
// and check substituions to find `U`.
ty::PredicateKind::Clause(ty::Clause::Trait(trait_predicate)) => {
if trait_predicate
.trait_ref
.substs
.types()
.skip(1) // Skip the implicit `Self` generic parameter
.any(|ty| contains_ty_adt_constructor_opaque_inner(cx, ty, needle, seen))
{
return true;
}
};
},
_ => (),
},
// For `impl Trait<Assoc=U>`, it will register a predicate of `<T as Trait>::Assoc = U`,
// so we check the term for `U`.
ty::PredicateKind::Clause(ty::Clause::Projection(projection_predicate)) => {
if let ty::TermKind::Ty(ty) = projection_predicate.term.unpack() {
if contains_ty_adt_constructor_opaque_inner(cx, ty, needle, seen) {
return true;
}
};
},
_ => (),
}
}
}
}

false
},
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
})
false
},
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
})
}

// A hash set to ensure that the same opaque type (`impl Trait` in RPIT or TAIT) is not
// visited twice.
let mut seen = FxHashSet::default();
contains_ty_adt_constructor_opaque_inner(cx, ty, needle, &mut seen)
}

/// Resolves `<T as Iterator>::Item` for `T`
Expand Down
23 changes: 23 additions & 0 deletions src/tools/clippy/tests/ui/new_ret_no_self.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![feature(type_alias_impl_trait)]
#![warn(clippy::new_ret_no_self)]
#![allow(dead_code)]

Expand Down Expand Up @@ -400,3 +401,25 @@ mod issue7344 {
}
}
}

mod issue10041 {
struct Bomb;

impl Bomb {
// Hidden <Rhs = Self> default generic paramter.
pub fn new() -> impl PartialOrd {
0i32
}
}

// TAIT with self-referencing bounds
type X = impl std::ops::Add<Output = X>;

struct Bomb2;

impl Bomb2 {
pub fn new() -> X {
0i32
}
}
}
42 changes: 29 additions & 13 deletions src/tools/clippy/tests/ui/new_ret_no_self.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:49:5
--> $DIR/new_ret_no_self.rs:50:5
|
LL | / pub fn new(_: String) -> impl R<Item = u32> {
LL | | S3
Expand All @@ -9,88 +9,104 @@ LL | | }
= note: `-D clippy::new-ret-no-self` implied by `-D warnings`

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:81:5
--> $DIR/new_ret_no_self.rs:82:5
|
LL | / pub fn new() -> u32 {
LL | | unimplemented!();
LL | | }
| |_____^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:90:5
--> $DIR/new_ret_no_self.rs:91:5
|
LL | / pub fn new(_: String) -> u32 {
LL | | unimplemented!();
LL | | }
| |_____^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:126:5
--> $DIR/new_ret_no_self.rs:127:5
|
LL | / pub fn new() -> (u32, u32) {
LL | | unimplemented!();
LL | | }
| |_____^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:153:5
--> $DIR/new_ret_no_self.rs:154:5
|
LL | / pub fn new() -> *mut V {
LL | | unimplemented!();
LL | | }
| |_____^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:171:5
--> $DIR/new_ret_no_self.rs:172:5
|
LL | / pub fn new() -> Option<u32> {
LL | | unimplemented!();
LL | | }
| |_____^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:224:9
--> $DIR/new_ret_no_self.rs:225:9
|
LL | fn new() -> String;
| ^^^^^^^^^^^^^^^^^^^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:236:9
--> $DIR/new_ret_no_self.rs:237:9
|
LL | fn new(_: String) -> String;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:271:9
--> $DIR/new_ret_no_self.rs:272:9
|
LL | / fn new() -> (u32, u32) {
LL | | unimplemented!();
LL | | }
| |_________^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:298:9
--> $DIR/new_ret_no_self.rs:299:9
|
LL | / fn new() -> *mut V {
LL | | unimplemented!();
LL | | }
| |_________^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:368:9
--> $DIR/new_ret_no_self.rs:369:9
|
LL | / fn new(t: T) -> impl Into<i32> {
LL | | 1
LL | | }
| |_________^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:389:9
--> $DIR/new_ret_no_self.rs:390:9
|
LL | / fn new(t: T) -> impl Trait2<(), i32> {
LL | | unimplemented!()
LL | | }
| |_________^

error: aborting due to 12 previous errors
error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:410:9
|
LL | / pub fn new() -> impl PartialOrd {
LL | | 0i32
LL | | }
| |_________^

error: methods called `new` usually return `Self`
--> $DIR/new_ret_no_self.rs:421:9
|
LL | / pub fn new() -> X {
LL | | 0i32
LL | | }
| |_________^

error: aborting due to 14 previous errors