Remove unnecessary unstable features from libsyntax (and all transitive dependencies) to allow rustfmt to work on stable and enable the deprecation of syntex

[bstrie]

rustfmt needs to parse Rust code. Its options are libsyntax and syntex. @erickt (and everyone else) wants to deprecate syntex now that it's no longer needed for Serde. But libsyntax uses unstable features, which means that it will make people require a nightly compiler in the same vein that clippy currently does (i.e. will need to have one installed, but will not infect user code with nightliness). One option is to just ship rustfmt officially and not care that it can't be built on stable, in the same vein as libstd. But @ubsan would prefer to get libsyntax running on stable, and she believes that it would not be prohibitively difficult to do so.

Excerpted conversation for reference:

13:13 < ubsan> #![feature(associated_consts)] #![feature(const_fn)]
               #![feature(optin_builtin_traits)] #![feature(rustc_private)]
               #![feature(staged_api)] #![feature(str_escape)]
               #![feature(unicode)] #![feature(rustc_diagnostic_macros)]
               #![feature(specialization)] #![feature(i128_type)]
13:13 < ubsan> I have no idea why most of those are being used
13:14 <&mbrubeck> library features like `rustc_private` feel like a different
                  category than language features like `associated_consts`
13:14 < ubsan> yeah
13:14 < ubsan> to me, it seems like the only one syntax should be using is i128
13:14 < ubsan> it seems... very strange... to have the rest of those
13:15 < ubsan> I don't even know what 3 of them do
13:15 <&mbrubeck> I can't see where `const fn` is used... at least it doesn't
                  seem to define any const fns
13:15 < ubsan> and why does syntax need associated consts, const fns, and
               optin-builtin-traits?
13:16 < bstrie> I don't suppose feature attributes warn when features aren't
                actually being used?
13:16 < bstrie> seems like that might be hard to determine
13:16   mbrubeck has just been doing this same thing with Servo:
                 https://github.com/servo/servo/issues/5286
13:17 <&mbrubeck> bstrie: There's a lint, yes. Don't know whether it's 100%
                  reliable or not.
13:17 <&mbrubeck> In fact, it must not be because one of the feature gates
                  removed here was unused but didn't warn:
                  https://github.com/servo/servo/pull/16681
13:18 < ubsan> it seems to me that if we remove all of the features of
               libsyntax except for the `rustc_` ones
13:19 < ubsan> and then featurize being a rustc crate
13:19 < ubsan> you could literally just have syntex be syntax
13:19 < bstrie> that would be nice
13:19 < ubsan> looks like associated_consts is used in one place
13:20 < ubsan> https://github.com/rust-lang/rust/blob/master/src/libsyntax/parse/parser.rs#L66
13:20 < bstrie> ubsan: we'd also need to remove all the feature gates on all
                the crates that libsyntax transitively uses
13:20 < ubsan> bstrie: that's true
13:21 < ubsan> I can't imagine it's very hard though
13:21 < ubsan> where's bitflags! defined?
13:21 < bstrie> probably not using the crates.io bitflags crate, eh?
13:21 < ubsan> nope
13:21 < ubsan> that's the other thing
13:22 < ubsan> I feel like rustc should use crates.io crates when possible
13:22 < bstrie> well that ability is relatively new, it's no wonder that it's
                not
13:22 <&mbrubeck> heh, rustc is using both bitflags and rustc_bitflags
13:22 < ubsan> as opposed to being subtly different
13:22 < ubsan> mbrubeck: lel
13:23 <&mbrubeck> oh, because bitflags is in the dependency graph of things
                  like mdbook and pulldown-cmark
