Skip to content

Commit a62d00b

Browse files
varkorvarkor
committed
Extend #[must_use] lint to user-defined ADTs
1 parent e649e90 commit a62d00b

File tree

3 files changed

+238
-16
lines changed

3 files changed

+238
-16
lines changed

‎src/librustc_lint/unused.rs‎

Lines changed: 69 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
use rustc::hir::def::{Res, DefKind};
22
use rustc::hir::def_id::DefId;
3+
use rustc::hir::HirVec;
34
use rustc::lint;
45
use rustc::ty::{self, Ty};
6+
use rustc::ty::subst::Subst;
57
use rustc::ty::adjustment;
8+
use rustc::mir::interpret::{GlobalId, ConstValue};
69
use rustc_data_structures::fx::FxHashMap;
710
use lint::{LateContext, EarlyContext, LintContext, LintArray};
811
use lint::{LintPass, EarlyLintPass, LateLintPass};
@@ -151,8 +154,40 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
151154
let descr_pre = &format!("{}boxed ", descr_pre);
152155
check_must_use_ty(cx, boxed_ty, expr, span, descr_pre, descr_post, plural)
153156
}
154-
ty::Adt(def, _) => {
155-
check_must_use_def(cx, def.did, span, descr_pre, descr_post)
157+
ty::Adt(def, subst) => {
158+
// Check the type itself for `#[must_use]` annotations.
159+
let mut has_emitted = check_must_use_def(
160+
cx, def.did, span, descr_pre, descr_post);
161+
// Check any fields of the type for `#[must_use]` annotations.
162+
// We ignore ADTs with more than one variant for simplicity and to avoid
163+
// false positives.
164+
// Unions are also ignored (though in theory, we could lint if every field of
165+
// a union was `#[must_use]`).
166+
if def.variants.len() == 1 && !def.is_union() {
167+
let fields = match &expr.node {
168+
hir::ExprKind::Struct(_, fields, _) => {
169+
fields.iter().map(|f| &*f.expr).collect()
170+
}
171+
hir::ExprKind::Call(_, args) => args.iter().collect(),
172+
_ => HirVec::new(),
173+
};
174+
175+
for variant in &def.variants {
176+
for (i, field) in variant.fields.iter().enumerate() {
177+
let descr_post
178+
= &format!(" in field `{}`", field.ident.as_str());
179+
let ty = cx.tcx.type_of(field.did).subst(cx.tcx, subst);
180+
let (expr, span) = if let Some(&field) = fields.get(i) {
181+
(field, field.span)
182+
} else {
183+
(expr, span)
184+
};
185+
has_emitted |= check_must_use_ty(
186+
cx, ty, expr, span, descr_pre, descr_post, plural);
187+
}
188+
}
189+
}
190+
has_emitted
156191
}
157192
ty::Opaque(def, _) => {
158193
let mut has_emitted = false;
@@ -202,24 +237,42 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
202237
for (i, ty) in tys.iter().map(|k| k.expect_ty()).enumerate() {
203238
let descr_post = &format!(" in tuple element {}", i);
204239
let span = *spans.get(i).unwrap_or(&span);
205-
if check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, plural) {
206-
has_emitted = true;
207-
}
240+
has_emitted |= check_must_use_ty(
241+
cx, ty, expr, span, descr_pre, descr_post, plural);
208242
}
209243
has_emitted
210244
}
211-
ty::Array(ty, len) => match len.assert_usize(cx.tcx) {
212-
// If the array is definitely non-empty, we can do `#[must_use]` checking.
213-
Some(n) if n != 0 => {
214-
let descr_pre = &format!(
215-
"{}array{} of ",
216-
descr_pre,
217-
plural_suffix,
218-
);
219-
check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, true)
245+
ty::Array(ty, mut len) => {
246+
// Try to evaluate the length if it's unevaluated.
247+
if let ConstValue::Unevaluated(def_id, substs) = len.val {
248+
let instance = ty::Instance::resolve(
249+
cx.tcx.global_tcx(),
250+
cx.param_env,
251+
def_id,
252+
substs,
253+
).unwrap();
254+
let global_id = GlobalId {
255+
instance,
256+
promoted: None
257+
};
258+
if let Ok(ct) = cx.tcx.const_eval(cx.param_env.and(global_id)) {
259+
len = ct;
260+
}
261+
}
262+
263+
match len.assert_usize(cx.tcx) {
264+
// If the array is definitely non-empty, we can do `#[must_use]` checking.
265+
Some(n) if n != 0 => {
266+
let descr_pre = &format!(
267+
"{}array{} of ",
268+
descr_pre,
269+
plural_suffix,
270+
);
271+
check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, true)
272+
}
273+
// Otherwise, we don't lint, to avoid false positives.
274+
_ => false,
220275
}
221-
// Otherwise, we don't lint, to avoid false positives.
222-
_ => false,
223276
}
224277
_ => false,
225278
}

