Skip to content

Commit

Permalink
ensure that all publicly reachable const fn have const stability info
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Nov 3, 2024
1 parent 144bd4e commit 96f9ada
Show file tree
Hide file tree
Showing 15 changed files with 185 additions and 241 deletions.
50 changes: 13 additions & 37 deletions compiler/rustc_attr/src/builtin.rs
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,32 +346,13 @@ 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
}
Expand All @@ -394,7 +370,7 @@ pub fn unmarked_crate_const_stab(
let const_stable_indirect =
attrs.iter().any(|a| a.name_or_empty() == sym::rustc_const_stable_indirect);
ConstStability {
feature: Some(regular_stab.feature),
feature: regular_stab.feature,
const_stable_indirect,
promotable: false,
level: regular_stab.level,
Expand Down
20 changes: 10 additions & 10 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,10 @@ 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 });
}
// 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.
Expand All @@ -710,13 +714,9 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
&& 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 @@ -725,8 +725,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 @@ -766,7 +766,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 @@ -780,8 +780,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}
}
Some(ConstStability {
feature: Some(feature),
level: StabilityLevel::Unstable { implied_by: implied_feature, .. },
feature,
..
}) => {
// An unstable const fn with a feature gate.
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_const_eval/src/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,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
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 96f9ada

Please sign in to comment.