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

Add size-parameter to unecessary_box_returns #10651

Merged
merged 1 commit into from
Apr 19, 2023

Conversation

lukaslueg
Copy link
Contributor

@lukaslueg lukaslueg commented Apr 16, 2023

Fixes #10641

This adds a configuration-knob to the unecessary_box_returns-lint which allows not linting a fn() -> Box<T> if T is "large". The default byte size above which we no longer lint is 128 bytes (due to #4652 (comment), also used in #9373). The overall rational is given in #10641.


changelog: Enhancement: [unnecessary_box_returns]: Added new lint configuration unnecessary-box-size to set the maximum size of T in Box<T> to be linted
#10651

@rustbot
Copy link
Collaborator

rustbot commented Apr 16, 2023

r? @xFrednet

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 16, 2023
Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

LGTM, tiny nit related to the lint documentation comment. It requires some specific syntax. :)

@@ -463,6 +463,10 @@ define_Conf! {
///
/// The maximum byte size a `Future` can have, before it triggers the `clippy::large_futures` lint
(future_size_threshold: u64 = 16 * 1024),
/// Lint: UNNECESSARY_BOX_RETURNS,
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
/// Lint: UNNECESSARY_BOX_RETURNS,
/// Lint: UNNECESSARY_BOX_RETURNS.

This list requires a dot at the end, to be parsed correctly. Once you do this, you also have to run cargo collect-matadata to update the lint configuration list in our book.

@xFrednet
Copy link
Member

xFrednet commented Apr 19, 2023

Thank you! You can r=me (meaning commenting bors r=xFrednet with an @ in front) once you did the change :)

@bors delegate+

@bors
Copy link
Collaborator

bors commented Apr 19, 2023

✌️ @lukaslueg can now approve this pull request

@lukaslueg
Copy link
Contributor Author

@bors r=xFrednet

@bors
Copy link
Collaborator

bors commented Apr 19, 2023

📌 Commit 4bc68f9 has been approved by xFrednet

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Apr 19, 2023

⌛ Testing commit 4bc68f9 with merge 0c44586...

@bors
Copy link
Collaborator

bors commented Apr 19, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 0c44586 to master...

@bors bors merged commit 0c44586 into rust-lang:master Apr 19, 2023
@lukaslueg lukaslueg deleted the issue10641 branch April 19, 2023 13:07
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.

unnecessary_box_returns don't lint on large objects
4 participants