13:23 <&mbrubeck> so possibly not actually used in rustc
13:23 <&mbrubeck> (I'm just searching through Cargo.lock)
13:25 < eddyb> ubsan: I was actually supposed to look into making that suck less
13:27 < ubsan> eddyb: make associated consts stable pls
13:28 < eddyb> ubsan: https://github.com/rust-lang/rust/issues/27812
13:59 < ubsan> bstrie: so my opinion is
13:59 < ubsan> removing the features should be easy
13:59 < ubsan> it takes maybe a few hours of work
13:59 < ubsan> then nobody would really *need* to keep syntex up to date
14:00 < ubsan> (except, perhaps, to replace the i/u128 type, but that should be
               an easy bit of work that doesn't need to be changed each time)
14:03 < eddyb> ubsan: so to stage i128
14:03 < eddyb> ubsan: we had a rustc_i128 crate that had fake ones
14:03 < eddyb> I think they were 64-bit instead but I'm not sure :P
14:04 < ubsan> eddyb: I mean, you can implement them with 64-bit types
14:04 < eddyb> hmm
14:04 < ubsan> the only issue is `as` casts
14:04 < eddyb> ubsan: you don't need all the ops, right?
14:04 < eddyb> heh
14:05 < ubsan> I mean, the thing is, the only op you can't get is as casts
14:05 < ubsan> (which is another reason why `as` is terrible *cough*)
14:06 < nagisa> I proposed to stabilise i128 on the tracking issue
14:06 < ubsan> that works too
15:35 < bstrie> erickt: in case you haven't been following the above saga,
                ubsan posits that it might be easier to remove the unstable
                features from libsyntax than to continue maintaining syntex for
                rustfmt's sake
15:36 <&erickt> bstrie: I'd love it if libsyntax removed unstable features
15:37 <&erickt> I did a pass on that, oh, two years ago?
15:37 <&erickt> got rid of some of em
15:38 < ubsan> erickt: there's only two features that really need to be worked
               on, left
15:38 < ubsan> besides the rustc specific ones
15:38 < ubsan> (which can be put under a feature gate)
15:39 < ubsan> erickt: i128, and unicode
15:53 < bstrie> < ubsan> it seems to me that if we remove all of the features
               of libsyntax except for the `rustc_` ones and then featurize being a rustc crate
15:53 < bstrie> ubsan: can you elaborate on ^
15:56 < ubsan> bstrie: like, a crate feature
15:59 < ubsan> #[cfg_attr(feature = "rustc", feature(rustc_doop))]

TL;DR: stabilize "i128" and "unicode", make the "rustc" features conditional on being used in rustc, and remove everything else. Should take "maybe a few hours of work". :P

[strega-nil]

ping #41729

[bstrie]

Tracking issue for stabilizing i128: #35118

[nrc]

I'm also in favour of this solution, we were independently discussing it on twitter last night :-)

The other major Syntex client is bindgen. I don't think forcing Rustfmt and Bindgen into the rustup distro model (necessary for stability) is a good idea, really.

IIRC actually making libsyntax stable is a bigger job than we might think, perhaps @erickt or @dtolnay could give estimates of how much work, but a few hours seems wildly on the low side.

Nominating for compiler team meeting.

[strega-nil]

@nrc maybe not a couple of hours, but I believe someone could do it within a week, given stable u/i128.

[kennytm]

Could i128 simply be cfg'ed out when not compiling in nightly?

[strega-nil]

@kennytm you'd have to have some sort of replacement, which means using a struct, which means as casts would have to be flipped over to, like, .into(), probably.

[strega-nil]

It wouldn't be a huge deal, but it would be a deal.

[kennytm]

@ubsan Is it possible to use the rustc_i128 approach i.e. type u128 = u64; when it is not supported?

(Basically I wouldn't like to see i128 rushed to stablization by this, given that AtomicU128 and FFI are still not quite solved, and I don't like to see this being blocked by i128.)

[strega-nil]

@kennytm that seems... bad?

[kennytm]

@ubsan Sufficient if the crippled libsyntax won't touch 128-bit literals 😄. The 128-bit types are used solely for LitKind::Int.

If a custom structure is needed, it only needs to provide u128::from_str_radix, impl Display, and impl From<u64> (and all primitives).

---

The dependencies of libsyntax is in a worse shape though, in particular rustc_data_structures (this should be swapped to std::collections or other crates using a Cargo feature?).

[dependencies]
serialize = { path = "../libserialize" }
log = "0.3"
rustc_bitflags = { path = "../librustc_bitflags" }
syntax_pos = { path = "../libsyntax_pos" }
rustc_errors = { path = "../librustc_errors" }
rustc_data_structures = { path = "../librustc_data_structures" }

Ignoring rustc_bitflags, we have:

rust/src/libsyntax_pos$ git grep '\[feature'
lib.rs:26:#![feature(const_fn)]
lib.rs:27:#![feature(custom_attribute)]
lib.rs:28:#![feature(optin_builtin_traits)]
lib.rs:30:#![feature(rustc_private)]
lib.rs:31:#![feature(staged_api)]
lib.rs:32:#![feature(specialization)]

rust/src/librustc_error$ git grep '\[feature'
lib.rs:20:#![feature(custom_attribute)]
lib.rs:22:#![feature(rustc_private)]
lib.rs:23:#![feature(staged_api)]
lib.rs:24:#![feature(range_contains)]
lib.rs:25:#![feature(libc)]

rust/src/librustc_data_structures$ git grep '\[feature'
lib.rs:28:#![feature(shared)]
lib.rs:29:#![feature(collections_range)]
lib.rs:30:#![feature(nonzero)]
lib.rs:31:#![feature(rustc_private)]
lib.rs:32:#![feature(staged_api)]
lib.rs:33:#![feature(unboxed_closures)]
lib.rs:34:#![feature(fn_traits)]
lib.rs:35:#![feature(untagged_unions)]
lib.rs:36:#![feature(associated_consts)]
lib.rs:37:#![feature(unsize)]
lib.rs:38:#![feature(i128_type)]
lib.rs:39:#![feature(conservative_impl_trait)]
lib.rs:40:#![feature(discriminant_value)]
lib.rs:41:#![feature(specialization)]
lib.rs:42:#![feature(manually_drop)]
lib.rs:43:#![feature(struct_field_attributes)]

rust/src/libsyntax$ git grep rustc_data_structures
Cargo.toml:17:rustc_data_structures = { path = "../librustc_data_structures" }
ast.rs:26:use rustc_data_structures::indexed_vec;
lib.rs:44:extern crate rustc_data_structures;
ptr.rs:46:use rustc_data_structures::stable_hasher::{StableHasher, StableHasherResult,
util/rc_slice.rs:15:use rustc_data_structures::stable_hasher::{StableHasher, StableHasherResult,
util/small_vector.rs:11:use rustc_data_structures::small_vec::SmallVec;

[pmarcelll]

racer also uses syntex, so it might be important for rls.

[nrc]

This is no longer needed - we're using rustup for rustfmt.