Skip to content

Commit

Permalink
"soft" (warn instead of error) feature-gate for #[must_use] on functions
Browse files Browse the repository at this point in the history
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 rust-lang#43302.
  • Loading branch information
zackmdavis committed Aug 23, 2017
1 parent 7b6e9b4 commit 8492ad2
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 13 deletions.
60 changes: 50 additions & 10 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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;
Expand Down Expand Up @@ -1011,24 +1018,42 @@ pub enum GateIssue {
Library(Option<u32>)
}

#[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();
}

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 {
GateIssue::Language => find_lang_feature_issue(feature),
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
Expand All @@ -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 =
Expand Down Expand Up @@ -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)
}
}}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down
14 changes: 12 additions & 2 deletions src/test/compile-fail/feature-gate-fn_must_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 8492ad2

Please sign in to comment.