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

Should the sets of library and language features be disjoint? #43089

Closed
est31 opened this issue Jul 6, 2017 · 4 comments
Closed

Should the sets of library and language features be disjoint? #43089

est31 opened this issue Jul 6, 2017 · 4 comments
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@est31
Copy link
Member

est31 commented Jul 6, 2017

Currently, there is a check in tidy that ensures that there is no library feature and language feature of the same name. Its presence is also mentioned in the documentation above.

This check was present since #21248, and has caused multiple additions that have both library and language components to be split up:

  • i128 / i128_type
  • inclusive_range / inclusive_range_syntax
  • profiler_runtime / profiler_runtime_lib

PR #40939 has challenged the status quo, by reusing the lang feature proc_macro inside libraries provided to programs. It actually added an exception to the tidy check in order to get it passing.

There has been discussion about this in the thread of #40939 and the resolution was to allow proc_macro to share the feature gate.

So my question is: which purpose does this check serve? should the check be present, or should it be abolished? Should the proc_macro feature gate be split up into a library component and a lang component?

@est31 est31 changed the title Should the sets of library and language features be disjunct? Should the sets of library and language features be disjoint? Jul 6, 2017
@durka
Copy link
Contributor

durka commented Jul 6, 2017

IMO we should just kill the tidy check. For inclusive ranges we are stabilizing the lib and syntax separately, but if they had been under the same gate it would have been easy to split them up, like we are doing for const fn.

@nikomatsakis
Copy link
Contributor

I too think we should lump things that are "the same underlying concept" under one feature gate.

@Mark-Simulacrum Mark-Simulacrum added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jul 6, 2017
@aidanhs
Copy link
Member

aidanhs commented Jul 6, 2017

Looks like the original code allowed for a 'joint features' whitelist, so it seems like it might have just been a sanity check that any intersection was intentional. Indeed, that script actually says that it's for doing some sanity checks.

If for some reason that sanity check is still desirable, at minimum I'd be +1 to creating a joint_features whitelist rather than ad-hoc growing the if condition.

@est31
Copy link
Member Author

est31 commented Jul 6, 2017

it seems like it might have just been a sanity check that any intersection was intentional

We can guard from that by doing a check for any joint features whether the tracking issue and stability status (stable or unstable) matches between lib and lang features.

est31 added a commit to est31/rust that referenced this issue Jul 15, 2017
This allows changes to the Rust language that have both library
and language components share one feature gate.

The feature gates need to be "about the same change", so that both
library and language components must either be both unstable, or
both stable, and share the tracking issue.

Removes the ugly "proc_macro" exception.

Closes rust-lang#43089
bors added a commit that referenced this issue Jul 20, 2017
Tidy: allow common lang+lib features

This allows changes to the Rust language that have both library
and language components share one feature gate.

The feature gates need to be "about the same change", so that both
library and language components must either be both unstable, or
both stable, and share the tracking issue.

Removes the ugly "proc_macro" exception added by #40939.

Closes #43089
bors added a commit that referenced this issue Dec 12, 2017
…henkov

Rename never_type_impls gate

We no longer need a separately-named `never_type_impls` gate thanks to #43089.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants