[WIP] Add new lint for xor-ing when `pow` was probably intended #6182

cgm616 · 2020-10-16T15:55:23Z

This is simply a resurrection of #4541, which is inactive. It looks like everything is good to go, pending review.

Closes #4205.

changelog: new lint: [xor_used_as_pow]

rust-highfive · 2020-10-16T15:55:26Z

(rust_highfive has picked a reviewer for you, use r? to override)

cgm616 · 2020-10-16T16:36:00Z

The pr should be totally updated, now---everything compiles in the lint itself, but I still can't compile clippy because of master branch problems.

ebroto · 2020-10-16T20:45:17Z

Because of compilation problems with the master branch of clippy, I haven't been able to test this

@cgm616 you can try rebasing on master, the problem should be fixed now (#6184)

cgm616 · 2020-10-16T23:32:46Z

Looks to be all set now!

clippy_lints/src/xor_used_as_pow.rs

src/lintlist/mod.rs

clippy_lints/src/xor_used_as_pow.rs

tests/ui/xor_used_as_pow.rs

clippy_lints/src/xor_used_as_pow.rs

cgm616 · 2020-10-22T14:06:25Z

I'm unsure how to fix the failing test.... it seems like it has something to do with clippy not fixing all of the errors in the uitest, so the .fixed file still doesn't compile without errors/warnings.

tests/ui/xor_used_as_pow.rs

bors · 2020-10-25T09:57:04Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

Minor docs and description changes Co-authored-by: Eduardo Broto <ebroto@tutanota.com>

cgm616 · 2020-10-25T13:35:46Z

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

ebroto

As a side note, I realized we could improve the lint further, but I think it would be better to leave these improvements for follow-up PRs. I will open the issues after we merge this PR.

We could give MachineApplicable suggestions for the .pow() case, but would have to handle potential overflows.
We could target EXPR ^ EXPR where any of the sides is const-foldable, not only literals. This may require enhancing the consts module.

ebroto · 2020-10-25T14:53:24Z

clippy_lints/src/xor_used_as_pow.rs

+    }
+}
+
+fn unwrap_dec_int_literal(cx: &LateContext<'_>, lit: &Lit) -> Option<(u128, LitIntType)> {


It seems that we are not using the second element of the returned tuple

ebroto · 2020-10-25T14:54:30Z

tests/ui/xor_used_as_pow_fixable.rs

+enum E {
+    First = 1 ^ 8,
+    Second = 2 ^ 8,
+    Third = 3 ^ 8,
+    Tenth = 10 ^ 8,
+}
+
+fn main() {
+    // These should succeed:
+    let _ = 9 ^ 3; // lhs other than 2 or 10
+    let _ = 0x02 ^ 6; // lhs not decimal
+    let _ = 2 ^ 0x10; // rhs hexadecimal
+    let _ = 10 ^ 0b0101; // rhs binary
+    let _ = 2 ^ 0o1; // rhs octal
+    let _ = 10 ^ -18; // negative rhs
+    let _ = 2 ^ 0; // zero rhs


We can remove the cases that are already tested in the non-fixable file.

ebroto · 2020-10-25T14:56:36Z

tests/ui/xor_used_as_pow.rs

+    {
+        let x = 15;
+        let _ = 10 ^ x;
+    }


We're missing two tests:

The RHS is a const

The RHS is an immutable binding initialized with a const

ebroto · 2020-10-25T14:58:44Z

clippy_lints/src/xor_used_as_pow.rs

+    }
+}
+
+fn report_with_ident(cx: &LateContext<'_>, lhs: u128, rhs: &QPath<'_>, span: Span) {


In these cases we're not handling the overflow.

let x = 42; let _ = 2 ^ x;

The suggestion would result in code that does not compile.

I was thinking about this while writing the relevant portions. In order to figure out if this overflows, I'll need to get the literal from the definition (or const or const-binding). Is there a utility for this in clippy already? Particularly, someone might have something like this:

const A: u32 = 42; const B: u32 = A; fn main() { let c = B; let _ = 2 ^ c; }

It seems like I'd need to iterate over the chain of definitions and grab the one that ends in a literal.

If I'm not mistaken, there is nothing like this in utils.

I remember doing something similar for the internal lint MATCH_TYPE_ON_DIAGNOSTIC_ITEM, in the function path_to_matched_type. I think that could help.

giraffate · 2020-11-11T23:42:16Z

ping from triage @cgm616. There seem to be some fixes left to be done. Do you have any questions on how to proceed here?

bors · 2020-11-25T16:51:01Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

giraffate · 2020-11-26T13:14:57Z

@ebroto 2 weeks have passed with no activity from previous ping, so this should be closed accorging to Rust triage procedure.

ebroto · 2020-11-26T22:41:44Z

Thanks for the heads-up, Takayuki ❤️

It breaks my heart having to close this as I think it's a cool lint. I will see if I can find some free time to address the last changes myself.

cgm616 · 2020-11-26T23:47:28Z

I know this is closed for triage reasons, but I am going to try to finish this. My school semester is almost done and I've been too busy over the last few weeks, but I should be able to pull this over the finish line soon.

rust-highfive assigned ebroto Oct 16, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 16, 2020

cgm616 marked this pull request as ready for review October 16, 2020 19:33

cgm616 force-pushed the xor-as-pow branch from 1c14158 to 2bab795 Compare October 16, 2020 21:18

cgm616 force-pushed the xor-as-pow branch from f818144 to 7a50ef1 Compare October 17, 2020 23:29

ebroto suggested changes Oct 18, 2020

View reviewed changes

ebroto added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Oct 18, 2020

cgm616 force-pushed the xor-as-pow branch 2 times, most recently from f9e21a1 to 03ca52e Compare October 22, 2020 04:08

giraffate reviewed Oct 22, 2020

View reviewed changes

tests/ui/xor_used_as_pow.rs Outdated Show resolved Hide resolved

cgm616 force-pushed the xor-as-pow branch from b4618f9 to 608ba73 Compare October 22, 2020 22:47

cgm616 requested a review from ebroto October 24, 2020 16:06

ebroto added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Oct 25, 2020

KamilaBorowska and others added 5 commits October 25, 2020 09:12

Add lint for use of ^ operator as exponentiation.

4336396

Update lint to current clippy API

773dc76

Apply suggestions from code review

282aafe

Minor docs and description changes Co-authored-by: Eduardo Broto <ebroto@tutanota.com>

Remove unrelated lints from tests

12f150e

Prevent linting on zero values

ebb71f2

cgm616 force-pushed the xor-as-pow branch from 608ba73 to ebb71f2 Compare October 25, 2020 13:13

ebroto suggested changes Oct 25, 2020

View reviewed changes

ebroto removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 25, 2020

ebroto added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Oct 25, 2020

ebroto closed this Nov 26, 2020

ebroto added S-inactive-closed Status: Closed due to inactivity and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Nov 26, 2020

giraffate mentioned this pull request Sep 26, 2022

Add lint for confusing use of ^ instead of .pow #9506

Merged

blyxyas removed the S-inactive-closed Status: Closed due to inactivity label Oct 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add new lint for xor-ing when `pow` was probably intended #6182

[WIP] Add new lint for xor-ing when `pow` was probably intended #6182

cgm616 commented Oct 16, 2020 •

edited by ebroto

Loading

rust-highfive commented Oct 16, 2020

cgm616 commented Oct 16, 2020

ebroto commented Oct 16, 2020

cgm616 commented Oct 16, 2020

cgm616 commented Oct 22, 2020

bors commented Oct 25, 2020

cgm616 commented Oct 25, 2020

ebroto left a comment •

edited

Loading

ebroto Oct 25, 2020

ebroto Oct 25, 2020

ebroto Oct 25, 2020

ebroto Oct 25, 2020

cgm616 Oct 25, 2020

ebroto Oct 25, 2020

giraffate commented Nov 11, 2020

bors commented Nov 25, 2020

giraffate commented Nov 26, 2020

ebroto commented Nov 26, 2020

cgm616 commented Nov 26, 2020

[WIP] Add new lint for xor-ing when pow was probably intended #6182

[WIP] Add new lint for xor-ing when pow was probably intended #6182

Conversation

cgm616 commented Oct 16, 2020 • edited by ebroto Loading

rust-highfive commented Oct 16, 2020

cgm616 commented Oct 16, 2020

ebroto commented Oct 16, 2020

cgm616 commented Oct 16, 2020

cgm616 commented Oct 22, 2020

bors commented Oct 25, 2020

cgm616 commented Oct 25, 2020

ebroto left a comment • edited Loading

Choose a reason for hiding this comment

ebroto Oct 25, 2020

Choose a reason for hiding this comment

ebroto Oct 25, 2020

Choose a reason for hiding this comment

ebroto Oct 25, 2020

Choose a reason for hiding this comment

ebroto Oct 25, 2020

Choose a reason for hiding this comment

cgm616 Oct 25, 2020

Choose a reason for hiding this comment

ebroto Oct 25, 2020

Choose a reason for hiding this comment

giraffate commented Nov 11, 2020

bors commented Nov 25, 2020

giraffate commented Nov 26, 2020

ebroto commented Nov 26, 2020

cgm616 commented Nov 26, 2020

[WIP] Add new lint for xor-ing when `pow` was probably intended #6182

[WIP] Add new lint for xor-ing when `pow` was probably intended #6182

cgm616 commented Oct 16, 2020 •

edited by ebroto

Loading

ebroto left a comment •

edited

Loading