Skip to content
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

rustc: Disallow unstable flags in stable releases #31731

Closed

Conversation

alexcrichton
Copy link
Member

This commit disallows passing recognized unstable flags to the compiler on the
stable/beta release channels. This includes options like -Z, --error-format,
--pretty, and --unpretty. Additionally, this removes interpreting the
-Z unstable-options flag as this is now just based on the release channel. The
parsing is left in for now to prevent scripts from immediately breaking.

This commit disallows passing recognized unstable flags to the compiler on the
stable/beta release channels. This includes options like `-Z`, `--error-format`,
`--pretty`, and `--unpretty`. Additionally, this removes interpreting the
`-Z unstable-options` flag as this is now just based on the release channel. The
parsing is left in for now to prevent scripts from immediately breaking.
@rust-highfive
Copy link
Collaborator

r? @Aatch

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

I've personally been perpetually bothered by the fact that we accept -Z flags and other unstable flags on stable releases, and I'd like to be a bit more proactive about gating usage of these flags on the stable channel. I believe it was always the intention that unstable flags were only allowed within the nightly release (similar to nightly only features), and this is just following through on those intentions.

This removes some of the extra support around -Z unstable-options, specifically a specially crafted error message that flags are not recognized because this is a stable compiler. I'd be fine adding the support back in, but I personally felt at least that we probably didn't need to do that.

r? @brson
cc @rust-lang/tools
cc @rust-lang/compiler

@rust-highfive rust-highfive assigned brson and unassigned Aatch Feb 17, 2016
@nagisa
Copy link
Member

nagisa commented Feb 17, 2016

Some -Z flags are very useful and I don’t see their behaviour being unstable at all. no-trans and time-passes and verbose and print-llvm-passes and ast-json and no-analysis all come to mind as those one would like to keep working regardless of channel.

EDIT: namely people (and, I think, cargo?) rely on no-trans/no-analysis for faster compile-edit cycle, and having everything else is useful for debugging issues with stable rustc (once the flags go away we can’t just ask people to provide output of -Z something).

@steveklabnik
Copy link
Member

"is it useful" is very different than "is it stable" though.

@vadimcn
Copy link
Contributor

vadimcn commented Feb 17, 2016 via email

@alexcrichton
Copy link
Member Author

@nagisa yes as @steveklabnik mentioned something being useful is very different than it being stable. This is basically the same argument for why we have feature gates that are only allowed on nightly compilers, a decision which has long since passed. I also find some unstable APIs useful from time to time, but that doesn't mean they should be available on stable compilers.

@vadimcn true, but --pretty I believe is somewhat broken in many other ways as it's got tons of bugs open on it. Perhaps rustfmt may eventually add an expanded mode one day, too, but the main principle here is that the flag is indeed unstable and shouldn't be present on stable compilers.

@durka
Copy link
Contributor

durka commented Feb 17, 2016

These flags are very useful, you already have to do a Simon Says with -Z unstable-options to use them so the expectation of stability is gone. I think this will just push more people (all macro authors, for example) to nightly. And as said it harms debugging, for one example if you can't trace macros on stable then you can't debug a macro expansion regression.

However, @nagisa luckily no-trans, time-passes, etc do not require -Z unstable-options so I would hope this PR is leaving them in?

@brson
Copy link
Contributor

brson commented Feb 17, 2016

It looks like this will make rustc pretend like unstable options don't exist at all on nightly, reporting unrecognized options. To be clearer and more consistent with other stability features I'd rather it report that the option can't be used on the channel because it is unstable.

I want to do this, but it needs a lot of consideration since it's a major policy change and breaking change.

Edit: I also feel like we haven't been very careful about deciding which options are stable and unstable, and if we did this we would probably want to reconsider each one.

Edit 2: Also, there's a very strong case here that the ship has sailed and we can't do this. This is a seriously breaking change.

@aturon
Copy link
Member

aturon commented Feb 17, 2016

cc @rust-lang/core

@pnkfelix
Copy link
Member

I'm not in favor of doing this. The need to opt in with -Z unstable-options seems like sufficient warning; what concrete benefit is expected here?

@nikomatsakis
Copy link
Contributor

It seems to me that the reason to "gate" access to these APIs is the same
as the reason to "gate" access to any other unstable thing: to avoid
implicit dependencies (precisely the kind of dependencies that are arising
even as we speak, e.g., cargo check relies on -Z no-trans. However, we
can't just yank the rug out from people -- like any other bug fix, we
should do this gradually, and offer stable alternatives for the most
popular options (and perhaps keep the current -Z variants as deprecated
aliases).

(Also, we discussed in the core team meeting today, where we said roughly
the same thing.)

On Wed, Feb 17, 2016 at 3:42 PM, Felix S Klock II notifications@github.com
wrote:

I'm not in favor of doing this. The need to opt in with -Z
unstable-options seems like sufficient warning; what concrete benefit is
expected here?


Reply to this email directly or view it on GitHub
#31731 (comment).

@nrc
Copy link
Member

nrc commented Feb 18, 2016

I'm strongly in favour of doing this - unstable options should not be available on release, it defeats the point of them being unstable options, really. Some other points from the thread:

  • agree about having warning cycles
  • the concrete benefit is the same as feature-gating lang features or libs - we get to experiment with features without worrying about de facto standardisation and lock-in
  • we should only make unstable options unstable, not all -Z options (no-trans in particular seems totally fine to leave as is), but...
  • agree we should probably make some kind of formal decision around stability.
  • I don't think the ship has sailed, worst case some more people end up using nightly, which I don't think is too bad.
  • --pretty is terrible and broken. I have a student working on better tools for macro authors, although there is unlikely to be anything usable until later in the year.

@nikomatsakis
Copy link
Contributor

On Thu, Feb 18, 2016 at 03:51:47PM -0800, Nick Cameron wrote:

  • we should only make unstable options unstable, not all -Z options (no-trans in particular seems totally fine to leave as is), but...

I agree that there is no partcular reason to break existing scripts,
but I'd like to deprecate the -Z form for stable options. I think
the idea of -Z was that it served a role like #[feature] -- if you
are using nightly but wnt to ensure that your command line is only
using stable options, you just have to look for -Z. So we should add
something like -C typecheck (instead of -Z no-trans), but just
keep -Z no-trans around as a deprecated alias (maybe not forever,
though it doesn't cost much).

@alexcrichton
Copy link
Member Author

Yes we discussed this at the core team meeting and reached a few conclusions. For now I've opened #31793 to "stem the bleeding" so we can ensure that all future unstable options are actually unstable. That PR also adds a warning to indicate that -Z and other unstable options will eventually be a hard error on the stable channel.

As a result, I'm going to close this PR in favor of that one for now.

@alexcrichton alexcrichton deleted the lets-gate-those-z-options branch February 20, 2016 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.