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

--print option to list available feature gates #45223

Closed

Conversation

zackmdavis
Copy link
Member

The option is only made available on nightly builds, for the obvious
reason. While we're here, also prevent the unstable target-spec-json
(tracking issue #38338) from showing up in the --help on stable builds
(although the way in which we accomplish this is slightly clumsy for
lifetime reasons, as noted in the FIXME).

We make REMOVED_FEATURES, ACCEPTED_FEATURES, &c. public for consistency,
even though the functionality at issue only needs ACTIVE_FEATURES.

The UI test will add a little bit of friction for future feature
developers, but it's better than under-testing.

Resolves #38768.

The option is only made available on nightly builds, for the obvious
reason. While we're here, also prevent the unstable `target-spec-json`
(tracking issue rust-lang#38338) from showing up in the --help on stable builds
(although the way in which we accomplish this is slightly clumsy for
lifetime reasons, as noted in the FIXME).

We make REMOVED_FEATURES, ACCEPTED_FEATURES, &c. public for consistency,
even though the functionality at issue only needs ACTIVE_FEATURES.

The UI test will add a little bit of friction for future feature
developers, but it's better than under-testing.

Resolves rust-lang#38768.
@rust-highfive
Copy link
Collaborator

r? @eddyb

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

@eddyb
Copy link
Member

eddyb commented Oct 12, 2017

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned eddyb Oct 12, 2017
@alexcrichton
Copy link
Member

Thanks for the PR! I'm not personally entirely sold on the motivation here as nothing here is stable, so you can't really do feature detection at all. The meaning of a feature foo can change over time and the actual dependency of code is not on a feature foo but rather foo plus a number of bugs being fixed.

@joshtriplett I wonder if may you have some thoughts on this as the original reporter?

@joshtriplett
Copy link
Member

It'd be nice if at least the list of stable features was available in stable...

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 14, 2017
@alexcrichton
Copy link
Member

@zackmdavis do you have thoughts on @joshtriplett's last comment?

@zackmdavis
Copy link
Member Author

I guess I was sort of thinking about this more in terms of, "Given that this nightly compiler knows how to accept feature flags, it should also know how to list them, because that seems like an obvious thing that a user might be curious about that we can answer without forcing them to read the source code."

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 21, 2017
@alexcrichton
Copy link
Member

As I explained above though there's no actual meaning to listing unstable features. The definition of an unstable feature changes over time and so you have to read the source anyway.

@carols10cents
Copy link
Member

@joshtriplett:

It'd be nice if at least the list of stable features was available in stable...

There are stable features...? confused I thought once something was stable, it wasn't a feature anymore....

@alexcrichton are you saying you're against landing this change?

@alexcrichton
Copy link
Member

Yes I am against landing this as-is, personally.

@carols10cents
Copy link
Member

@alexcrichton what changes would make you in favor of landing this?

@joshtriplett
Copy link
Member

@carols10cents It's still a feature, just not a feature-gated one.

@zackmdavis
Copy link
Member Author

As I explained above though there's no actual meaning to listing unstable features.

That sounds a little exaggerated to me: surely "Beware; the behavior or scope of this feature flag could change" (e.g., like how stmt_expr_attributes used be needed for any attributes on non-item statements or expressions, but now only for expressions) isn't the same as "It is literally meaningless to speak of 'the same' feature flag in different nightly compilers"?

I thought once something was stable, it wasn't a feature anymore....

The compiler does know about "accepted" features (and this was put to use in diagnostics at least previously), but I think the case for "It's meaningless to talk about this" is stronger here—after the feature has been accepted, it's indistinguishable from any other part of the language (in contrast to "active", unaccepted features activating some identifiable branches of code, even if that code can change from nightly to nightly).

@kennytm
Copy link
Member

kennytm commented Nov 6, 2017

commented 7 days ago

Hi @alexcrichton @joshtriplett @zackmdavis @carols10cents,

What is the expectation of this PR, should it be closed, or reworded for merging, or be nominated to a team (T-docs?) for further discussion?

@bors
Copy link
Contributor

bors commented Nov 7, 2017

☔ The latest upstream changes (presumably #45666) made this pull request unmergeable. Please resolve the merge conflicts.

@zackmdavis
Copy link
Member Author

@kennytm I think the people already in this thread have said what they have to say; I leave it to your judgment whether to close or summon the docs team for more input.

@kennytm
Copy link
Member

kennytm commented Nov 13, 2017

cc @rust-lang/docs then.

Personally I think this feature is not harmful, as we already have the unstable book, but this is not useful either as the library features (e.g. try_trait) are not listed.

@eddyb
Copy link
Member

eddyb commented Nov 13, 2017

I think we should've long stopped keeping removed feature gates in the compiler.
AFAIK we don't keep stabilized library features around, so why are language features different?
cc @rust-lang/compiler

@shepmaster shepmaster added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 18, 2017
@shepmaster
Copy link
Member

In the interest of moving this forward, I've picked a team and nominated it to be discussed at the next meeting. Y'all are welcome to change the team, of course.

@zackmdavis
Copy link
Member Author

Hm, on second thought, the lack of enthusiasm (this patch doesn't meet the original reporter's satisfaction; I only wrote this patch because it seemed like a cool thing the compiler might as well be able to do; I don't actually need to use it for anything) makes me think we should close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants