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

Use dereference instead of references for comparisons #10100

Open
Valchap opened this issue Dec 18, 2022 · 4 comments
Open

Use dereference instead of references for comparisons #10100

Valchap opened this issue Dec 18, 2022 · 4 comments
Labels
A-lint Area: New lints

Comments

@Valchap
Copy link

Valchap commented Dec 18, 2022

What it does

There are two possibilities for comparing a variable and a reference

either making a reference to the variable

&variable == reference

or dereferencing the reference

variable == *reference

Since dereferencing generates better assembly it should be recommended instead of its counterpart

Here is the assembly I get with the compiler explorer with the following source code

pub fn compare_with_reference(a: u32, b: &u32) -> bool {
    &a == b
}

pub fn compare_with_dereference(a: u32, b: &u32) -> bool {
    a == *b
}
compare_with_reference:
        sub     rsp, 24
        mov     dword ptr [rsp + 4], edi
        mov     qword ptr [rsp + 8], rsi
        lea     rax, [rsp + 4]
        mov     qword ptr [rsp + 16], rax
        lea     rdi, [rsp + 16]
        lea     rsi, [rsp + 8]
        call    qword ptr [rip + core::cmp::impls::<impl core::cmp::PartialEq<&B> for &A>::eq@GOTPCREL]
        mov     byte ptr [rsp + 3], al
        mov     al, byte ptr [rsp + 3]
        and     al, 1
        movzx   eax, al
        add     rsp, 24
        ret

compare_with_dereference:
        cmp     edi, dword ptr [rsi]
        sete    al
        and     al, 1
        movzx   eax, al
        ret

Lint Name

unnecessary_reference_in_comparison

Category

perf

Advantage

  • Unified way to compare variable with references across the code base
  • Smaller faster and more readable assembly is generated

Drawbacks

  • With opt-level 1 and higher the two ways of comparing generate the same assembly

Example

pub fn compare(a: u32, b: &u32) -> bool {
    &a == b
}

Could be written as:

pub fn compare(a: u32, b: &u32) -> bool {
    a == *b
}
@Valchap Valchap added the A-lint Area: New lints label Dec 18, 2022
@matthiaskrgr
Copy link
Member

Maybe this should be a mir opt instead..

@kraktus
Copy link
Contributor

kraktus commented Dec 18, 2022

with -C opt-level=3 (ie: release mode), the code generates the same assembly, as expected. By default Godbout generates code without optimisations.

example::compare_with_reference:
        cmp     dword ptr [rsi], edi
        sete    al
        ret
example::compare_with_dereference:
        cmp     dword ptr [rsi], edi
        sete    al
        ret

@Valchap
Copy link
Author

Valchap commented Dec 18, 2022

Yes, it's already the case with opt-level=1 as stated in the drawbacks.
It would only be interesting in term of performance for debug builds.
But I feel like the most interesting benefit is to have a unique way to compare variables with references across the code base.

@Valchap
Copy link
Author

Valchap commented Dec 20, 2022

Actually this lint already exists : https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
but it is only working for

 &a == &b

and not for the case where one side is a reference and not the other even though it is the given lint example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

3 participants