Stream: general

Topic: libc traits - https://github.com/rust-lang/rfcs/pull/2235


gnzlbg (Nov 28 2018 at 15:33, on Zulip):

@Alex Crichton why do you think the libc traits have to be behind a feature at all ? I don't really follow the rationale.

gnzlbg (Nov 28 2018 at 15:35, on Zulip):

I mean, during the RFC it was argued that this should depend on what impact these features would have, and @Susurrus went out of its way to actually implement this, and measure the effect of these features on compile-times.

The effect is negligible, ~1 extra second of compile time in @Susurrus benchmarks. I think that having this as an off by default single feature like @SimonSapin suggests could be "ok" (don't pay for what you don't use), but I am not sure if its worth it because it's something else in the matrix that we have to test, gets less use if not everybody has it enabled, all code would be full of cfg_attrs, etc. and all of this just to shave 1 second in compile times "once".

gnzlbg (Nov 28 2018 at 15:36, on Zulip):

And even then, libc is probably compiled in parallel with other crates, so that ~1 extra second might turn out to be even less than that in practice.

Alex Crichton (Nov 28 2018 at 15:37, on Zulip):

@gnzlbg for me it's just being conservative, we can always turn it on by default later

Alex Crichton (Nov 28 2018 at 15:37, on Zulip):

libc is just so tricky to change

Alex Crichton (Nov 28 2018 at 15:37, on Zulip):

I agree there's no technical known reason to not turn on by default

Alex Crichton (Nov 28 2018 at 15:37, on Zulip):

I figure we can see what the experience is if they're turned off

gnzlbg (Nov 28 2018 at 15:37, on Zulip):

yeah, i would prefer to say that this has to be implemented behind a single feature first, but than then we should decide whether we keep it that way, or not

gnzlbg (Nov 28 2018 at 15:37, on Zulip):

the moment the cargo feature is "stable", we have to keep it around forever

Alex Crichton (Nov 28 2018 at 15:38, on Zulip):

I think w/e we publish to crates.io must be considered insta-stable though

gnzlbg (Nov 28 2018 at 15:38, on Zulip):

implementing this behind one cargo feature per-trait as @David Tolnay suggests feels a bit overkill TBH

Alex Crichton (Nov 28 2018 at 15:38, on Zulip):

I don't think we have much of an out to avoiding that

Alex Crichton (Nov 28 2018 at 15:38, on Zulip):

it doesn't matter too much though?

Alex Crichton (Nov 28 2018 at 15:39, on Zulip):

they're small names and small features

gnzlbg (Nov 28 2018 at 15:39, on Zulip):

I think @Susurrus wanted to send a single PR implementing this for the whole library at once

Alex Crichton (Nov 28 2018 at 15:39, on Zulip):

sure? but that's still fine to have a feature-per-trait

gnzlbg (Nov 28 2018 at 15:40, on Zulip):

Its one cfg_attr per trait per type, I mean, can be done, but looks like too much work for too little win

RalfJ (Nov 28 2018 at 15:40, on Zulip):

(FWIW, it'd be nice if there was link to context here, because without this discussion is rather meaningless to observers^^)

gnzlbg (Nov 28 2018 at 15:40, on Zulip):

https://github.com/rust-lang/rfcs/pull/2235

RalfJ (Nov 28 2018 at 15:41, on Zulip):

thanks!

Alex Crichton (Nov 28 2018 at 15:42, on Zulip):

@gnzlbg there's just one location this is being done though? in one macro?

gnzlbg (Nov 28 2018 at 15:42, on Zulip):

I mean: #[cfg_attr(feature = "trait-eq", derive(Eq))] #[cfg_attr(feature = "trait-..."), derive(...)] ... we can put it all behind macros, but we would be having all these features to shave a fraction of that 1 extra second

Alex Crichton (Nov 28 2018 at 15:42, on Zulip):

I'm not necessarily worried about the exact time today

Alex Crichton (Nov 28 2018 at 15:42, on Zulip):

but moreso about the long-term-future of libc

gnzlbg (Nov 28 2018 at 15:42, on Zulip):

yes, i think that would all go in the s macro

Alex Crichton (Nov 28 2018 at 15:42, on Zulip):

as we continue to add more and more types

Alex Crichton (Nov 28 2018 at 15:43, on Zulip):

like this was a clear win in winapi

gnzlbg (Nov 28 2018 at 15:43, on Zulip):

i mean, winapi is huge

gnzlbg (Nov 28 2018 at 15:43, on Zulip):

if libc turns into winapi, then everybody will be paying for a lot of stuff they don't use anyways, just check winapi feature flags

gnzlbg (Nov 28 2018 at 15:44, on Zulip):

they have dozens and dozens of features

Alex Crichton (Nov 28 2018 at 15:44, on Zulip):

I guess I just don't understand why there's pushback here

Alex Crichton (Nov 28 2018 at 15:44, on Zulip):

it shoudl be very easy to implement this

Alex Crichton (Nov 28 2018 at 15:44, on Zulip):

and we can change it in the future

Alex Crichton (Nov 28 2018 at 15:44, on Zulip):

what's the downside?

gnzlbg (Nov 28 2018 at 15:45, on Zulip):

the downside is that's just more features to test in the matrix for little win

Alex Crichton (Nov 28 2018 at 15:45, on Zulip):

we definitely don't need to test this on all platforms, that's just overkill

Alex Crichton (Nov 28 2018 at 15:45, on Zulip):

it'd just be like one extra entry in one place

gnzlbg (Nov 28 2018 at 15:46, on Zulip):

we have to build with this on all platforms at least

gnzlbg (Nov 28 2018 at 15:47, on Zulip):

not run the libc-tests (that's pointless I think), but check that we build properly

gnzlbg (Nov 28 2018 at 15:47, on Zulip):

some of the repr attributes interact poorly with deriving some traits

gnzlbg (Nov 28 2018 at 15:47, on Zulip):

e.g. the packed structs cannot derive Debug

Alex Crichton (Nov 28 2018 at 15:48, on Zulip):

Ok? Can you take these arguments to the RFC? I'm not convinced but others might

gnzlbg (Nov 28 2018 at 15:48, on Zulip):

in any case, I'm fine with putting all these traits behind a single feature, but one per trait feels overkill to me

gnzlbg (Nov 28 2018 at 15:49, on Zulip):

i didn't consider your argument that this allow us to not offer this "by default" in the future if libc grows enough that this would become a problem

Last update: Nov 20 2019 at 12:35UTC