Skip to content

Conversation

@sanxiyn
Copy link
Contributor

@sanxiyn sanxiyn commented Feb 11, 2014

No description provided.

@sanxiyn
Copy link
Contributor Author

sanxiyn commented Feb 11, 2014

Before:

span.rs:6         self.a
                  ^~~~~
span.rs:10     let s = S { a: 0 };
                       ^

After:

span.rs:6         self.a
                  ^~~~
span.rs:10     let s = S { a: 0 };
                       ^~~~~~~~~~

@bors bors closed this Feb 11, 2014
@bors bors merged commit f3b5ec2 into rust-lang:master Feb 11, 2014
@sanxiyn sanxiyn deleted the accurate-span-4 branch February 12, 2014 00:40
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 8, 2024
[`unconditional_recursion`]: compare by `Ty`s instead of `DefId`s

Fixes rust-lang#12154
Fixes rust-lang#12181 (this was later edited in, so the rest of the description refers to the first linked issue)

Before this change, the lint would work with `DefId`s and use those to compare types. This PR changes it to compare types directly. It fixes the linked issue, but also other false positives I found in a lintcheck run. For example, one of the issues is that some types don't have `DefId`s (primitives, references, etc., leading to possible FNs), and the helper function used to extract a `DefId` didn't handle type parameters.

Another issue was that the lint would use `.peel_refs()` in a few places where that could lead to false positives (one such FP was in the `http` crate). See the doc comment on one of the added functions and also the test case for what I mean.

The code in the linked issue was linted because the receiver type is `T` (a `ty::Param`), which was not handled in `get_ty_def_id` and returned `None`, so this wouldn't actually *get* to comparing `self_arg != ty_id` here, and skip the early-return:
https://github.com/rust-lang/rust-clippy/blob/70573af31eb9b8431c2e7923325c82ba0304cbb2/clippy_lints/src/unconditional_recursion.rs#L171-L178

This alone could be fixed by doing something like `&& get_ty_def_id(ty).map_or(true, |ty_id)| self_arg != ty_id)`, but we don't really need to work with `DefId`s in the first place, I don't think.

changelog: [`unconditional_recursion`]: avoid linting when the other comparison type is a type parameter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants