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

Corrected suggestion for generic parameters in function_item_references lint #78659

Merged
merged 1 commit into from
Nov 3, 2020

Conversation

ayrtonm
Copy link
Contributor

@ayrtonm ayrtonm commented Nov 2, 2020

This commit handles functions with generic type parameters like you pointed out as well as const generics. Also this is probably a minor thing, but the type alias you used in the example doesn't show up so the suggestion right now would be size_of::<[u8; 16]> as fn() ->. This is because the lint checker works with MIR instead of HIR. I don't think we can get the alias at that point, but let me know if I'm wrong and there's a way to fix this. Also I put you as the reviewer, but I'm not sure if you want to review it or if it makes more sense to ask one of the original reviewers of this lint.
closes #78571

…ces` lint

This lint was incorrectly suggesting casting a function to a pointer without
specifying generic type parameters or const generics. This would cause a
compiler error since the missing parameters couldn't be inferred. This commit
fixed the suggestion and added a few tests with generics.
@JohnTitor
Copy link
Member

r? @oli-obk, following GitHub suggestions.

@JohnTitor JohnTitor added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Nov 2, 2020

Another alternative is to not rebuild the name at all, but to grab the original snippet from the code being linted and just plainly reuse it here. You can use span_snippet for that, if it's possible to build a span for that part, which I'm not sure about.

If that is not possible, I think it's fine to merge the current version, it covers the usually occurring situations.

@ayrtonm
Copy link
Contributor Author

ayrtonm commented Nov 3, 2020

You can use span_snippet for that, if it's possible to build a span for that part, which I'm not sure about.

Thanks for pointing that out. I don't think it'll work though since the span we pass to emit_lint contains the & that causes the warning. Even if we ignore rare cases where the span is a superset of the part that should be replaced, I'm not sure how feasible it is to walk the MIR to get the correct span without relying on implementation details of the formatting macros.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 3, 2020

That makes sense, thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Nov 3, 2020

📌 Commit ace02c4 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 3, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 3, 2020
Corrected suggestion for generic parameters in `function_item_references` lint

This commit handles functions with generic type parameters like you pointed out as well as const generics. Also this is probably a minor thing, but the type alias you used in the example doesn't show up so the suggestion right now would be `size_of::<[u8; 16]> as fn() ->`. This is because the lint checker works with MIR instead of HIR. I don't think we can get the alias at that point, but let me know if I'm wrong and there's a way to fix this. Also I put you as the reviewer, but I'm not sure if you want to review it or if it makes more sense to ask one of the original reviewers of this lint.
closes rust-lang#78571
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 3, 2020
Rollup of 7 pull requests

Successful merges:

 - rust-lang#77950 (Add support for SHA256 source file hashing)
 - rust-lang#78624 (Sync rustc_codegen_cranelift)
 - rust-lang#78626 (Improve errors about #[deprecated] attribute)
 - rust-lang#78659 (Corrected suggestion for generic parameters in `function_item_references` lint)
 - rust-lang#78687 (Suggest library/std when running all stage 0 tests)
 - rust-lang#78699 (Show more error information in lldb_batchmode)
 - rust-lang#78709 (Fix panic in bootstrap for non-workspace path dependencies.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f347dab into rust-lang:master Nov 3, 2020
@rustbot rustbot added this to the 1.49.0 milestone Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect suggestion of function_item_references lint
5 participants