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

Confusing new warning suspicious_double_ref_op in beta 1.71.0 #112489

Closed
tspiteri opened this issue Jun 10, 2023 · 4 comments · Fixed by #112517
Closed

Confusing new warning suspicious_double_ref_op in beta 1.71.0 #112489

tspiteri opened this issue Jun 10, 2023 · 4 comments · Fixed by #112517
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@tspiteri
Copy link
Contributor

Code

use std::borrow::Borrow;

struct S;

trait T: Sized {
    fn foo(self) {}
}

impl T for S {}
impl T for &S {}

fn main() {
    let s = S;
    s.borrow().foo();
    s.foo();
}

Current output

Compiling borrow v0.1.0 (/tmp/borrow)
warning: using `.borrow()` on a double reference, which returns `&S` instead of borrowing the inner type
  --> src/main.rs:14:6
   |
14 |     s.borrow().foo();
   |      ^^^^^^^^^
   |
   = note: `#[warn(suspicious_double_ref_op)]` on by default

warning: `borrow` (bin "borrow") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s

Desired output

Compiling borrow v0.1.0 (/tmp/borrow)
    Finished dev [unoptimized + debuginfo] target(s) in 0.20s

Rationale and extra context

If I have a type S, and a trait T that is implemented for both S and &S, s.borrow() is a valid way to get a reference &S from an object of type S. But since beta 1.71, this emits a warning. I have no idea what the warning is trying to convey, what "double reference" is referring to, or what "the inner type" is.

Other cases

No response

Anything else?

$ rustc +beta --version
rustc 1.71.0-beta.3 (78a6ac0a8 2023-06-08)
@tspiteri tspiteri added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 10, 2023
@Mark-Simulacrum Mark-Simulacrum added this to the 1.71.0 milestone Jun 10, 2023
@Mark-Simulacrum Mark-Simulacrum added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Jun 10, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 10, 2023
@workingjubilee
Copy link
Member

cc @fee1-dead

@fee1-dead
Copy link
Member

Ah. then I think this lint shouldn't apply to Borrow since x.borrow() can mean specifically to borrow x. But I think this should still apply to .clone() and .deref()

@apiraino
Copy link
Contributor

(sorry)

@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 13, 2023
@bors bors closed this as completed in db7d837 Jun 16, 2023
SWvheerden added a commit to tari-project/tari-crypto that referenced this issue Aug 3, 2023
There's a warning about an allegedly suspicious borrow in one of the
range proof tests. It's likely [already
fixed](rust-lang/rust#112489), but easy enough
to use the ampersand operator to clean up the warning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants