Politely mention that Box<&T> is almost the most useless type you can have

[oli-obk]

Found in the wild: https://stackoverflow.com/questions/48406077/mutation-of-a-variable-inside-while-loop

List of types that make no sense that we should be linting about:

• Box<&T>
• Box::new(SomeT) where sizeof::<T>() <= sizeof::<usize>()
  • unless there are Box::into_raw calls within the function
• Rc<Box<T>>
• Rc<Rc<T>>
• Rc<&T>
• more...?

[clarfonthey]

There's already a lint for Box<Vec>; why not extend this to the above types?

We don't include Box<String> iirc.

[killercup]

Hm, so Box<&T> is most likely by accident, I agree.

But thinking about the box_vec lint I'm not sure I agree with its reasoning:

Vec already keeps its contents in a separate area on the heap. So if you Box it, you just add another level of indirection without any benefit whatsoever.

There is a benefit I can think of: Boxing a Vec or String reduces the size from 3 words to 1. I'm sure some this may be a good idea, and, more importantly, this is most likely done on purpose. But maybe this is just a good idea in my imagination. Or maybe there is a crate that gives you a Vec-like API but stores length an capacity as part of the heap data and we should suggest using that?

[spacekookie]

Hi! I'd be interested in implementing this, the Contributing.md told me to leave a comment here and be assigned a mentor? 😄

comments

[oli-obk]

👍 great

So a lot has already been mentioned here. Most importantly the fact that we already have related lints that could be extended to cover some of the cases here.

I think as a first (and uncontroversial) step you should concentrate on the Box<&T> case.

[killercup]

Not sure if it's mentioned in the Contributing file, or how useful it is in this case, but clippy brings with it an #[author] attribute (src) that generates pattern matching code for the items you added it to.

(Btw, saw your barrel crate earlier! Will definitely need to try it out! Oh, and: Have you seen this stuff? :))

[spacekookie]

@killercup Hey 😄 Yea, I've seen those blocks in a few other lint files already. Although I'm not sure exactly how to start, at least from the ones I've looked at so far. Is there maybe one of the lints that especially easy so I can wrap my head around the structure?

(I talked to Sean yesterday about integrating barrel as an optional feature into Diesel 😄 Definitely excited about that!)

[Manishearth]

As an aside, I want to eventually change the author lint so that it lets us do coccinelle style lints, i.e. your lint code is just the code template you're looking for, wrapped in a macro.

[Manishearth]

Oh, and to answer the original question, I can mentor, but I'm out of town for another day and may have a spotty connection. But the tips in this issue should get you started, lmk if you have questions!

[spacekookie]

@Manishearth That's absolutely fine. I thought I'd drop the comment here tonight, didn't expect an immediate response 😅

I'll be in a position to start work on this tomorrow. Should I drop you a message on gitter to stop spamming here then?

[Manishearth]

I'm not very active on gutter: spamming here is the best way to go. Or IRC, but IRC isn't good for asynchronous discussions.

If you're afraid of bothering folks, don't be. But if you want you can open a partial pull request and we can discuss these instead.

[spacekookie]

So I've finally gotten around trying to write some code for this but I have an issue. Commit 6f48e37 doesn't actually compile for me which makes testing my own code rather difficult 😜

Any way I can get around that?

[oli-obk]

Uh... what do you mean by "doesn't compile"? Are you on the most recent nightly?

[spacekookie]

Ah d'oh! I could have sworn I ran rustup update before but I think I must have been on a nightly from a couple weeks ago

[03k64]

@spacekookie are you still working on this? It might provide a good first issue to tackle as part of the impl days, if you are working on it would you be able to update on any progress?

[spacekookie]

Hey, yea no I haven't done anything on this in a while >.> I still wanted to try to do it but if someone else wants to work on this, go ahead 🙂

[clarfonthey]

This isn't just a beginner issue; rust-lang/rust#51281 shows an example of this in the compiler. This should probably look through type aliases too.

[cauebs]

Is there something left to be done here?

Maybe this?

• Box::new(SomeT) where sizeof::<T>() <= sizeof::<usize>()
  • unless there are Box::into_raw calls within the function

I'm willing to work on it, just need to make sure it's wanted.

[jyn514]

more...?

Rc<String> (should just be Rc<str>)

[Manishearth]

I'm unsure about small boxes, but it seems fine. Definitely worth making a PR

[joao-conde]

I this being worked on currently? Could I take it? Which remaining boxed types would we like to raise an alert for?