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

Change Rc<Box<T>> recommendation to be Rc<T> instead of Box<T> #5899

Merged
merged 2 commits into from
Aug 13, 2020

Conversation

JarredAllen
Copy link
Contributor

@JarredAllen JarredAllen commented Aug 12, 2020

Fixes #5722

changelog: Suggest Rc<Box<T>> -> Rc<T> in [redundant_allocation] lint

@rust-highfive
Copy link

r? @yaahc

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 12, 2020
Copy link
Member

@yaahc yaahc left a comment

Choose a reason for hiding this comment

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

Looks great, ty for the PR.

cc @flip1995

Comment on lines 360 to 366
_ => panic!("Box that isn't a type"),
},
_ => panic!("Rc without type argument"),
};
let inner_span = match &last_path_segment(&box_ty).args.unwrap().args[0] {
GenericArg::Type(ty) => ty.span,
_ => panic!("Box without type argument"),
Copy link
Member

Choose a reason for hiding this comment

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

it's probably fine, I don't think any of these branches can get hit, but I think it might be better to early return rather than panicking.

@yaahc
Copy link
Member

yaahc commented Aug 12, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Aug 12, 2020

📌 Commit 4e28d99 has been approved by yaahc

@bors
Copy link
Contributor

bors commented Aug 12, 2020

⌛ Testing commit 4e28d99 with merge 68a2b5d...

bors added a commit that referenced this pull request Aug 12, 2020
Change Rc<Box<T>> recommendation to be Rc<T> instead of Box<T>

Changelog: Fixes #5722
@bors
Copy link
Contributor

bors commented Aug 12, 2020

💔 Test failed - checks-action_test

@flip1995
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Aug 13, 2020

⌛ Testing commit 4e28d99 with merge d5f5487...

@bors
Copy link
Contributor

bors commented Aug 13, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: yaahc
Pushing d5f5487 to master...

@bors bors merged commit d5f5487 into rust-lang:master Aug 13, 2020
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.

Clippy suggest Rc<Box<T>> -> Box<T>
5 participants