‎src/test/ui/lint/must_use-adt.rs‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
#![deny(unused_must_use)]
2+
3+
#[must_use]
4+
struct S;
5+
6+
#[must_use]
7+
trait A {}
8+
9+
struct B;
10+
11+
impl A for B {}
12+
13+
struct T(S);
14+
15+
struct U {
16+
x: (),
17+
y: T,
18+
}
19+
20+
struct V {
21+
a: S,
22+
}
23+
24+
struct W {
25+
w: [(u8, Box<dyn A>); 2],
26+
x: u32,
27+
y: (B, B),
28+
z: (S, S),
29+
e: [(u8, Box<dyn A>); 2],
30+
f: S,
31+
}
32+
33+
fn get_v() -> V {
34+
V { a: S }
35+
}
36+
37+
struct Z([(u8, Box<dyn A>); 2]);
38+
39+
fn get_wrapped_arr() -> Z {
40+
Z([(0, Box::new(B)), (0, Box::new(B))])
41+
}
42+
43+
fn get_tuple_arr() -> ([(u8, Box<dyn A>); 2],) {
44+
([(0, Box::new(B)), (0, Box::new(B))],)
45+
}
46+
47+
struct R<T> {
48+
r: T
49+
}
50+
51+
struct List<T>(T, Option<Box<Self>>);
52+
53+
fn main() {
54+
S; //~ ERROR unused `S` that must be used
55+
T(S); //~ ERROR unused `S` in field `0` that must be used
56+
U { x: (), y: T(S) }; //~ ERROR unused `S` in field `0` that must be used
57+
get_v(); //~ ERROR unused `S` in field `a` that must be used
58+
V { a: S }; //~ ERROR unused `S` in field `a` that must be used
59+
W {
60+
w: [(0, Box::new(B)), (0, Box::new(B))],
61+
//~^ ERROR unused array of boxed `A` trait objects in tuple element 1 that must be used
62+
x: 0,
63+
y: (B, B),
64+
z: (S, S),
65+
//~^ unused `S` in tuple element 0 that must be used
66+
//~^^ unused `S` in tuple element 1 that must be used
67+
e: [(0, Box::new(B)), (0, Box::new(B))],
68+
//~^ unused array of boxed `A` trait objects in tuple element 1 that must be used
69+
f: S, //~ ERROR unused `S` in field `f` that must be used
70+
};
71+
get_wrapped_arr();
72+
//~^ ERROR unused array of boxed `A` trait objects in tuple element 1 that must be use
73+
get_tuple_arr();
74+
//~^ ERROR unused array of boxed `A` trait objects in tuple element 1 that must be used
75+
R { r: S }; //~ ERROR unused `S` in field `r` that must be used
76+
List(S, Some(Box::new(List(S, None)))); //~ ERROR unused `S` in field `0` that must be used
77+
}

‎src/test/ui/lint/must_use-adt.stderr‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
error: unused `S` that must be used
2+
--> $DIR/must_use-adt.rs:54:5
3+
|
4+
LL | S;
5+
| ^^
6+
|
7+
note: lint level defined here
8+
--> $DIR/must_use-adt.rs:1:9
9+
|
10+
LL | #![deny(unused_must_use)]
11+
| ^^^^^^^^^^^^^^^
12+
13+
error: unused `S` in field `0` that must be used
14+
--> $DIR/must_use-adt.rs:55:7
15+
|
16+
LL | T(S);
17+
| ^
18+
19+
error: unused `S` in field `0` that must be used
20+
--> $DIR/must_use-adt.rs:56:21
21+
|
22+
LL | U { x: (), y: T(S) };
23+
| ^
24+
25+
error: unused `S` in field `a` that must be used
26+
--> $DIR/must_use-adt.rs:57:5
27+
|
28+
LL | get_v();
29+
| ^^^^^^^^
30+
31+
error: unused `S` in field `a` that must be used
32+
--> $DIR/must_use-adt.rs:58:12
33+
|
34+
LL | V { a: S };
35+
| ^
36+
37+
error: unused array of boxed `A` trait objects in tuple element 1 that must be used
38+
--> $DIR/must_use-adt.rs:60:12
39+
|
40+
LL | w: [(0, Box::new(B)), (0, Box::new(B))],
41+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
42+
43+
error: unused `S` in tuple element 0 that must be used
44+
--> $DIR/must_use-adt.rs:64:13
45+
|
46+
LL | z: (S, S),
47+
| ^
48+
49+
error: unused `S` in tuple element 1 that must be used
50+
--> $DIR/must_use-adt.rs:64:16
51+
|
52+
LL | z: (S, S),
53+
| ^
54+
55+
error: unused array of boxed `A` trait objects in tuple element 1 that must be used
56+
--> $DIR/must_use-adt.rs:67:12
57+
|
58+
LL | e: [(0, Box::new(B)), (0, Box::new(B))],
59+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
60+
61+
error: unused `S` in field `f` that must be used
62+
--> $DIR/must_use-adt.rs:69:12
63+
|
64+
LL | f: S,
65+
| ^
66+
67+
error: unused array of boxed `A` trait objects in tuple element 1 that must be used
68+
--> $DIR/must_use-adt.rs:71:5
69+
|
70+
LL | get_wrapped_arr();
71+
| ^^^^^^^^^^^^^^^^^^
72+
73+
error: unused array of boxed `A` trait objects in tuple element 1 that must be used
74+
--> $DIR/must_use-adt.rs:73:5
75+
|
76+
LL | get_tuple_arr();
77+
| ^^^^^^^^^^^^^^^^
78+
79+
error: unused `S` in field `r` that must be used
80+
--> $DIR/must_use-adt.rs:75:12
81+
|
82+
LL | R { r: S };
83+
| ^
84+
85+
error: unused `S` in field `0` that must be used
86+
--> $DIR/must_use-adt.rs:76:10
87+
|
88+
LL | List(S, Some(Box::new(List(S, None))));
89+
| ^
90+
91+
error: aborting due to 14 previous errors
92+

0 commit comments

Comments
 (0)