feat: Add support for more packages to DeepSize

[pmnoxx]

I would like to add support for a few types from tokio, actix.

• use tokio::net::TcpStream
• use actix::Addr

[pmnoxx]

@matklad Please, let me know whenever it's a good idea to split this change. I can split it into 4 parts

• more array sizes
• std
• tokio
• actix

[matklad]

For array sizes, https://github.com/Aeledfyr/deepsize/pull/16 is already split off.

I guess the rest looks fine, but thats for @Aeledfyr to decide :)

I would probably suggest to refactor the code a bit to make sure that per-crate impls live in crate-specific file (ie, alloc.rs, std.rs, tokio.rs, actix.rs), but that is orthogonal to adding the impls at all.

[Aeledfyr]

This looks good. One issue though is with adding feature dependencies: https://github.com/Aeledfyr/deepsize/blob/90a1efc15e02cdc617b93fb72b2bd61235e0e59c/Cargo.toml#L23 By adding a dependency on tokio with the full feature set, I believe that we enable that feature set in all crates that depend on deepsize, even if they don't enable the tokio feature in deepsize.

Is there any way to properly deal with this? For example, it might be possible to have a tokio-net feature that depends on tokio with the net feature enabled, but that only works if the feature flags of optional dependencies are ignored when the dependency isn't explicitly enabled.

[Aeledfyr]

Splitting the std_time and std_net parts out from this (into another PR?) would be a good idea, since they are easy to merge without dealing with dependency resolution.

Also, most of those implementations could use the known_deep_size macro to reduce verbosity, like the chrono impl does: https://github.com/Aeledfyr/deepsize/blob/23065db143811324ada5aeb87bc4410cf832c6a5/src/external_impls.rs#L167-L172

[pmnoxx]

@Aeledfyr Ok, I made a split https://github.com/Aeledfyr/deepsize/pull/19

[pmnoxx]

@Aeledfyr Can you take a look again?

[Aeledfyr]

There are still some issues with dependency resolution. This PR fails to compile with tokio being the only feature (cargo check --features tokio), since it doesn't enable the net feature. However, if you compile it with all features (cargo check --all-features) it will work, because actix enables the net feature in tokio.

Also, the actix dependency should be specified with default-features=false to avoid enabling its macro feature, for dependencies attempting to reduce compile time.

I think the cleanest way to solve the tokio feature issue would be to make tokio an optional feature with default-features=false, and to make a feature tokio_net (or something similar) that enables tokio, tokio/net (to enable the feature in tokio), and enables the DeepSize implementations.

As a basic example of how I think it could work:

[dependencies]
tokio = { version = "^1.1", optional = true, default-features = false }
# default-features is technically unnecessary here as tokio
# has no default features, but adding it is good practice for
# a library
# ...

[features]
tokio_net = ["tokio", "tokio/net"]
# ...

#[cfg(feature = "tokio_net")]
mod tokio_net_impl {
    // ...
}

I haven't verified that this works, but it seems like it should. I also haven't verified that this won't lead to accidentally enabling features in dependant's crates, but from what I can tell this requires the feature to be explicitly enabled for the tokio/net feature to be enabled.

[pmnoxx]

@Aeledfyr Yes, it works. Thanks.

[pmnoxx]

@Aeledfyr I updated the PR, can you take a look?

[Aeledfyr]

Sorry about the delay; I believe the actix dependency should also have default-features = false, to avoid enabling the macros feature in actix for dependencies.

[pmnoxx]

Sorry about the delay; I believe the actix dependency should also have default-features = false, to avoid enabling the macros feature in actix for dependencies.

@miguelfrde Thanks, I updated the PR.

tokio = { version = "^1.1", optional = true, default-features = false }
actix = { version = "^0.11.0", optional = true, default-features = false }

[Aeledfyr]

I can't verify that the feature resolution works entirely properly, but from cargo tree -e features and cargo expand, it looks like this works, and looks good to merge.