Skip to content

Use a better message for toplevel_ref_arg lint #14132

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

Merged
merged 1 commit into from
Feb 1, 2025

Conversation

samueltardieu
Copy link
Contributor

A ref pattern applied to an argument is not ignored. It creates a reference as expected, but still requires the function to take ownership of the argument given to it.

Fix #14131

changelog: [toplevel_ref_arg]: use a clearer lint message

@rustbot
Copy link
Collaborator

rustbot commented Feb 1, 2025

r? @Manishearth

rustbot has assigned @Manishearth.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 1, 2025
@samueltardieu
Copy link
Contributor Author

Don't hesitate to suggest an even clearer message

@@ -169,7 +169,7 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
TOPLEVEL_REF_ARG,
arg.hir_id,
arg.pat.span,
"`ref` directly on a function argument is ignored. \
"`ref` directly on a function argument requires ownership of the argument nonetheless. \
Copy link
Member

Choose a reason for hiding this comment

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

"does not prevent taking ownership of the passed parameter"

Copy link
Contributor Author

@samueltardieu samueltardieu Feb 1, 2025

Choose a reason for hiding this comment

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

Better.

I suggest: "ref directly on a function parameter does not prevent taking ownership of the passed argument" which is stricter in terms of param/arg. Is that ok?

A `ref` pattern applied to an argument is not ignored. It creates a
reference as expected, but still requires the function to take ownership
of the argument given to it.
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

Ah yes I had parameter/argument backwards

@Manishearth Manishearth added this pull request to the merge queue Feb 1, 2025
Merged via the queue into rust-lang:master with commit 7e5cfbe Feb 1, 2025
11 checks passed
@samueltardieu samueltardieu deleted the push-uxzmzkkulryr branch February 1, 2025 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

toplevel_ref_arg warning for a function argument is misleading
3 participants