Skip to content

Conversation

@c410-f3r
Copy link
Contributor

@c410-f3r c410-f3r commented Jul 7, 2022

Fixes #8903

r? @llogiq

changelog: Add Arithmetic lint

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 7, 2022
@c410-f3r c410-f3r force-pushed the arith branch 2 times, most recently from f74764c to 097d049 Compare July 7, 2022 01:24
@c410-f3r c410-f3r force-pushed the arith branch 3 times, most recently from 6e322f8 to f2f447c Compare July 7, 2022 13:48
@Jarcho
Copy link
Contributor

Jarcho commented Jul 11, 2022

Just a general comment about the approach here. This would be better done using resolved items rather than paths. LateContext::qpath_res will resolve any QPath if you need it. Generally you should be able to use cx.typeck_results().expr_ty(_) to get the type of an Expr. You can also look at the disallowed_types lint for how it deals with configuring which types to block.

@c410-f3r
Copy link
Contributor Author

Just a general comment about the approach here. This would be better done using resolved items rather than paths. LateContext::qpath_res will resolve any QPath if you need it. Generally you should be able to use cx.typeck_results().expr_ty(_) to get the type of an Expr. You can also look at the disallowed_types lint for how it deals with configuring which types to block.

Exactly what I was looking for! Thanks!

@bors
Copy link
Contributor

bors commented Jul 18, 2022

☔ The latest upstream changes (presumably #9199) made this pull request unmergeable. Please resolve the merge conflicts.

@c410-f3r
Copy link
Contributor Author

I think the code is now in a good shape for review.

@c410-f3r
Copy link
Contributor Author

@llogiq Perhaps another reviewer?

@llogiq
Copy link
Contributor

llogiq commented Jul 25, 2022

No, I just had private stuff keeping me from reviewing. Sorry for the delay. I agree, the code is in good shape.

@bors r+

@bors
Copy link
Contributor

bors commented Jul 25, 2022

📌 Commit 31e5465 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 25, 2022

⌛ Testing commit 31e5465 with merge 8882578...

@c410-f3r
Copy link
Contributor Author

No, I just had private stuff keeping me from reviewing. Sorry for the delay. I agree, the code is in good shape.

@bors r+

Thank you @llogiq

@bors
Copy link
Contributor

bors commented Jul 25, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 8882578 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Arithmetic lint

5 participants