-
Notifications
You must be signed in to change notification settings - Fork 184
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
Make ChainedConnector use tuples instead of nesting #976
base: main
Are you sure you want to change the base?
Conversation
91a1ec1
to
703d9b1
Compare
One problem with this approach is that macros are re-expanded and re-compiled every compilation. I don't believe there is any caching for them. I've been meaning to revisit my use of macros for stuff like get/head/post etc to unroll them and get rid of macros altogether. This is kinda taking us backwards unless we unroll these macros upfront as well. |
I thought this was possible: #[derive(Debug)]
pub struct DefaultConnector {
inner: ChainedConnector<(), (
#[cfg(feature = "_test")]
test::TestConnector,
#[cfg(feature = "socks-proxy")]
SocksConnector,
#[cfg(feature = "socks-proxy")]
WarnOnNoSocksConnector,
TcpConnector,
#[cfg(feature = "rustls")]
RustlsConnector,
#[cfg(feature = "native-tls")]
NativeTlsConnector,
#[cfg(feature = "_tls")]
WarnOnMissingTlsProvider,
)>,
} but it's not possible because tuples in a type definition cannot have some elements behind a I still think it can lead to better type definitions and error messages. |
I expanded the macros, but benchmarking shows no significant performance improvement. Hyperfine for
For the commit after expansion:
Running the benchmark multiple times leads to inconsistent results. The difference between the two is too small to be distinguished from the noise. I don't think it's worth the added maintenance to have duplicated code. Note that by the same logic it would make sense to pre-expand all the |
607a745
to
e8ab815
Compare
No description provided.