Skip to content

Conversation

@jpospychala
Copy link
Contributor

refers to #2394

Don't use Box<&T>

@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Mar 12, 2020
@jpospychala jpospychala force-pushed the box_rptr branch 3 times, most recently from b7edf08 to ca5f1c0 Compare March 13, 2020 20:50
@jpospychala jpospychala force-pushed the box_rptr branch 2 times, most recently from d7eea04 to 7196d14 Compare March 17, 2020 20:37
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM overall. Some small name/doc/test changes remaining

@jpospychala jpospychala force-pushed the box_rptr branch 2 times, most recently from a35a8b2 to f9d5758 Compare March 20, 2020 20:01
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.

r=me with one change

cx,
BOX_BORROWS,
hir_ty.span,
"usage of `Box<&T>`",
Copy link
Member

Choose a reason for hiding this comment

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

We should use span_lint_and_suggestion here and emit a suggestion instead of a help

Copy link
Member

Choose a reason for hiding this comment

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

With the text being something like "Box<&T> does not offer any advantages over &T, please try: {}"

Copy link
Member

Choose a reason for hiding this comment

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

You'd want to also add a fixture test for this (add run-rustfix to the top and run/update the tests a couple times)

@bors
Copy link
Contributor

bors commented Mar 30, 2020

☔ The latest upstream changes (presumably #5294) made this pull request unmergeable. Please resolve the merge conflicts.

@jpospychala
Copy link
Contributor Author

moving those changes to #5349

@jpospychala jpospychala closed this Apr 1, 2020
bors added a commit that referenced this pull request Apr 2, 2020
useless Rc<Rc<T>>, Rc<Box<T>>, Rc<&T>, Box<&T>

refers to  #2394

changelog: Add lints for Rc<Rc<T>> and Rc<Box<T>> and Rc<&T>, Box<&T>

this is based on top of another change #5310 so probably should go after that one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants