From 8492ad2479379d2e07ccf2d3439ec29b19d164b8 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Tue, 22 Aug 2017 16:05:01 -0700 Subject: [PATCH] "soft" (warn instead of error) feature-gate for #[must_use] on functions Before `#[must_use]` for functions was implemented, a `#[must_use]` attribute on a function was a no-op. To avoid a breaking change in this behavior, we add an option for "this-and-such feature is experimental" feature-gate messages to be a mere warning rather than a compilation-halting failure (so old code that used to have a useless no-op `#[must_use]` attribute now warns rather than breaking). When we're on stable, we add a help note to clarify that the feature isn't "on." This is in support of #43302. --- src/libsyntax/feature_gate.rs | 60 +++++++++++++++---- .../compile-fail/feature-gate-fn_must_use.rs | 14 ++++- .../issue-43106-gating-of-builtin-attrs.rs | 2 +- 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index c82f0554393dc..79f15f2fe1958 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -918,20 +918,27 @@ struct Context<'a> { } macro_rules! gate_feature_fn { - ($cx: expr, $has_feature: expr, $span: expr, $name: expr, $explain: expr) => {{ - let (cx, has_feature, span, name, explain) = ($cx, $has_feature, $span, $name, $explain); + ($cx: expr, $has_feature: expr, $span: expr, $name: expr, $explain: expr, $level: expr) => {{ + let (cx, has_feature, span, + name, explain, level) = ($cx, $has_feature, $span, $name, $explain, $level); let has_feature: bool = has_feature(&$cx.features); debug!("gate_feature(feature = {:?}, span = {:?}); has? {}", name, span, has_feature); if !has_feature && !span.allows_unstable() { - emit_feature_err(cx.parse_sess, name, span, GateIssue::Language, explain); + leveled_feature_err(cx.parse_sess, name, span, GateIssue::Language, explain, level) + .emit(); } }} } macro_rules! gate_feature { ($cx: expr, $feature: ident, $span: expr, $explain: expr) => { - gate_feature_fn!($cx, |x:&Features| x.$feature, $span, stringify!($feature), $explain) - } + gate_feature_fn!($cx, |x:&Features| x.$feature, $span, + stringify!($feature), $explain, GateStrength::Hard) + }; + ($cx: expr, $feature: ident, $span: expr, $explain: expr, $level: expr) => { + gate_feature_fn!($cx, |x:&Features| x.$feature, $span, + stringify!($feature), $explain, $level) + }; } impl<'a> Context<'a> { @@ -941,7 +948,7 @@ impl<'a> Context<'a> { for &(n, ty, ref gateage) in BUILTIN_ATTRIBUTES { if name == n { if let Gated(_, name, desc, ref has_feature) = *gateage { - gate_feature_fn!(self, has_feature, attr.span, name, desc); + gate_feature_fn!(self, has_feature, attr.span, name, desc, GateStrength::Hard); } debug!("check_attribute: {:?} is builtin, {:?}, {:?}", attr.path, ty, gateage); return; @@ -1011,6 +1018,14 @@ pub enum GateIssue { Library(Option) } +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum GateStrength { + /// A hard error. (Most feature gates should use this.) + Hard, + /// Only a warning. (Use this only as backwards-compatibility demands.) + Soft, +} + pub fn emit_feature_err(sess: &ParseSess, feature: &str, span: Span, issue: GateIssue, explain: &str) { feature_err(sess, feature, span, issue, explain).emit(); @@ -1018,6 +1033,11 @@ pub fn emit_feature_err(sess: &ParseSess, feature: &str, span: Span, issue: Gate pub fn feature_err<'a>(sess: &'a ParseSess, feature: &str, span: Span, issue: GateIssue, explain: &str) -> DiagnosticBuilder<'a> { + leveled_feature_err(sess, feature, span, issue, explain, GateStrength::Hard) +} + +fn leveled_feature_err<'a>(sess: &'a ParseSess, feature: &str, span: Span, issue: GateIssue, + explain: &str, level: GateStrength) -> DiagnosticBuilder<'a> { let diag = &sess.span_diagnostic; let issue = match issue { @@ -1025,10 +1045,15 @@ pub fn feature_err<'a>(sess: &'a ParseSess, feature: &str, span: Span, issue: Ga GateIssue::Library(lib) => lib, }; - let mut err = if let Some(n) = issue { - diag.struct_span_err(span, &format!("{} (see issue #{})", explain, n)) + let explanation = if let Some(n) = issue { + format!("{} (see issue #{})", explain, n) } else { - diag.struct_span_err(span, explain) + explain.to_owned() + }; + + let mut err = match level { + GateStrength::Hard => diag.struct_span_err(span, &explanation), + GateStrength::Soft => diag.struct_span_warn(span, &explanation), }; // #23973: do not suggest `#![feature(...)]` if we are in beta/stable @@ -1038,7 +1063,15 @@ pub fn feature_err<'a>(sess: &'a ParseSess, feature: &str, span: Span, issue: Ga feature)); } + // If we're on stable and only emitting a "soft" warning, add a note to + // clarify that the feature isn't "on" (rather than being on but + // warning-worthy). + if !sess.unstable_features.is_nightly_build() && level == GateStrength::Soft { + err.help("a nightly build of the compiler is required to enable this feature"); + } + err + } const EXPLAIN_BOX_SYNTAX: &'static str = @@ -1095,6 +1128,12 @@ macro_rules! gate_feature_post { if !span.allows_unstable() { gate_feature!(cx.context, $feature, span, $explain) } + }}; + ($cx: expr, $feature: ident, $span: expr, $explain: expr, $level: expr) => {{ + let (cx, span) = ($cx, $span); + if !span.allows_unstable() { + gate_feature!(cx.context, $feature, span, $explain, $level) + } }} } @@ -1239,7 +1278,8 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { } if attr::contains_name(&i.attrs[..], "must_use") { gate_feature_post!(&self, fn_must_use, i.span, - "`#[must_use]` on functions is experimental"); + "`#[must_use]` on functions is experimental", + GateStrength::Soft); } } diff --git a/src/test/compile-fail/feature-gate-fn_must_use.rs b/src/test/compile-fail/feature-gate-fn_must_use.rs index 7bb533251dc08..a222f36614535 100644 --- a/src/test/compile-fail/feature-gate-fn_must_use.rs +++ b/src/test/compile-fail/feature-gate-fn_must_use.rs @@ -8,7 +8,17 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![feature(rustc_attrs)] + #[must_use] -fn need_to_use_it() -> bool { true } //~ ERROR `#[must_use]` on functions is experimental +fn need_to_use_it() -> bool { true } //~ WARN `#[must_use]` on functions is experimental + -fn main() {} +// Feature gates are tidy-required to have a specially named (or +// comment-annotated) compile-fail test (which MUST fail), but for +// backwards-compatibility reasons, we want `#[must_use]` on functions to be +// compilable even if the `fn_must_use` feature is absent, thus necessitating +// the usage of `#[rustc_error]` here, pragmatically if awkwardly solving this +// dilemma until a superior solution can be devised. +#[rustc_error] +fn main() {} //~ ERROR compilation successful diff --git a/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs b/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs index 021daf88420d1..204190d64acc1 100644 --- a/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs +++ b/src/test/compile-fail/feature-gate/issue-43106-gating-of-builtin-attrs.rs @@ -680,7 +680,7 @@ mod must_use { mod inner { #![must_use="1400"] } #[must_use = "1400"] fn f() { } - //~^ ERROR `#[must_use]` on functions is experimental + //~^ WARN `#[must_use]` on functions is experimental #[must_use = "1400"] struct S;