Closed
Description
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