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_feature::Features: explain what that 'Option<Symbol>' is about #132098

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

RalfJung
Copy link
Member

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Oct 24, 2024

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added 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. labels Oct 24, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Oct 24, 2024

Question: is it worth it to introduce a struct with named fields instead of a 3-tuple? The comments are good -- but the callsite would still see the 3-tuple only.

@RalfJung
Copy link
Member Author

RalfJung commented Oct 24, 2024 via email

/// Returns a list of triples with:
/// - feature gate name
/// - the span of the `#[feature]` attribute
/// - (for already stable features) the version since which it is stable
pub fn enabled_lang_features(&self) -> &Vec<(Symbol, Span, Option<Symbol>)> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion: not for this PR, but like

pub struct LangFeature {
    name: Symbol,
    attr_sp: Span,
    stable_since: Option<Symbol>,
}

pub fn enabled_lang_features(&self) -> &Vec<LangFeature> { ... }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type should probably be called EnabledLangFeature, but other than that -- yeah makes sense.

@jieyouxu
Copy link
Member

jieyouxu commented Oct 24, 2024

I'll see if it makes sense to package them into a struct in a follow-up.

@jieyouxu
Copy link
Member

Thanks for the docs!

r? jieyouxu
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 24, 2024

📌 Commit 282f291 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 24, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#129248 (Taking a raw ref (`&raw (const|mut)`) of a deref of pointer (`*ptr`) is always safe)
 - rust-lang#131906 (rustdoc: adjust spacing and typography in header)
 - rust-lang#132084 (Consider param-env candidates even if they have errors)
 - rust-lang#132096 (Replace an FTP link in comments with an equivalent HTTPS link)
 - rust-lang#132098 (rustc_feature::Features: explain what that 'Option<Symbol>' is about)

r? `@ghost`
`@rustbot` modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2024
Rollup merge of rust-lang#132098 - RalfJung:features-since, r=jieyouxu

rustc_feature::Features: explain what that 'Option<Symbol>' is about
@bors bors merged commit 7c22f47 into rust-lang:master Oct 24, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 24, 2024
@RalfJung RalfJung deleted the features-since branch October 25, 2024 08:50
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 26, 2024
Use `Enabled{Lang,Lib}Feature`  instead of n-tuples

Instead of passing around e.g. `(gate_name, attr_span, stable_since)` 3-tuples for enabled lang features or `(gate_name, attr_span)` 2-tuples for enabled lib features, use `Enabled{Lang,Lib}Feature` structs with named fields.

Also did some minor code-golfing of involved iterator chains to hopefully make them easier to follow.

Follow-up to rust-lang#132098 (comment) cc `@RalfJung.`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 26, 2024
Rollup merge of rust-lang#132114 - jieyouxu:features-bundle, r=fee1-dead

Use `Enabled{Lang,Lib}Feature`  instead of n-tuples

Instead of passing around e.g. `(gate_name, attr_span, stable_since)` 3-tuples for enabled lang features or `(gate_name, attr_span)` 2-tuples for enabled lib features, use `Enabled{Lang,Lib}Feature` structs with named fields.

Also did some minor code-golfing of involved iterator chains to hopefully make them easier to follow.

Follow-up to rust-lang#132098 (comment) cc `@RalfJung.`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

5 participants