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

uplift clippy::clone_double_ref as suspicious_double_ref_op #110955

Merged
merged 3 commits into from
May 2, 2023

Conversation

fee1-dead
Copy link
Member

Split from #109842.

r? @compiler-errors

@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 Apr 28, 2023
@rustbot
Copy link
Collaborator

rustbot commented Apr 28, 2023

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@fee1-dead fee1-dead changed the title uplift clippy::clone_double_ref as suspicious_double_ref_ops uplift clippy::clone_double_ref as suspicious_double_ref_op Apr 28, 2023
@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member

r=me with that rustdoc fix

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Apr 29, 2023

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@fee1-dead
Copy link
Member Author

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Apr 29, 2023

📌 Commit e928067 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 Apr 29, 2023
@@ -50,6 +50,9 @@ lint_deprecated_lint_name =
lint_renamed_or_removed_lint = {$msg}
.suggestion = use the new name

lint_suspicious_double_ref_op =
using `.{$call}()` on a double reference, which returns `{$ty}` instead of {$op} the inner type
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
using `.{$call}()` on a double reference, which returns `{$ty}` instead of {$op} the inner type
using `.{$call}()` on a double reference, which returns `{$ty}` instead of {$op}, the inner type

Correct me (a non-native speaker of english) if I'm wrong, but I think there needs to either be a comma, or it needs to be reodered instead of the inner type {$op}. I think it's better to have the two types close to each other, so I prefer the variant with the comma.

Copy link
Member Author

Choose a reason for hiding this comment

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

$op is not a type. It is either "dereferencing", "borrowing", or "cloning" depending on the trait.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Those words should also be in the ftl file then however.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh right.. hmmmm. How should I do that

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose they don't have to? Since https://projectfluent.org/ shows that you can match on variables.

Copy link
Member

Choose a reason for hiding this comment

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

I think at the very least you should list it explicitly, so that translators know that there is only these options. From what I understand, the translation system was designed so that translators do not have to look at the source code at all.

{$op ->
    *[should_not_happen] [{$op}]
    [deref] dereferencing
    [borrow] borrowing
    [clone] cloning
}

How to pair that up with enums, I don't really know.

@fee1-dead
Copy link
Member Author

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Apr 29, 2023

📌 Commit 475378f has been approved by compiler-errors

It is now in the queue for this repository.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 30, 2023
…=compiler-errors

uplift `clippy::clone_double_ref` as `suspicious_double_ref_op`

Split from rust-lang#109842.

r? `@compiler-errors`
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2023
Rollup of 7 pull requests

Successful merges:

 - rust-lang#105076 (Refactor core::char::EscapeDefault and co. structures)
 - rust-lang#108161 (Add `ConstParamTy` trait)
 - rust-lang#108668 (Stabilize debugger_visualizer)
 - rust-lang#110512 (Fix elaboration with associated type bounds)
 - rust-lang#110895 (Remove `all` in target_thread_local cfg)
 - rust-lang#110955 (uplift `clippy::clone_double_ref` as `suspicious_double_ref_op`)
 - rust-lang#111048 (Mark`feature(return_position_impl_trait_in_trait)` and`feature(async_fn_in_trait)` as not incomplete)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 40c4ed4 into rust-lang:master May 2, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 2, 2023
@fee1-dead fee1-dead deleted the sus-operation branch May 2, 2023 10:33
flip1995 pushed a commit to flip1995/rust that referenced this pull request May 5, 2023
…=compiler-errors

uplift `clippy::clone_double_ref` as `suspicious_double_ref_op`

Split from rust-lang#109842.

r? ``@compiler-errors``
tamird added a commit to tamird/gimli that referenced this pull request May 23, 2023
tamird added a commit to tamird/gimli that referenced this pull request May 23, 2023
tamird added a commit to tamird/gimli that referenced this pull request May 24, 2023
tamird added a commit to tamird/gimli that referenced this pull request May 24, 2023
imbillow pushed a commit to imbillow/gimli that referenced this pull request Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

6 participants