Make `i128` support automatic

[dhardy]

To revisit #236, it would be more convenient for users if i128 support was enabled automatically on compatible compilers. We already argued in July that this was the best option for Rand.

The linked PR shows how this can be achieved without any breakage (except for old nightlies, which are generally not possible to ensure support for anyway).

[dhardy]

Since build scripts cannot enable features, this would actually require that @BurntSushi's byteorder also use auto-detection, and then that bincode depend on the new version of byteorder.

[BurntSushi]

I agree it would be nice to have auto detection here. It looks like i128 wasn't stabilized until Rust 1.26, which feels fairly recent to me.

In terms of auto detection, what are we thinking here? We could use build.rs to version sniff and enable i128 support that way. I've done this with regex, but the concern I have here is that this would be adding new public APIs based on the version of Rust used to compile byteorder. Is there precedent for this in the ecosystem? Are there unintended consequences of this? (I see now that rand is already doing this. Has this worked well?)

[dhardy]

We haven't had any issues — but I don't believe this implemented for the 0.5 release, and 0.6 is still pending, so it hasn't had a lot of testing.

[dhardy]

As @newpavlov points out, we shouldn't have to depend on bincode just to enable a feature in case users want to use binary serialisation (in this case, the type being serialised uses u128).

Users of old (pre-1.26) stable compilers can't use i128 anyway, and we shouldn't care about old nightlies, so the only effects of auto-enabling this are: (1) users of recent compilers don't have to bother with the i128 feature any more.

I was going to add (2), users may not notice when they do something losing backwards compatibility. But (a) the only way to ensure this is to test and (b) i128 obviously requires >= 1.26 anyway.

Conclusion: I don't think there's any reason not to do this.

[TyOverby]

This seems reasonable to me; If @BurntSushi thinks that this is a good solution for byteorder, I'll implement it for bincode.

[BurntSushi]

Yeah I've tried hard to think about downsides to this. Changing the public API of a library based on the compiler version makes me very nervous, but I'm having a hard time finding concrete examples to justify it. One thing that pops to mind is viewing the docs for a library which contain new API items because the docs were generated with a newer compiler, while developing on an older compiler and being surprised when a public API item isn't available. AIUI, I think the first error message they'll see is "some_thing is not available" rather than "some_thing uses u128 which isn't available on your Rust 1.22 compiler." So I could see the failure modes being pretty mystifying here. I don't know how heavily to weight them though.

Since this is going to impact foundational crates, I'd feel more comfortable if we got more eyes on this issue. I opened an internals thread: https://internals.rust-lang.org/t/trade-offs-for-adding-new-apis-based-on-compiler-version/8549

[dhardy]

Could we please move forward with this? There has been no further discussion in the internals thread since October.

I recognise the doc problem; probably the best solution is to mention the Rustc version requirement in the doc (which already happens for the std API doc; unfortunately the stable attribute cannot be used outside the standard library, so you'll just have to use words).

[BurntSushi]

@dhardy Oh, sorry, I had assumed that my comment here meant that I was good with version sniffing and automatically adding u128 to the public API. I don't know when exactly I'd get around to actually doing it, but it seems like it should be easy to copy the version sniffing from regex and add a new cfg knob.

[dhardy]

It should be even easier than that; please see this PR: rust-random/rand#664

@TyOverby would you like to prepare this for both crates? Otherwise I can make a PR.

[BurntSushi]

Interesting. I linked to the way I did it in regex to specifically avoid adding a new crate dep, but I hadn't seen autocfg before. Its README inspires confidence. @dtolnay As someone who I know has been fairly fastidious about adding deps for detecting rustc versions, what do you think about autocfg?

[dtolnay]

I do basically the way regex does it (for example here in serde and here in syn). But I like autocfg and wouldn't mind if it takes off.

Whether you use it will depend on whether you want to be an early adopter (where people might notice the extra 1 second compile time or decide not to use your crate because managing two external crates in their non-Cargo build environment isn't worth it) or a late adopter (by which time every nontrivial project already pulls in autocfg anyway).

[BurntSushi]

@dtolnay Thanks for your thoughts! That's a good point about early/late adopter. For byteorder, it's probably a good idea to be a late adopter. Although, I could see using it for regex since it already has several external crate dependencies.

[BurntSushi]

byteorder 1.3.0 now automatically enables i128 support for Rust >= 1.26.