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

False positive in new_ret_no_self if self type carries lifetime annotation? #734

Closed
llogiq opened this issue Mar 2, 2016 · 7 comments
Closed
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@llogiq
Copy link
Contributor

llogiq commented Mar 2, 2016

Example (from lib_json):

impl<'a> CharIter<'a> {
    pub fn new<'b>( s: &'b str ) -> CharIter<'b> { .. }
    ..
}

While the warning can be fixed by changing it to pub fn new(s: &'a str) -> CharIter<'a> { .. }, I am unsure if the lifetime difference is really relevant here.

@mcarton mcarton added the C-bug Category: Clippy is not doing the correct thing label Mar 2, 2016
@mcarton
Copy link
Member

mcarton commented Mar 2, 2016

I should probably not compare types with ==. I’ll look into that tomorrow.

@mcarton
Copy link
Member

mcarton commented Mar 3, 2016

If I use ctxt::erase_regions and InferCtx::can_equate instead of simply ==, it fixes lifetime problems. But I still have a problem with:

impl<T> CharIter<T> {
    pub fn new<U>( s: &str ) -> CharIter<U> { .. }
    ..
}

Any idea how to compare types with type parameters? 🆘

@llogiq
Copy link
Contributor Author

llogiq commented Mar 4, 2016

This is a bit complex, because the types can literally come from anywhere. But in general, we should consider two unbound type variables in the same position equal, regardless there name, so

impl<T> CharIter<T> {
    pub fn new<U>(..) -> CharIter<U> { .. }
    // should be considered equal to
    pub fn new2(..) -> CharIter<T> { .. }
}

Both T and U are introduced as unbound type variables, one by the inherent impl, the other by the function.

@mcarton
Copy link
Member

mcarton commented Mar 4, 2016

But in general, we should consider two unbound type variables in the same position equal, regardless there name

I know, but any idea what function does that in rustc?

@llogiq
Copy link
Contributor Author

llogiq commented Mar 4, 2016

I don't think there's a function for this in rustc (because for rustc the types are not equal at all). We'll have to walk the type's TyParams and compare their bounds. If the bounds are equal (either unbounded or bounded by some trait)), we should deem the types equal.

@phansch
Copy link
Member

phansch commented Apr 25, 2020

triage:

The first example is fixed (I'm going to add a testcase for that).
The second example still triggers, while it shouldn't:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=6a4da2c0118b2f6dfac4456041c2b332

flip1995 added a commit to flip1995/rust-clippy that referenced this issue Apr 25, 2020
…, r=flip1995

Add lifetime test case for `new_ret_no_self`

cc rust-lang#734 (comment)

changelog: none
flip1995 added a commit to flip1995/rust-clippy that referenced this issue Apr 25, 2020
…, r=flip1995

Add lifetime test case for `new_ret_no_self`

cc rust-lang#734 (comment)

changelog: none
@phansch phansch added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Dec 19, 2020
@Jarcho
Copy link
Contributor

Jarcho commented Mar 27, 2021

This was fixed in #6952

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-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

5 participants