diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 195bd2acce0fe..cbc4ebe90fd09 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -160,21 +160,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { }; let mut fn_warned = false; - let maybe_def = match expr.node { - hir::ExprCall(ref callee, _) => { - match callee.node { - hir::ExprPath(ref qpath) => Some(cx.tables.qpath_def(qpath, callee.hir_id)), - _ => None - } - }, - hir::ExprMethodCall(..) => { - cx.tables.type_dependent_defs().get(expr.hir_id).cloned() - }, - _ => { None } - }; - if let Some(def) = maybe_def { - let def_id = def.def_id(); - fn_warned = check_must_use(cx, def_id, s.span, "return value of "); + if cx.tcx.sess.features.borrow().fn_must_use { + let maybe_def = match expr.node { + hir::ExprCall(ref callee, _) => { + match callee.node { + hir::ExprPath(ref qpath) => { + Some(cx.tables.qpath_def(qpath, callee.hir_id)) + }, + _ => None + } + }, + hir::ExprMethodCall(..) => { + cx.tables.type_dependent_defs().get(expr.hir_id).cloned() + }, + _ => None + }; + if let Some(def) = maybe_def { + let def_id = def.def_id(); + fn_warned = check_must_use(cx, def_id, s.span, "return value of "); + } } if !(ty_warned || fn_warned) { diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 801343689b728..09574d5ba129e 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -112,8 +112,8 @@ macro_rules! declare_features { // was set. This is most important for knowing when a particular feature became // stable (active). // -// NB: The featureck.py script parses this information directly out of the source -// so take care when modifying it. +// NB: tools/tidy/src/features.rs parses this information directly out of the +// source, so take care when modifying it. declare_features! ( (active, asm, "1.0.0", Some(29722)), @@ -372,6 +372,9 @@ declare_features! ( // #[doc(cfg(...))] (active, doc_cfg, "1.21.0", Some(43781)), + + // allow `#[must_use]` on functions (RFC 1940) + (active, fn_must_use, "1.21.0", Some(43302)), ); declare_features! ( @@ -915,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> { @@ -938,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; @@ -1008,13 +1018,26 @@ 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(); } pub fn feature_err<'a>(sess: &'a ParseSess, feature: &str, span: Span, issue: GateIssue, - explain: &str) -> DiagnosticBuilder<'a> { + 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 { @@ -1022,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 @@ -1035,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 = @@ -1092,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) + } }} } @@ -1234,6 +1276,11 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { function may change over time, for now \ a top-level `fn main()` is required"); } + if attr::contains_name(&i.attrs[..], "must_use") { + gate_feature_post!(&self, fn_must_use, i.span, + "`#[must_use]` on functions is experimental", + GateStrength::Soft); + } } ast::ItemKind::Struct(..) => { @@ -1271,7 +1318,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { and possibly buggy"); } - ast::ItemKind::Impl(_, polarity, defaultness, _, _, _, _) => { + ast::ItemKind::Impl(_, polarity, defaultness, _, _, _, ref impl_items) => { if polarity == ast::ImplPolarity::Negative { gate_feature_post!(&self, optin_builtin_traits, i.span, @@ -1284,6 +1331,16 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { i.span, "specialization is unstable"); } + + for impl_item in impl_items { + if let ast::ImplItemKind::Method(..) = impl_item.node { + if attr::contains_name(&impl_item.attrs[..], "must_use") { + gate_feature_post!(&self, fn_must_use, impl_item.span, + "`#[must_use]` on methods is experimental", + GateStrength::Soft); + } + } + } } ast::ItemKind::MacroDef(ast::MacroDef { legacy: false, .. }) => { diff --git a/src/test/compile-fail/feature-gate-fn_must_use.rs b/src/test/compile-fail/feature-gate-fn_must_use.rs new file mode 100644 index 0000000000000..2dd6b90407267 --- /dev/null +++ b/src/test/compile-fail/feature-gate-fn_must_use.rs @@ -0,0 +1,31 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(rustc_attrs)] + +struct MyStruct; + +impl MyStruct { + #[must_use] + fn need_to_use_method() -> bool { true } //~ WARN `#[must_use]` on methods is experimental +} + +#[must_use] +fn need_to_use_it() -> bool { true } //~ WARN `#[must_use]` on functions is experimental + + +// 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 29a2b0609fcd2..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,6 +680,7 @@ mod must_use { mod inner { #![must_use="1400"] } #[must_use = "1400"] fn f() { } + //~^ WARN `#[must_use]` on functions is experimental #[must_use = "1400"] struct S; diff --git a/src/test/ui/lint/fn_must_use.rs b/src/test/ui/lint/fn_must_use.rs index 5aea5f2ca0642..c549ded4db218 100644 --- a/src/test/ui/lint/fn_must_use.rs +++ b/src/test/ui/lint/fn_must_use.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![feature(fn_must_use)] #![warn(unused_must_use)] struct MyStruct { diff --git a/src/test/ui/lint/fn_must_use.stderr b/src/test/ui/lint/fn_must_use.stderr index 20eb7452aea71..242837793a0bf 100644 --- a/src/test/ui/lint/fn_must_use.stderr +++ b/src/test/ui/lint/fn_must_use.stderr @@ -1,18 +1,18 @@ warning: unused return value of `need_to_use_this_value` which must be used: it's important - --> $DIR/fn_must_use.rs:30:5 + --> $DIR/fn_must_use.rs:31:5 | -30 | need_to_use_this_value(); +31 | need_to_use_this_value(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: lint level defined here - --> $DIR/fn_must_use.rs:11:9 + --> $DIR/fn_must_use.rs:12:9 | -11 | #![warn(unused_must_use)] +12 | #![warn(unused_must_use)] | ^^^^^^^^^^^^^^^ warning: unused return value of `MyStruct::need_to_use_this_method_value` which must be used - --> $DIR/fn_must_use.rs:33:5 + --> $DIR/fn_must_use.rs:34:5 | -33 | m.need_to_use_this_method_value(); +34 | m.need_to_use_this_method_value(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^