Feat: Add debug/copy derives and enable missing debug/copy lint

[stegaBOB]

Types that can implement copy generally should: https://doc.rust-lang.org/std/marker/trait.Copy.html#when-should-my-type-be-copy

For debug, the rust API guidelines suggest debug should always be implemented in public types: https://rust-lang.github.io/api-guidelines/debuggability.html#all-public-types-implement-debug-c-debug

[febo]

Great, thanks!

[LStan]

Is it a good idea to derive Copy in code that tends to be zero-copy? Especially for sysvars (and perhaps Account in instruction.rs).
Imagine I have the following code:

let clock = Clock::get()?;
some_fn(clock, ...);

Here, a move happens, so I don't really want to bother to use reference.
However, after some time, I add more code after some_fn that uses clock variable.
Without the Copy trait, the compiler will tell me that I’m using a moved variable, and I'll consider changing the parameter of some_fn from Clock to &Clock. But with the Copy trait, I might not realize that a copy is happening.

[febo]

Is it a good idea to derive Copy in code that tends to be zero-copy? Especially for sysvars (and perhaps Account in instruction.rs). Imagine I have the following code:

let clock = Clock::get()?;
some_fn(clock, ...);

Here, a move happens, so I don't really want to bother to use reference. However, after some time, I add more code after some_fn that uses clock variable. Without the Copy trait, the compiler will tell me that I’m using a moved variable, and I'll consider changing the parameter of some_fn from Clock to &Clock. But with the Copy trait, I might not realize that a copy is happening.

This is a good point. Perhaps would be prudent to not implement Copy in our case since we should be minimizing copies in a program. Agree with you that Copy makes "moves" silent in the sense that every assignment is a copy. What do you think @stegaBOB?

[stegaBOB]

I still think if a type can be copy it probably should (which is what the std docs say as well). I'd imagine the compiler would probably optimize any unnecessary copies out anyway, although that would perhaps be worth benchmarking. If you really don't want the type to be copy, you could always make a newtype wrapper around the struct and not impl copy on that.

#[derive(Clone)]
#[repr(transparent)]
pub struct RentNoCopy(Rent);

Having it copy allows more choices. In star frame, we plop sysvars in a Cell, which is only possible since they're copy: https://github.com/staratlasmeta/star_frame/blob/eb1d9f8b8a3a10e88bfc08bd459433881cce56b6/star_frame/src/context.rs#L17

[febo]

I still think if a type can be copy it probably should (which is what the std docs say as well). I'd imagine the compiler would probably optimize any unnecessary copies out anyway, although that would perhaps be worth benchmarking. If you really don't want the type to be copy, you could always make a newtype wrapper around the struct and not impl copy on that.

#[derive(Clone)]
#[repr(transparent)]
pub struct RentNoCopy(Rent);

Having it copy allows more choices. In star frame, we plop sysvars in a Cell, which is only possible since they're copy: https://github.com/staratlasmeta/star_frame/blob/eb1d9f8b8a3a10e88bfc08bd459433881cce56b6/star_frame/src/context.rs#L17

Yeah, that is also a good point. 😊 Given that it is possible to go from Copy to non-Copy, but not possible the other way around, let's keep it.

[LStan]

The advice to implement Copy makes sense for std and non-constrained environment. But Pinocchio is used in a very constrained environment. The code in my example will not be optimized by the compiler, it will create a copy and use additional CUs without you realizing it.
Also, sysvars in the original sdk don't implement Copy, only CloneZeroed.
Maybe it is better to put Copy derivation into a feature?

[febo]

Maybe it is better to put Copy derivation into a feature?

This is probable the best compromise.