Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

suspicious_double_ref_op: don't lint on .borrow() #112517

Merged
merged 2 commits into from
Jun 16, 2023

Conversation

fee1-dead
Copy link
Member

closes #112489

@rustbot
Copy link
Collaborator

rustbot commented Jun 11, 2023

r? @b-naber

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 11, 2023
@@ -129,6 +129,9 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
NoopMethodCallDiag { method: call.ident.name, receiver_ty, label: span },
);
} else {
if op == "borrow" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment here explaining why borrow makes sense?

// FIXME(fee1-dead) stop using stringly typed diagnostic here
// If `type_of(x) == T` and `x.borrow()` is used to get `&T`,
// gthen that should be allowed
if op == "borrow" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
if op == "borrow" {
if name == sym::noop_method_borrow {

@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 15, 2023

📌 Commit 1caed51 has been approved by compiler-errors

It is now in the queue for this repository.

@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 Jun 15, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2023
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#112403 (Prevent `.eh_frame` from being emitted for `-C panic=abort`)
 - rust-lang#112517 (`suspicious_double_ref_op`: don't lint on `.borrow()`)
 - rust-lang#112529 (Extend `unused_must_use` to cover block exprs)
 - rust-lang#112614 (tweak suggestion for argument-position `impl ?Sized`)
 - rust-lang#112654 (normalize closure output in equate_inputs_and_outputs)
 - rust-lang#112660 (Migrate GUI colors test to original CSS color format)
 - rust-lang#112664 (Add support for test tmpdir to fuchsia test runner)
 - rust-lang#112669 (Fix comment for ptr alignment checks in codegen)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit db7d837 into rust-lang:master Jun 16, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 16, 2023
@tspiteri
Copy link
Contributor

tspiteri commented Jun 19, 2023

Should this be backported to the 1.71.0 beta? I'm asking because the issue is listed in the 1.71.0 milestone.

@cuviper
Copy link
Member

cuviper commented Jul 25, 2023

It's too late now for 1.71.0, of course, but it could be considered for 1.71.1. A warn-by-default lint is not such a big deal, but this is a trivial backport (clean cherry-pick), and we're planning to have a point release anyway.

FWIW, I also got a bug for this in CentOS Stream: https://bugzilla.redhat.com/show_bug.cgi?id=2225471

@rustbot label +stable-nominated

@rustbot rustbot added the stable-nominated Nominated for backporting to the compiler in the stable channel. label Jul 25, 2023
@fee1-dead fee1-dead deleted the sus-op-no-borrow branch July 26, 2023 03:52
@apiraino
Copy link
Contributor

Beta backport accepted as per compiler team on Zulip

@rustbot label +stable-accepted

@rustbot rustbot added the stable-accepted Accepted for backporting to the compiler in the stable channel. label Jul 28, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 31, 2023
Prepare Rust 1.71.1

This PR prepares the Rust 1.71.1 release, which contains:

* rust-lang#113802
* rust-lang#113579
* rust-lang#111516
* rust-lang#112517
* rust-lang@67b5990 (from rust-lang#113678)

r? `@ghost`
cc `@rust-lang/release`
@cuviper cuviper removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Aug 12, 2023
@cuviper cuviper modified the milestones: 1.72.0, 1.71.1 Aug 12, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. stable-accepted Accepted for backporting to the compiler in the stable channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing new warning suspicious_double_ref_op in beta 1.71.0
9 participants