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

Extend #[must_use] to nested structures #62262

Closed
wants to merge 17 commits into from
Closed
2 changes: 1 addition & 1 deletion src/liballoc/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1568,7 +1568,7 @@ impl String {
Unbounded => {},
};

unsafe {
let _ = unsafe {
self.as_mut_vec()
}.splice(range, replace_with.bytes());
}
Expand Down
6 changes: 3 additions & 3 deletions src/liballoc/tests/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ fn test_drain_inclusive_out_of_bounds() {
fn test_splice() {
let mut v = vec![1, 2, 3, 4, 5];
let a = [10, 11, 12];
v.splice(2..4, a.iter().cloned());
let _ = v.splice(2..4, a.iter().cloned());
assert_eq!(v, &[1, 2, 10, 11, 12, 5]);
v.splice(1..3, Some(20));
assert_eq!(v, &[1, 20, 11, 12, 5]);
Expand All @@ -606,15 +606,15 @@ fn test_splice_inclusive_range() {
fn test_splice_out_of_bounds() {
let mut v = vec![1, 2, 3, 4, 5];
let a = [10, 11, 12];
v.splice(5..6, a.iter().cloned());
let _ = v.splice(5..6, a.iter().cloned());
}

#[test]
#[should_panic]
fn test_splice_inclusive_out_of_bounds() {
let mut v = vec![1, 2, 3, 4, 5];
let a = [10, 11, 12];
v.splice(5..=5, a.iter().cloned());
let _ = v.splice(5..=5, a.iter().cloned());
}

#[test]
Expand Down
95 changes: 66 additions & 29 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use rustc::hir::def::{Res, DefKind};
use rustc::hir::def_id::DefId;
use rustc::hir::HirVec;
use rustc::lint;
use rustc::ty::{self, Ty};
use rustc::ty::subst::Subst;
use rustc::ty::adjustment;
use rustc_data_structures::fx::FxHashMap;
use lint::{LateContext, EarlyContext, LintContext, LintArray};
Expand Down Expand Up @@ -48,7 +50,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
}

let ty = cx.tables.expr_ty(&expr);
let type_permits_lack_of_use = check_must_use_ty(cx, ty, &expr, s.span, "", "", 1);
let type_permits_lack_of_use = check_must_use_ty(cx, ty, &expr, s.span, "", "", 1, false);

let mut fn_warned = false;
let mut op_warned = false;
Expand All @@ -73,7 +75,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
_ => None
};
if let Some(def_id) = maybe_def_id {
fn_warned = check_must_use_def(cx, def_id, s.span, "return value of ", "");
fn_warned = check_must_use_def(cx, def_id, s.span, "return value of ", "", false);
} else if type_permits_lack_of_use {
// We don't warn about unused unit or uninhabited types.
// (See https://github.com/rust-lang/rust/issues/43806 for details.)
Expand Down Expand Up @@ -135,24 +137,61 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
span: Span,
descr_pre: &str,
descr_post: &str,
plural_len: usize,
len: usize,
err: bool, // HACK: Report an error rather than a lint, for crater testing.
) -> bool {
if ty.is_unit() || cx.tcx.is_ty_uninhabited_from(
cx.tcx.hir().get_module_parent(expr.hir_id), ty)
{
let module = cx.tcx.hir().get_module_parent(expr.hir_id);

if ty.is_unit() || cx.tcx.is_ty_uninhabited_from(module, ty) {
return true;
}

let plural_suffix = pluralise!(plural_len);
let plural_suffix = pluralise!(len);

match ty.kind {
ty::Adt(..) if ty.is_box() => {
let boxed_ty = ty.boxed_ty();
let descr_pre = &format!("{}boxed ", descr_pre);
check_must_use_ty(cx, boxed_ty, expr, span, descr_pre, descr_post, plural_len)
check_must_use_ty(cx, boxed_ty, expr, span, descr_pre, descr_post, len, err)
}
ty::Adt(def, _) => {
check_must_use_def(cx, def.did, span, descr_pre, descr_post)
ty::Adt(def, subst) => {
// Check the type itself for `#[must_use]` annotations.
let mut has_emitted = check_must_use_def(
cx, def.did, span, descr_pre, descr_post, err);
// Check any fields of the type for `#[must_use]` annotations.
// We ignore ADTs with more than one variant for simplicity and to avoid
// false positives.
// Unions are also ignored (though in theory, we could lint if every field of
// a union was `#[must_use]`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably go into a separate method and the comments above can become doc comments...

if def.variants.len() == 1 && !def.is_union() {
let fields = match &expr.kind {
hir::ExprKind::Struct(_, fields, _) => {
fields.iter().map(|f| &*f.expr).collect()
}
hir::ExprKind::Call(_, args) => args.iter().collect(),
_ => HirVec::new(),
};

for variant in &def.variants {
for (i, field) in variant.fields.iter().enumerate() {
let is_visible = def.is_enum() ||
field.vis.is_accessible_from(module, cx.tcx);
if is_visible {
let descr_post
= &format!(" in field `{}`", field.ident.as_str());
let ty = cx.tcx.type_of(field.did).subst(cx.tcx, subst);
let (expr, span) = if let Some(&field) = fields.get(i) {
(field, field.span)
} else {
(expr, span)
};
has_emitted |= check_must_use_ty(
cx, ty, expr, span, descr_pre, descr_post, len, true);
}
}
}
}
has_emitted
}
ty::Opaque(def, _) => {
let mut has_emitted = false;
Expand All @@ -165,7 +204,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
descr_pre,
plural_suffix,
);
if check_must_use_def(cx, def_id, span, descr_pre, descr_post) {
if check_must_use_def(cx, def_id, span, descr_pre, descr_post, err) {
has_emitted = true;
break;
}
Expand All @@ -183,7 +222,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
plural_suffix,
descr_post,
);
if check_must_use_def(cx, def_id, span, descr_pre, descr_post) {
if check_must_use_def(cx, def_id, span, descr_pre, descr_post, err) {
has_emitted = true;
break;
}
Expand All @@ -202,32 +241,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
for (i, ty) in tys.iter().map(|k| k.expect_ty()).enumerate() {
let descr_post = &format!(" in tuple element {}", i);
let span = *spans.get(i).unwrap_or(&span);
if check_must_use_ty(
cx,
ty,
expr,
span,
descr_pre,
descr_post,
plural_len
) {
has_emitted = true;
}
has_emitted |= check_must_use_ty(
cx, ty, expr, span, descr_pre, descr_post, len, err);
}
has_emitted
}
ty::Array(ty, len) => match len.try_eval_usize(cx.tcx, cx.param_env) {
// If the array is definitely non-empty, we can do `#[must_use]` checking.
Some(n) if n != 0 => {
// Empty arrays won't contain any `#[must_use]` types.
Some(0) => false,
// If the array may be non-empty, we do `#[must_use]` checking.
_ => {
let descr_pre = &format!(
"{}array{} of ",
descr_pre,
plural_suffix,
);
check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, n as usize + 1)
// `2` is just a stand-in for a number greater than 1, for correct plurals
// in diagnostics.
check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, 2, err)
}
// Otherwise, we don't lint, to avoid false positives.
_ => false,
}
_ => false,
}
Expand All @@ -240,12 +272,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
span: Span,
descr_pre_path: &str,
descr_post_path: &str,
force_err: bool, // HACK: Report an error rather than a lint, for crater testing.
) -> bool {
for attr in cx.tcx.get_attrs(def_id).iter() {
if attr.check_name(sym::must_use) {
let msg = format!("unused {}`{}`{} that must be used",
descr_pre_path, cx.tcx.def_path_str(def_id), descr_post_path);
let mut err = cx.struct_span_lint(UNUSED_MUST_USE, span, &msg);
let mut err = if !force_err {
cx.struct_span_lint(UNUSED_MUST_USE, span, &msg)
} else {
cx.sess().struct_span_err(span, &msg)
};
// check for #[must_use = "..."]
if let Some(note) = attr.value_str() {
err.note(&note.as_str());
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/add_retag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl<'tcx> MirPass<'tcx> for AddRetag {
.filter(needs_retag)
.collect::<Vec<_>>();
// Emit their retags.
basic_blocks[START_BLOCK].statements.splice(0..0,
let _ = basic_blocks[START_BLOCK].statements.splice(0..0,
places.into_iter().map(|place| Statement {
source_info,
kind: StatementKind::Retag(RetagKind::FnEntry, box(place)),
Expand Down
4 changes: 1 addition & 3 deletions src/libstd/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,7 @@ pub fn set_hook(hook: Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>) {
HOOK_LOCK.write_unlock();

if let Hook::Custom(ptr) = old_hook {
#[allow(unused_must_use)] {
Box::from_raw(ptr);
}
mem::drop(Box::from_raw(ptr));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/incremental/change_crate_order/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ use b::B;

//? #[rustc_clean(label="typeck_tables_of", cfg="rpass2")]
pub fn main() {
A + B;
let _ = A + B;
}
2 changes: 1 addition & 1 deletion src/test/incremental/warnings-reemitted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@
#![warn(const_err)]

fn main() {
255u8 + 1; //~ WARNING this expression will panic at run-time
let _ = 255u8 + 1; //~ WARNING this expression will panic at run-time
}
2 changes: 2 additions & 0 deletions src/test/mir-opt/const_prop/ref_deref.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(unused_must_use)]

fn main() {
*(&4);
}
Expand Down
1 change: 1 addition & 0 deletions src/test/pretty/block-disambig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use std::cell::Cell;

#[allow(unused_must_use)]
fn test1() { let val = &0; { } *val; }

fn test2() -> isize { let val = &0; { } *val }
Expand Down
1 change: 1 addition & 0 deletions src/test/pretty/unary-op-disambig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ fn alt_semi() -> isize { match true { true => { f() } _ => { } }; -1 }

fn alt_no_semi() -> isize { (match true { true => { 0 } _ => { 1 } }) - 1 }

#[allow(unused_must_use)]
fn stmt() { { f() }; -1; }
2 changes: 1 addition & 1 deletion src/test/run-fail/binop-fail-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ fn foo() -> ! {

#[allow(resolve_trait_on_defaulted_unit)]
fn main() {
foo() == foo(); // these types wind up being defaulted to ()
let _ = foo() == foo(); // these types wind up being defaulted to ()
}
2 changes: 1 addition & 1 deletion src/test/run-fail/binop-panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ fn my_err(s: String) -> ! {
panic!("quux");
}
fn main() {
3_usize == my_err("bye".to_string());
let _ = 3_usize == my_err("bye".to_string());
}
2 changes: 1 addition & 1 deletion src/test/run-fail/generator-resume-after-panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fn main() {
panic!();
yield;
};
panic::catch_unwind(panic::AssertUnwindSafe(|| {
let _ = panic::catch_unwind(panic::AssertUnwindSafe(|| {
let x = Pin::new(&mut g).resume();
}));
Pin::new(&mut g).resume();
Expand Down
2 changes: 1 addition & 1 deletion src/test/run-fail/issue-28934.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ impl<'i, 't> Parser<'i, 't> {

fn main() {
let x = 0u8;
Parser(&x, &x).parse_nested_block(|input| input.expect_exhausted()).unwrap();
let _ = Parser(&x, &x).parse_nested_block(|input| input.expect_exhausted()).unwrap();
}
2 changes: 1 addition & 1 deletion src/test/run-fail/panic-set-unset-handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ fn main() {
panic::set_hook(Box::new(|i| {
eprint!("greetings from the panic handler");
}));
panic::take_hook();
let _ = panic::take_hook();
panic!("foobar");
}
2 changes: 1 addition & 1 deletion src/test/run-fail/panic-take-handler-nop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
use std::panic;

fn main() {
panic::take_hook();
let _ = panic::take_hook();
panic!("foobar");
}
4 changes: 3 additions & 1 deletion src/test/run-make-fulldeps/save-analysis-fail/foo.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#![ crate_name = "test" ]
#![crate_name = "test"]
#![feature(box_syntax)]
#![feature(rustc_private)]

#![allow(unused_must_use)]

extern crate graphviz;
// A simple rust project

Expand Down
6 changes: 3 additions & 3 deletions src/test/run-make-fulldeps/save-analysis-fail/krate2.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![ crate_name = "krate2" ]
#![ crate_type = "lib" ]
#![crate_name = "krate2"]
#![crate_type = "lib"]

use std::io::Write;

pub fn hello() {
std::io::stdout().write_all(b"hello world!\n");
let _ = std::io::stdout().write_all(b"hello world!\n");
}
4 changes: 3 additions & 1 deletion src/test/run-make-fulldeps/save-analysis/foo.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#![ crate_name = "test" ]
#![crate_name = "test"]
#![feature(box_syntax)]
#![feature(rustc_private)]
#![feature(associated_type_defaults)]
#![feature(external_doc)]

#![allow(unused_must_use)]

extern crate graphviz;
// A simple rust project

Expand Down
2 changes: 1 addition & 1 deletion src/test/run-make-fulldeps/save-analysis/krate2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
use std::io::Write;

pub fn hello() {
std::io::stdout().write_all(b"hello world!\n");
let _ = std::io::stdout().write_all(b"hello world!\n");
}
2 changes: 1 addition & 1 deletion src/test/ui/cross-crate/auxiliary/cci_capture_clause.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::sync::mpsc::{Receiver, channel};
pub fn foo<T:'static + Send + Clone>(x: T) -> Receiver<T> {
let (tx, rx) = channel();
thread::spawn(move|| {
tx.send(x.clone());
let _ = tx.send(x.clone());
});
rx
}
2 changes: 1 addition & 1 deletion src/test/ui/issues/auxiliary/issue-2723-a.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
pub unsafe fn f(xs: Vec<isize> ) {
xs.iter().map(|_x| { unsafe fn q() { panic!(); } }).collect::<Vec<()>>();
let _ = xs.iter().map(|_x| { unsafe fn q() { panic!(); } }).collect::<Vec<()>>();
}
2 changes: 1 addition & 1 deletion src/test/ui/issues/auxiliary/issue-9906.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ mod other {
}

pub fn foo(){
1+1;
let _ = 1 + 1;
}
}
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-48132.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ where I: Iterator,
}

fn main() {
outer(std::iter::once(&1).cloned());
let _ = outer(std::iter::once(&1).cloned());
}
Loading