Skip to content

Commit

Permalink
Unrolled build for rust-lang#132541
Browse files Browse the repository at this point in the history 
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`
  • Loading branch information
@rust-timer
rust-timer authored Nov 12, 2024
2 parents 6503543 + 3780496 commit 597256c
Show file tree
Hide file tree
Showing 18 changed files with 340 additions and 205 deletions.
68 changes: 32 additions & 36 deletions compiler/rustc_attr/src/builtin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ use rustc_session::lint::BuiltinLintDiag;
use rustc_session::lint::builtin::UNEXPECTED_CFGS;
use rustc_session::parse::feature_err;
use rustc_session::{RustcVersion, Session};
use rustc_span::Span;
use rustc_span::hygiene::Transparency;
use rustc_span::symbol::{Symbol, kw, sym};
use rustc_span::{DUMMY_SP, Span};

use crate::fluent_generated;
use crate::session_diagnostics::{self, IncorrectReprFormatGenericCause};
Expand Down Expand Up @@ -92,9 +92,7 @@ impl Stability {
#[derive(HashStable_Generic)]
pub struct ConstStability {
pub level: StabilityLevel,
/// This can be `None` for functions that do not have an explicit const feature.
/// We still track them for recursive const stability checks.
pub feature: Option<Symbol>,
pub feature: Symbol,
/// This is true iff the `const_stable_indirect` attribute is present.
pub const_stable_indirect: bool,
/// whether the function has a `#[rustc_promotable]` attribute
Expand Down Expand Up @@ -272,22 +270,19 @@ pub fn find_stability(

/// Collects stability info from `rustc_const_stable`/`rustc_const_unstable`/`rustc_promotable`
/// attributes in `attrs`. Returns `None` if no stability attributes are found.
///
/// `is_const_fn` indicates whether this is a function marked as `const`.
pub fn find_const_stability(
sess: &Session,
attrs: &[Attribute],
item_sp: Span,
is_const_fn: bool,
) -> Option<(ConstStability, Span)> {
let mut const_stab: Option<(ConstStability, Span)> = None;
let mut promotable = false;
let mut const_stable_indirect = None;
let mut const_stable_indirect = false;

for attr in attrs {
match attr.name_or_empty() {
sym::rustc_promotable => promotable = true,
sym::rustc_const_stable_indirect => const_stable_indirect = Some(attr.span),
sym::rustc_const_stable_indirect => const_stable_indirect = true,
sym::rustc_const_unstable => {
if const_stab.is_some() {
sess.dcx()
Expand All @@ -299,7 +294,7 @@ pub fn find_const_stability(
const_stab = Some((
ConstStability {
level,
feature: Some(feature),
feature,
const_stable_indirect: false,
promotable: false,
},
Expand All @@ -317,7 +312,7 @@ pub fn find_const_stability(
const_stab = Some((
ConstStability {
level,
feature: Some(feature),
feature,
const_stable_indirect: false,
promotable: false,
},
Expand All @@ -340,7 +335,7 @@ pub fn find_const_stability(
}
}
}
if const_stable_indirect.is_some() {
if const_stable_indirect {
match &mut const_stab {
Some((stab, _)) => {
if stab.is_const_unstable() {
Expand All @@ -351,36 +346,37 @@ pub fn find_const_stability(
})
}
}
_ => {}
_ => {
// This function has no const stability attribute, but has `const_stable_indirect`.
// We ignore that; unmarked functions are subject to recursive const stability
// checks by default so we do carry out the user's intent.
}
}
}
// Make sure if `const_stable_indirect` is present, that is recorded. Also make sure all `const
// fn` get *some* marker, since we are a staged_api crate and therefore will do recursive const
// stability checks for them. We need to do this because the default for whether an unmarked
// function enforces recursive stability differs between staged-api crates and force-unmarked
// crates: in force-unmarked crates, only functions *explicitly* marked `const_stable_indirect`
// enforce recursive stability. Therefore when `lookup_const_stability` is `None`, we have to
// assume the function does not have recursive stability. All functions that *do* have recursive
// stability must explicitly record this, and so that's what we do for all `const fn` in a
// staged_api crate.
if (is_const_fn || const_stable_indirect.is_some()) && const_stab.is_none() {
let c = ConstStability {
feature: None,
const_stable_indirect: const_stable_indirect.is_some(),
promotable: false,
level: StabilityLevel::Unstable {
reason: UnstableReason::Default,
issue: None,
is_soft: false,
implied_by: None,
},
};
const_stab = Some((c, const_stable_indirect.unwrap_or(DUMMY_SP)));
}

const_stab
}

/// Calculates the const stability for a const function in a `-Zforce-unstable-if-unmarked` crate
/// without the `staged_api` feature.
pub fn unmarked_crate_const_stab(
_sess: &Session,
attrs: &[Attribute],
regular_stab: Stability,
) -> ConstStability {
assert!(regular_stab.level.is_unstable());
// The only attribute that matters here is `rustc_const_stable_indirect`.
// We enforce recursive const stability rules for those functions.
let const_stable_indirect =
attrs.iter().any(|a| a.name_or_empty() == sym::rustc_const_stable_indirect);
ConstStability {
feature: regular_stab.feature,
const_stable_indirect,
promotable: false,
level: regular_stab.level,
}
}

/// Collects stability info from `rustc_default_body_unstable` attributes in `attrs`.
/// Returns `None` if no stability attributes are found.
pub fn find_body_stability(
Expand Down
45 changes: 32 additions & 13 deletions compiler/rustc_const_eval/src/check_consts/check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::assert_matches::assert_matches;
use std::borrow::Cow;
use std::mem;
use std::num::NonZero;
use std::ops::Deref;

use rustc_attr::{ConstStability, StabilityLevel};
Expand Down Expand Up @@ -709,24 +710,26 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {

// Intrinsics are language primitives, not regular calls, so treat them separately.
if let Some(intrinsic) = tcx.intrinsic(callee) {
if !tcx.is_const_fn(callee) {
// Non-const intrinsic.
self.check_op(ops::IntrinsicNonConst { name: intrinsic.name });
// If we allowed this, we're in miri-unleashed mode, so we might
// as well skip the remaining checks.
return;
}
// We use `intrinsic.const_stable` to determine if this can be safely exposed to
// stable code, rather than `const_stable_indirect`. This is to make
// `#[rustc_const_stable_indirect]` an attribute that is always safe to add.
// We also ask is_safe_to_expose_on_stable_const_fn; this determines whether the intrinsic
// fallback body is safe to expose on stable.
let is_const_stable = intrinsic.const_stable
|| (!intrinsic.must_be_overridden
&& tcx.is_const_fn(callee)
&& is_safe_to_expose_on_stable_const_fn(tcx, callee));
match tcx.lookup_const_stability(callee) {
None => {
// Non-const intrinsic.
self.check_op(ops::IntrinsicNonConst { name: intrinsic.name });
}
Some(ConstStability { feature: None, .. }) => {
// Intrinsic does not need a separate feature gate (we rely on the
// regular stability checker). However, we have to worry about recursive
// const stability.
// This doesn't need a separate const-stability check -- const-stability equals
// regular stability, and regular stability is checked separately.
// However, we *do* have to worry about *recursive* const stability.
if !is_const_stable && self.enforce_recursive_const_stability() {
self.dcx().emit_err(errors::UnmarkedIntrinsicExposed {
span: self.span,
Expand All @@ -735,8 +738,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}
}
Some(ConstStability {
feature: Some(feature),
level: StabilityLevel::Unstable { .. },
feature,
..
}) => {
self.check_op(ops::IntrinsicUnstable {
Expand Down Expand Up @@ -773,7 +776,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
Some(ConstStability { level: StabilityLevel::Stable { .. }, .. }) => {
// All good.
}
None | Some(ConstStability { feature: None, .. }) => {
None => {
// This doesn't need a separate const-stability check -- const-stability equals
// regular stability, and regular stability is checked separately.
// However, we *do* have to worry about *recursive* const stability.
Expand All @@ -787,8 +790,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}
}
Some(ConstStability {
feature: Some(feature),
level: StabilityLevel::Unstable { implied_by: implied_feature, .. },
level: StabilityLevel::Unstable { implied_by: implied_feature, issue, .. },
feature,
..
}) => {
// An unstable const fn with a feature gate.
Expand All @@ -810,7 +813,23 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
// to allow this.
let feature_enabled = callee.is_local()
|| tcx.features().enabled(feature)
|| implied_feature.is_some_and(|f| tcx.features().enabled(f));
|| implied_feature.is_some_and(|f| tcx.features().enabled(f))
|| {
// When we're compiling the compiler itself we may pull in
// crates from crates.io, but those crates may depend on other
// crates also pulled in from crates.io. We want to ideally be
// able to compile everything without requiring upstream
// modifications, so in the case that this looks like a
// `rustc_private` crate (e.g., a compiler crate) and we also have
// the `-Z force-unstable-if-unmarked` flag present (we're
// compiling a compiler crate), then let this missing feature
// annotation slide.
// This matches what we do in `eval_stability_allow_unstable` for
// regular stability.
feature == sym::rustc_private
&& issue == NonZero::new(27812)
&& self.tcx.sess.opts.unstable_opts.force_unstable_if_unmarked
};
// We do *not* honor this if we are in the "danger zone": we have to enforce
// recursive const-stability and the callee is not safe-to-expose. In that
// case we need `check_op` to do the check.
Expand Down
20 changes: 11 additions & 9 deletions compiler/rustc_const_eval/src/check_consts/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 `-Zforce-unstable-if-unmarked` are enabled,
// since in such crates `lookup_const_stability` will always be `None`.
self.const_kind == Some(hir::ConstContext::ConstFn)
&& self.tcx.features().staged_api()
&& (self.tcx.features().staged_api()
|| self.tcx.sess.opts.unstable_opts.force_unstable_if_unmarked)
&& is_safe_to_expose_on_stable_const_fn(self.tcx, self.def_id().to_def_id())
}

Expand Down Expand Up @@ -109,14 +110,15 @@ pub fn is_safe_to_expose_on_stable_const_fn(tcx: TyCtxt<'_>, def_id: DefId) -> b

match tcx.lookup_const_stability(def_id) {
None => {
// Only marked functions can be trusted. Note that this may be a function in a
// non-staged-API crate where no recursive checks were done!
false
// In a `staged_api` crate, we do enforce recursive const stability for all unmarked
// functions, so we can trust local functions. But in another crate we don't know which
// rules were applied, so we can't trust that.
def_id.is_local() && tcx.features().staged_api()
}
Some(stab) => {
// We consider things safe-to-expose if they are stable, if they don't have any explicit
// const stability attribute, or if they are marked as `const_stable_indirect`.
stab.is_const_stable() || stab.feature.is_none() || stab.const_stable_indirect
// We consider things safe-to-expose if they are stable or if they are marked as
// `const_stable_indirect`.
stab.is_const_stable() || stab.const_stable_indirect
}
}
}
4 changes: 1 addition & 3 deletions compiler/rustc_expand/src/base.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -866,9 +866,7 @@ impl SyntaxExtension {
})
.unwrap_or_else(|| (None, helper_attrs));
let stability = attr::find_stability(sess, attrs, span);
// We set `is_const_fn` false to avoid getting any implicit const stability.
let const_stability =
attr::find_const_stability(sess, attrs, span, /* is_const_fn */ false);
let const_stability = attr::find_const_stability(sess, attrs, span);
let body_stability = attr::find_body_stability(sess, attrs);
if let Some((_, sp)) = const_stability {
sess.dcx().emit_err(errors::MacroConstStability {
Expand Down
Loading

0 comments on commit 597256c

Please sign in to comment.