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

Proper support for cross-crate recursive const stability checks #132541

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Nov 2, 2024

Stacked on top of #132492; only the last three commits are new.

In a crate without staged_api but with -Zforce-unstable-if-unmarked, we now subject all functions marked with #[rustc_const_stable_indirect] to recursive const stability checks. We require an opt-in so that by default, a crate can be built with -Zforce-unstable-if-unmarked and use nightly features as usual. This property is recorded in the crate metadata so when a staged_api crate calls such a function, it sees the #[rustc_const_stable_indirect] and allows it to be exposed on stable. This, finally, will let us expose const fn from hashbrown on stable.

The second commit makes const stability more like regular stability: via check_missing_const_stability, we ensure that all publicly reachable functions have a const stability attribute -- both in staged_api crates and -Zforce-unstable-if-unmarked crates. To achieve this, we move around the stability computation so that const stability is computed after regular stability is done. This lets us access the final result of the regular stability computation, which we use so that const fn can inherit the regular stability (but only if that is "unstable"). Fortunately, this lets us get rid of an Option in ConstStability.

This is the last PR that I have planned in this series.

r? @compiler-errors

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 2, 2024
// `#[rustc_const_stable_indirect]` also requires a const fn
#[rustc_const_stable_indirect]
#[unstable(feature = "unstable", issue = "none")]
pub fn not_a_const_fn() {}
Copy link
Member Author

Choose a reason for hiding this comment

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

This regresses complaining about rustc_const_stable_indirect on non-const-fn... but keeping that error would make the code even more messy, and we also don't error for rustc_promotable on non-const-fn, so 🤷

There's an ongoing effort to redo the way we do attributes, that should hopefully make this easier in the future.

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Nov 2, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 2, 2024

That's a tricky one... when one force-unstable-if-unmarked crate calls things from another one, how does that work? The callee crate has everything under the rustc_private feature gate, which the calling crate does not have enabled. Somehow this works for regular stability; we'll have to use the same logic for const stability.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Nov 3, 2024

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

@bors
Copy link
Contributor

bors commented Nov 4, 2024

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

@bors
Copy link
Contributor

bors commented Nov 4, 2024

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

@RalfJung
Copy link
Member Author

RalfJung commented Nov 5, 2024

The base PR landed, so this is ready to go now. :)

@rustbot
Copy link
Collaborator

rustbot commented Nov 9, 2024

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

@bors
Copy link
Contributor

bors commented Nov 10, 2024

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

@@ -53,10 +53,11 @@ impl<'mir, 'tcx> ConstCx<'mir, 'tcx> {
}

pub fn enforce_recursive_const_stability(&self) -> bool {
// We can skip this if `staged_api` is not enabled, since in such crates
// `lookup_const_stability` will always be `None`.
// We can skip this if neither `staged_api` nor `-Zforrce-unstable-if-unmarked` are enabled,
Copy link
Member

Choose a reason for hiding this comment

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

typo on the arg

@compiler-errors
Copy link
Member

r=me after really tiny nit

@RalfJung
Copy link
Member Author

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Nov 12, 2024

📌 Commit 3780496 has been approved by compiler-errors

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 Nov 12, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 12, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#131831 (extend the "if-unchanged" logic for compiler builds)
 - rust-lang#132541 (Proper support for cross-crate recursive const stability checks)
 - rust-lang#132657 (AIX: add run-make support)
 - rust-lang#132901 (Warn about invalid `mir-enable-passes` pass names)
 - rust-lang#132923 (Triagebot: Consolidate the T-compiler ad hoc assignment groups)
 - rust-lang#132938 (Make precise capturing suggestion machine-applicable only if it has no APITs)
 - rust-lang#132947 (clarify `must_produce_diag` ICE for debugging)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4a699fc into rust-lang:master Nov 12, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 12, 2024
Rollup merge of rust-lang#132541 - RalfJung:const-stable-extern-crate, r=compiler-errors

Proper support for cross-crate recursive const stability checks

~~Stacked on top of rust-lang#132492; only the last three commits are new.~~

In a crate without `staged_api` but with `-Zforce-unstable-if-unmarked`, we now subject all functions marked with `#[rustc_const_stable_indirect]` to recursive const stability checks. We require an opt-in so that by default, a crate can be built with `-Zforce-unstable-if-unmarked` and use nightly features as usual. This property is recorded in the crate metadata so when a `staged_api` crate calls such a function, it sees the `#[rustc_const_stable_indirect]` and allows it to be exposed on stable. This, finally, will let us expose `const fn` from hashbrown on stable.

The second commit makes const stability more like regular stability: via `check_missing_const_stability`, we ensure that all publicly reachable functions have a const stability attribute -- both in  `staged_api` crates and `-Zforce-unstable-if-unmarked` crates. To achieve this, we move around the stability computation so that const stability is computed after regular stability is done. This lets us access the final result of the regular stability computation, which we use so that `const fn` can inherit the regular stability (but only if that is "unstable"). Fortunately, this lets us get rid of an `Option` in `ConstStability`.

This is the last PR that I have planned in this series.

r? `@compiler-errors`
@RalfJung RalfJung deleted the const-stable-extern-crate branch November 13, 2024 06:10
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 14, 2024
…, r=compiler-errors

Proper support for cross-crate recursive const stability checks

~~Stacked on top of rust-lang#132492; only the last three commits are new.~~

In a crate without `staged_api` but with `-Zforce-unstable-if-unmarked`, we now subject all functions marked with `#[rustc_const_stable_indirect]` to recursive const stability checks. We require an opt-in so that by default, a crate can be built with `-Zforce-unstable-if-unmarked` and use nightly features as usual. This property is recorded in the crate metadata so when a `staged_api` crate calls such a function, it sees the `#[rustc_const_stable_indirect]` and allows it to be exposed on stable. This, finally, will let us expose `const fn` from hashbrown on stable.

The second commit makes const stability more like regular stability: via `check_missing_const_stability`, we ensure that all publicly reachable functions have a const stability attribute -- both in  `staged_api` crates and `-Zforce-unstable-if-unmarked` crates. To achieve this, we move around the stability computation so that const stability is computed after regular stability is done. This lets us access the final result of the regular stability computation, which we use so that `const fn` can inherit the regular stability (but only if that is "unstable"). Fortunately, this lets us get rid of an `Option` in `ConstStability`.

This is the last PR that I have planned in this series.

r? `@compiler-errors`
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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc 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