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

Allow library features to be marked internal as well #115623

Closed

Conversation

compiler-errors
Copy link
Member

allows unstable(..., is_internal, ...) to mark a library feature as internal. Not sure if we should just have a different stability attribute, like #[internal(...)] with the same fields as unstable.

Part of #115597.

This does no modification to the standard library yet.

@rustbot
Copy link
Collaborator

rustbot commented Sep 6, 2023

r? @cjgillot

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

@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 Sep 6, 2023
desc { "calculating the lib features defined in a crate" }
separate_provide_extern
arena_cache
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I consolidated this into one query -- it's not necessarily a requirement of this PR to do so, so if that's not desirable, I could revert it.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is useful. Could you extract it in a standalone query? Or even a standalone PR, as there may be discussion on this PR.

@rust-log-analyzer

This comment has been minimized.

@petrochenkov
Copy link
Contributor

Double-gating internal features was a mistake by itself, and now other unnecessary complexity is accumulating around that mistake.

@@ -1027,7 +1030,7 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {

// We always collect the lib features declared in the current crate, even if there are
// no unknown features, because the collection also does feature attribute validation.
let local_defined_features = tcx.lib_features(()).to_vec();
let local_defined_features = tcx.lib_features(rustc_hir::def_id::LOCAL_CRATE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please import this.

desc { "calculating the lib features defined in a crate" }
separate_provide_extern
arena_cache
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is useful. Could you extract it in a standalone query? Or even a standalone PR, as there may be discussion on this PR.

}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should all this method go in rustc_passes::stability::check_unused_or_stable_features?

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

I was thinking we could do this a lot simpler, and just trigger the lint for all features whose name ends in _internal or _internals. That should also work pretty well, shouldn't it? We'll have to rename core_intrinsics to core_intrinsics_internal, but other than that I think the internal library features already follow this convention.

@compiler-errors
Copy link
Member Author

I kinda prefer the more general solution, but yeah, I'm happy doing it that way too.

@bors
Copy link
Contributor

bors commented Sep 28, 2023

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

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 21, 2023
…gillot

Unify `defined_lib_features` and `lib_features` queries

Extracts part of rust-lang#115623 (comment)

I went with also introducing a `FeatureStability` enum, instead of using `Some(span)` to mean stable and `None` to mean unstable.

r? `@cjgillot`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 5, 2023
…compiler-errors

Add support for making lib features internal

We have the notion of an "internal" lang feature: a feature that is never intended to be stabilized, and using which can cause ICEs and other issues without that being considered a bug.

This extends that idea to lib features as well. It is an alternative to rust-lang#115623: instead of using an attribute to declare lib features internal, we simply do this based on the name. Everything ending in `_internals` or `_internal` is considered internal.

Then we rename `core_intrinsics` to `core_intrinsics_internal`, which fixes rust-lang#115597.
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Dec 5, 2023
…compiler-errors

Add support for making lib features internal

We have the notion of an "internal" lang feature: a feature that is never intended to be stabilized, and using which can cause ICEs and other issues without that being considered a bug.

This extends that idea to lib features as well. It is an alternative to rust-lang#115623: instead of using an attribute to declare lib features internal, we simply do this based on the name. Everything ending in `_internals` or `_internal` is considered internal.

Then we rename `core_intrinsics` to `core_intrinsics_internal`, which fixes rust-lang#115597.
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Dec 5, 2023
…compiler-errors

Add support for making lib features internal

We have the notion of an "internal" lang feature: a feature that is never intended to be stabilized, and using which can cause ICEs and other issues without that being considered a bug.

This extends that idea to lib features as well. It is an alternative to rust-lang#115623: instead of using an attribute to declare lib features internal, we simply do this based on the name. Everything ending in `_internals` or `_internal` is considered internal.

Then we rename `core_intrinsics` to `core_intrinsics_internal`, which fixes rust-lang#115597.
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Dec 5, 2023
…compiler-errors

Add support for making lib features internal

We have the notion of an "internal" lang feature: a feature that is never intended to be stabilized, and using which can cause ICEs and other issues without that being considered a bug.

This extends that idea to lib features as well. It is an alternative to rust-lang#115623: instead of using an attribute to declare lib features internal, we simply do this based on the name. Everything ending in `_internals` or `_internal` is considered internal.

Then we rename `core_intrinsics` to `core_intrinsics_internal`, which fixes rust-lang#115597.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 6, 2023
Rollup merge of rust-lang#118123 - RalfJung:internal-lib-features, r=compiler-errors

Add support for making lib features internal

We have the notion of an "internal" lang feature: a feature that is never intended to be stabilized, and using which can cause ICEs and other issues without that being considered a bug.

This extends that idea to lib features as well. It is an alternative to rust-lang#115623: instead of using an attribute to declare lib features internal, we simply do this based on the name. Everything ending in `_internals` or `_internal` is considered internal.

Then we rename `core_intrinsics` to `core_intrinsics_internal`, which fixes rust-lang#115597.
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Dec 12, 2023
…errors

Add support for making lib features internal

We have the notion of an "internal" lang feature: a feature that is never intended to be stabilized, and using which can cause ICEs and other issues without that being considered a bug.

This extends that idea to lib features as well. It is an alternative to rust-lang/rust#115623: instead of using an attribute to declare lib features internal, we simply do this based on the name. Everything ending in `_internals` or `_internal` is considered internal.

Then we rename `core_intrinsics` to `core_intrinsics_internal`, which fixes rust-lang/rust#115597.
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
…errors

Add support for making lib features internal

We have the notion of an "internal" lang feature: a feature that is never intended to be stabilized, and using which can cause ICEs and other issues without that being considered a bug.

This extends that idea to lib features as well. It is an alternative to rust-lang/rust#115623: instead of using an attribute to declare lib features internal, we simply do this based on the name. Everything ending in `_internals` or `_internal` is considered internal.

Then we rename `core_intrinsics` to `core_intrinsics_internal`, which fixes rust-lang/rust#115597.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
…errors

Add support for making lib features internal

We have the notion of an "internal" lang feature: a feature that is never intended to be stabilized, and using which can cause ICEs and other issues without that being considered a bug.

This extends that idea to lib features as well. It is an alternative to rust-lang/rust#115623: instead of using an attribute to declare lib features internal, we simply do this based on the name. Everything ending in `_internals` or `_internal` is considered internal.

Then we rename `core_intrinsics` to `core_intrinsics_internal`, which fixes rust-lang/rust#115597.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

7 participants