Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stabilize #[must_use] for functions and must-use comparison operators (RFC 1940) #48925

Merged
merged 2 commits into from
May 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 0 additions & 30 deletions src/doc/unstable-book/src/language-features/fn-must-use.md

This file was deleted.

2 changes: 1 addition & 1 deletion src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
#![feature(dropck_eyepatch)]
#![feature(exact_size_is_empty)]
#![feature(fmt_internals)]
#![feature(fn_must_use)]
#![cfg_attr(stage0, feature(fn_must_use))]
#![feature(from_ref)]
#![feature(fundamental)]
#![feature(lang_items)]
Expand Down
1 change: 1 addition & 0 deletions src/liballoc/tests/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1282,6 +1282,7 @@ fn test_box_slice_clone() {
}

#[test]
#[allow(unused_must_use)] // here, we care about the side effects of `.clone()`
#[cfg_attr(target_os = "emscripten", ignore)]
fn test_box_slice_clone_panics() {
use std::sync::Arc;
Expand Down
2 changes: 1 addition & 1 deletion src/libcore/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
#![feature(doc_cfg)]
#![feature(doc_spotlight)]
#![feature(extern_types)]
#![feature(fn_must_use)]
#![feature(fundamental)]
#![feature(intrinsics)]
#![feature(iterator_flatten)]
Expand Down Expand Up @@ -114,6 +113,7 @@

#![cfg_attr(stage0, feature(target_feature))]
#![cfg_attr(stage0, feature(cfg_target_feature))]
#![cfg_attr(stage0, feature(fn_must_use))]

#[prelude_import]
#[allow(unused)]
Expand Down
100 changes: 53 additions & 47 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use rustc::hir::def::Def;
use rustc::hir::def_id::DefId;
use rustc::ty;
use rustc::ty::adjustment;
Expand Down Expand Up @@ -72,54 +73,59 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {

let mut fn_warned = false;
let mut op_warned = false;
if cx.tcx.features().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 ");
}
let must_use_op = match expr.node {
// Hardcoding operators here seemed more expedient than the
// refactoring that would be needed to look up the `#[must_use]`
// attribute which does exist on the comparison trait methods
hir::ExprBinary(bin_op, ..) => {
match bin_op.node {
hir::BiEq | hir::BiLt | hir::BiLe | hir::BiNe | hir::BiGe | hir::BiGt => {
Some("comparison")
},
hir::BiAdd | hir::BiSub | hir::BiDiv | hir::BiMul | hir::BiRem => {
Some("arithmetic operation")
},
hir::BiAnd | hir::BiOr => {
Some("logical operation")
},
hir::BiBitXor | hir::BiBitAnd | hir::BiBitOr | hir::BiShl | hir::BiShr => {
Some("bitwise operation")
},
}
},
hir::ExprUnary(..) => Some("unary operation"),
_ => None
};
if let Some(must_use_op) = must_use_op {
cx.span_lint(UNUSED_MUST_USE, expr.span,
&format!("unused {} which must be used", must_use_op));
op_warned = true;
}
let maybe_def = match expr.node {
hir::ExprCall(ref callee, _) => {
match callee.node {
hir::ExprPath(ref qpath) => {
let def = cx.tables.qpath_def(qpath, callee.hir_id);
if let Def::Fn(_) = def {
Some(def)
} else { // `Def::Local` if it was a closure, for which we
None // do not currently support must-use linting
}
},
_ => 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 ");
}
let must_use_op = match expr.node {
// Hardcoding operators here seemed more expedient than the
// refactoring that would be needed to look up the `#[must_use]`
// attribute which does exist on the comparison trait methods
hir::ExprBinary(bin_op, ..) => {
match bin_op.node {
hir::BiEq | hir::BiLt | hir::BiLe | hir::BiNe | hir::BiGe | hir::BiGt => {
Some("comparison")
},
hir::BiAdd | hir::BiSub | hir::BiDiv | hir::BiMul | hir::BiRem => {
Some("arithmetic operation")
},
hir::BiAnd | hir::BiOr => {
Some("logical operation")
},
hir::BiBitXor | hir::BiBitAnd | hir::BiBitOr | hir::BiShl | hir::BiShr => {
Some("bitwise operation")
},
}
},
hir::ExprUnary(..) => Some("unary operation"),
_ => None
};

if let Some(must_use_op) = must_use_op {
cx.span_lint(UNUSED_MUST_USE, expr.span,
&format!("unused {} which must be used", must_use_op));
op_warned = true;
}

if !(ty_warned || fn_warned || op_warned) {
cx.span_lint(UNUSED_RESULTS, s.span, "unused result");
}
Expand Down
2 changes: 2 additions & 0 deletions src/libstd/ffi/c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,7 @@ impl CStr {
/// behavior when `ptr` is used inside the `unsafe` block:
///
/// ```no_run
/// # #![allow(unused_must_use)]
/// use std::ffi::{CString};
///
/// let ptr = CString::new("Hello").unwrap().as_ptr();
Expand All @@ -1003,6 +1004,7 @@ impl CStr {
/// To fix the problem, bind the `CString` to a local variable:
///
/// ```no_run
/// # #![allow(unused_must_use)]
/// use std::ffi::{CString};
///
/// let hello = CString::new("Hello").unwrap();
Expand Down
2 changes: 2 additions & 0 deletions src/libstd/sync/mpsc/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ mod tests {
}
}

#[allow(unused_must_use)]
#[test]
fn cloning() {
let (tx1, rx1) = channel::<i32>();
Expand All @@ -540,6 +541,7 @@ mod tests {
tx3.send(()).unwrap();
}

#[allow(unused_must_use)]
#[test]
fn cloning2() {
let (tx1, rx1) = channel::<i32>();
Expand Down
22 changes: 3 additions & 19 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,6 @@ declare_features! (
// #[doc(include="some-file")]
(active, external_doc, "1.22.0", Some(44732), None),

// allow `#[must_use]` on functions and comparison operators (RFC 1940)
(active, fn_must_use, "1.21.0", Some(43302), None),

// Future-proofing enums/structs with #[non_exhaustive] attribute (RFC 2008)
(active, non_exhaustive, "1.22.0", Some(44109), None),

Expand Down Expand Up @@ -591,6 +588,8 @@ declare_features! (
(accepted, target_feature, "1.27.0", None, None),
// Trait object syntax with `dyn` prefix
(accepted, dyn_trait, "1.27.0", Some(44662), None),
// allow `#[must_use]` on functions; and, must-use operators (RFC 1940)
(accepted, fn_must_use, "1.27.0", Some(43302), None),
);

// If you change this, please modify src/doc/unstable-book as well. You must
Expand Down Expand Up @@ -1545,11 +1544,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
function may change over time, for now \
a top-level `fn main()` is required");
}
if let Some(attr) = attr::find_by_name(&i.attrs[..], "must_use") {
gate_feature_post!(&self, fn_must_use, attr.span,
"`#[must_use]` on functions is experimental",
GateStrength::Soft);
}
}

ast::ItemKind::Struct(..) => {
Expand Down Expand Up @@ -1581,7 +1575,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
"trait aliases are not yet fully implemented");
}

ast::ItemKind::Impl(_, polarity, defaultness, _, _, _, ref impl_items) => {
ast::ItemKind::Impl(_, polarity, defaultness, _, _, _, _) => {
if polarity == ast::ImplPolarity::Negative {
gate_feature_post!(&self, optin_builtin_traits,
i.span,
Expand All @@ -1594,16 +1588,6 @@ 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 let Some(attr) = attr::find_by_name(&impl_item.attrs[..], "must_use") {
gate_feature_post!(&self, fn_must_use, attr.span,
"`#[must_use]` on methods is experimental",
GateStrength::Soft);
}
}
}
}

ast::ItemKind::Trait(ast::IsAuto::Yes, ..) => {
Expand Down
22 changes: 0 additions & 22 deletions src/test/ui/feature-gate-fn_must_use-cap-lints-allow.rs

This file was deleted.

8 changes: 0 additions & 8 deletions src/test/ui/feature-gate-fn_must_use-cap-lints-allow.stderr

This file was deleted.

31 changes: 0 additions & 31 deletions src/test/ui/feature-gate-fn_must_use.rs

This file was deleted.

24 changes: 0 additions & 24 deletions src/test/ui/feature-gate-fn_must_use.stderr

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,6 @@ 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;

Expand Down
Loading