Skip to content

Commit bd53b3e

Browse files
committed
apply review feedback
1 parent 41ab0fc commit bd53b3e

File tree

4 files changed

+32
-18
lines changed

4 files changed

+32
-18
lines changed

compiler/rustc_codegen_gcc/src/gcc_util.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ pub(crate) fn global_gcc_features(sess: &Session, diagnostics: bool) -> Vec<Stri
9595
sess.dcx().emit_warn(unknown_feature);
9696
}
9797
Some((_, stability, _)) => {
98-
if let Err(reason) = stability.compute(&sess.target).allow_toggle() {
98+
if let Err(reason) =
99+
stability.compute_toggleability(&sess.target).allow_toggle()
100+
{
99101
sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, reason });
100102
} else if stability.requires_nightly().is_some() {
101103
// An unstable feature. Warn about using it. (It makes little sense

compiler/rustc_codegen_llvm/src/llvm_util.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -714,12 +714,14 @@ pub(crate) fn global_llvm_features(
714714
sess.dcx().emit_warn(unknown_feature);
715715
}
716716
Some((_, stability, _)) => {
717-
if let Err(reason) = stability.compute(&sess.target).allow_toggle() {
717+
if let Err(reason) =
718+
stability.compute_toggleability(&sess.target).allow_toggle()
719+
{
718720
sess.dcx().emit_warn(ForbiddenCTargetFeature { feature, reason });
719721
} else if stability.requires_nightly().is_some() {
720-
// An unstable feature. Warn about using it. (It makes little sense
722+
// An unstable feature. Warn about using it. It makes little sense
721723
// to hard-error here since we just warn about fully unknown
722-
// features above).
724+
// features above.
723725
sess.dcx().emit_warn(UnstableCTargetFeature { feature });
724726
}
725727
}

compiler/rustc_codegen_ssa/src/target_features.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -153,14 +153,14 @@ pub(crate) fn provide(providers: &mut Providers) {
153153
// rustdoc needs to be able to document functions that use all the features, so
154154
// whitelist them all
155155
rustc_target::target_features::all_rust_features()
156-
.map(|(a, b)| (a.to_string(), b.compute(target)))
156+
.map(|(a, b)| (a.to_string(), b.compute_toggleability(target)))
157157
.collect()
158158
} else {
159159
tcx.sess
160160
.target
161161
.rust_target_features()
162162
.iter()
163-
.map(|&(a, b, _)| (a.to_string(), b.compute(target)))
163+
.map(|&(a, b, _)| (a.to_string(), b.compute_toggleability(target)))
164164
.collect()
165165
}
166166
},

compiler/rustc_target/src/target_features.rs

+22-12
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,17 @@ pub const RUSTC_SPECIFIC_FEATURES: &[&str] = &["crt-static"];
1717
pub const RUSTC_SPECIAL_FEATURES: &[&str] = &["backchain"];
1818

1919
/// Stability information for target features.
20-
/// `AllowToggle` is the type storing whether (un)stable features can be toggled:
20+
/// `Toggleability` is the type storing whether (un)stable features can be toggled:
2121
/// this is initially a function since it can depend on `Target`, but for stable hashing
2222
/// it needs to be something hashable to we have to make the type generic.
2323
#[derive(Debug, Clone, Copy)]
24-
pub enum Stability<AllowToggle> {
24+
pub enum Stability<Toggleability> {
2525
/// This target feature is stable, it can be used in `#[target_feature]` and
2626
/// `#[cfg(target_feature)]`.
2727
Stable {
28-
/// When enabling/dsiabling the feature via `-Ctarget-feature` or `#[target_feature]`,
28+
/// When enabling/disabling the feature via `-Ctarget-feature` or `#[target_feature]`,
2929
/// determine if that is allowed.
30-
allow_toggle: AllowToggle,
30+
allow_toggle: Toggleability,
3131
},
3232
/// This target feature is unstable. It is only present in `#[cfg(target_feature)]` on
3333
/// nightly and using it in `#[target_feature]` requires enabling the given nightly feature.
@@ -36,7 +36,7 @@ pub enum Stability<AllowToggle> {
3636
/// feature gate!
3737
nightly_feature: Symbol,
3838
/// See `Stable::allow_toggle` comment above.
39-
allow_toggle: AllowToggle,
39+
allow_toggle: Toggleability,
4040
},
4141
/// This feature can not be set via `-Ctarget-feature` or `#[target_feature]`, it can only be
4242
/// set in the basic target definition. It is never set in `cfg(target_feature)`. Used in
@@ -50,7 +50,7 @@ pub type StabilityUncomputed = Stability<fn(&Target) -> Result<(), &'static str>
5050
/// `Stability` where `allow_toggle` has already been computed.
5151
pub type StabilityComputed = Stability<Result<(), &'static str>>;
5252

53-
impl<CTX, AllowToggle: HashStable<CTX>> HashStable<CTX> for Stability<AllowToggle> {
53+
impl<CTX, Toggleability: HashStable<CTX>> HashStable<CTX> for Stability<Toggleability> {
5454
#[inline]
5555
fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) {
5656
std::mem::discriminant(self).hash_stable(hcx, hasher);
@@ -69,15 +69,22 @@ impl<CTX, AllowToggle: HashStable<CTX>> HashStable<CTX> for Stability<AllowToggl
6969
}
7070
}
7171

72-
impl<AllowToggle> Stability<AllowToggle> {
73-
/// Returns whether the feature can be queried in `cfg` ever.
74-
/// (It might still be nightly-only even if this returns `true`).
72+
impl<Toggleability> Stability<Toggleability> {
73+
/// Returns whether the feature can be used in `cfg(target_feature)` ever.
74+
/// (It might still be nightly-only even if this returns `true`, so make sure to also check
75+
/// `requires_nightly`.)
7576
pub fn in_cfg(self) -> bool {
7677
!matches!(self, Stability::Forbidden { .. })
7778
}
7879

79-
/// Returns the nightly feature that is required to toggle or query this target feature. Ensure
80-
/// to also check `allow_toggle()` before allowing to toggle!
80+
/// Returns the nightly feature that is required to toggle this target feature via
81+
/// `#[target_feature]`/`-Ctarget-feature` or to test it via `cfg(target_feature)`.
82+
/// (For `cfg` we only care whether the feature is nightly or not, we don't require
83+
/// the feature gate to actually be enabled when using a nightly compiler.)
84+
///
85+
/// Before calling this, ensure the feature is even permitted for this use:
86+
/// - for `#[target_feature]`/`-Ctarget-feature`, check `allow_toggle()`
87+
/// - for `cfg(target_feature)`, check `in_cfg`
8188
pub fn requires_nightly(self) -> Option<Symbol> {
8289
match self {
8390
Stability::Unstable { nightly_feature, .. } => Some(nightly_feature),
@@ -88,7 +95,7 @@ impl<AllowToggle> Stability<AllowToggle> {
8895
}
8996

9097
impl StabilityUncomputed {
91-
pub fn compute(self, target: &Target) -> StabilityComputed {
98+
pub fn compute_toggleability(self, target: &Target) -> StabilityComputed {
9299
use Stability::*;
93100
match self {
94101
Stable { allow_toggle } => Stable { allow_toggle: allow_toggle(target) },
@@ -101,6 +108,9 @@ impl StabilityUncomputed {
101108
}
102109

103110
impl StabilityComputed {
111+
/// Returns whether the feature may be toggled via `#[target_feature]` or `-Ctarget-feature`.
112+
/// (It might still be nightly-only even if this returns `true`, so make sure to also check
113+
/// `requires_nightly`.)
104114
pub fn allow_toggle(self) -> Result<(), &'static str> {
105115
match self {
106116
Stability::Stable { allow_toggle } => allow_toggle,

0 commit comments

Comments
 (0)