Skip to content

Commit 6ffb8f5

Browse files
borsbors
committed
Auto merge of #61100 - varkor:must_use-tuple-expr, r=cramertj
Apply #[must_use] lint to components of tuples Fixes #61061.
2 parents 61d286e + de2bf3a commit 6ffb8f5

File tree

3 files changed

+136
-42
lines changed

3 files changed

+136
-42
lines changed

‎src/librustc_lint/unused.rs

+72-42
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use rustc::hir::def::{Res, DefKind};
22
use rustc::hir::def_id::DefId;
33
use rustc::lint;
4-
use rustc::ty;
4+
use rustc::ty::{self, Ty};
55
use rustc::ty::adjustment;
66
use rustc_data_structures::fx::FxHashMap;
77
use lint::{LateContext, EarlyContext, LintContext, LintArray};
@@ -47,43 +47,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
4747
return;
4848
}
4949

50-
let t = cx.tables.expr_ty(&expr);
51-
let type_permits_lack_of_use = if t.is_unit()
52-
|| cx.tcx.is_ty_uninhabited_from(
53-
cx.tcx.hir().get_module_parent_by_hir_id(expr.hir_id), t)
54-
{
55-
true
56-
} else {
57-
match t.sty {
58-
ty::Adt(def, _) => check_must_use(cx, def.did, s.span, "", ""),
59-
ty::Opaque(def, _) => {
60-
let mut must_use = false;
61-
for (predicate, _) in &cx.tcx.predicates_of(def).predicates {
62-
if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate {
63-
let trait_ref = poly_trait_predicate.skip_binder().trait_ref;
64-
if check_must_use(cx, trait_ref.def_id, s.span, "implementer of ", "") {
65-
must_use = true;
66-
break;
67-
}
68-
}
69-
}
70-
must_use
71-
}
72-
ty::Dynamic(binder, _) => {
73-
let mut must_use = false;
74-
for predicate in binder.skip_binder().iter() {
75-
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate {
76-
if check_must_use(cx, trait_ref.def_id, s.span, "", " trait object") {
77-
must_use = true;
78-
break;
79-
}
80-
}
81-
}
82-
must_use
83-
}
84-
_ => false,
85-
}
86-
};
50+
let ty = cx.tables.expr_ty(&expr);
51+
let type_permits_lack_of_use = check_must_use_ty(cx, ty, &expr, s.span, "");
8752

8853
let mut fn_warned = false;
8954
let mut op_warned = false;
@@ -108,7 +73,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
10873
_ => None
10974
};
11075
if let Some(def_id) = maybe_def_id {
111-
fn_warned = check_must_use(cx, def_id, s.span, "return value of ", "");
76+
fn_warned = check_must_use_def(cx, def_id, s.span, "return value of ", "");
11277
} else if type_permits_lack_of_use {
11378
// We don't warn about unused unit or uninhabited types.
11479
// (See https://github.com/rust-lang/rust/issues/43806 for details.)
@@ -162,18 +127,83 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
162127
cx.span_lint(UNUSED_RESULTS, s.span, "unused result");
163128
}
164129

165-
fn check_must_use(
130+
// Returns whether an error has been emitted (and thus another does not need to be later).
131+
fn check_must_use_ty<'tcx>(
132+
cx: &LateContext<'_, 'tcx>,
133+
ty: Ty<'tcx>,
134+
expr: &hir::Expr,
135+
span: Span,
136+
descr_post_path: &str,
137+
) -> bool {
138+
if ty.is_unit() || cx.tcx.is_ty_uninhabited_from(
139+
cx.tcx.hir().get_module_parent_by_hir_id(expr.hir_id), ty)
140+
{
141+
return true;
142+
}
143+
144+
match ty.sty {
145+
ty::Adt(def, _) => check_must_use_def(cx, def.did, span, "", descr_post_path),
146+
ty::Opaque(def, _) => {
147+
let mut has_emitted = false;
148+
for (predicate, _) in &cx.tcx.predicates_of(def).predicates {
149+
if let ty::Predicate::Trait(ref poly_trait_predicate) = predicate {
150+
let trait_ref = poly_trait_predicate.skip_binder().trait_ref;
151+
let def_id = trait_ref.def_id;
152+
if check_must_use_def(cx, def_id, span, "implementer of ", "") {
153+
has_emitted = true;
154+
break;
155+
}
156+
}
157+
}
158+
has_emitted
159+
}
160+
ty::Dynamic(binder, _) => {
161+
let mut has_emitted = false;
162+
for predicate in binder.skip_binder().iter() {
163+
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate {
164+
let def_id = trait_ref.def_id;
165+
if check_must_use_def(cx, def_id, span, "", " trait object") {
166+
has_emitted = true;
167+
break;
168+
}
169+
}
170+
}
171+
has_emitted
172+
}
173+
ty::Tuple(ref tys) => {
174+
let mut has_emitted = false;
175+
let spans = if let hir::ExprKind::Tup(comps) = &expr.node {
176+
debug_assert_eq!(comps.len(), tys.len());
177+
comps.iter().map(|e| e.span).collect()
178+
} else {
179+
vec![]
180+
};
181+
for (i, ty) in tys.iter().map(|k| k.expect_ty()).enumerate() {
182+
let descr_post_path = &format!(" in tuple element {}", i);
183+
let span = *spans.get(i).unwrap_or(&span);
184+
if check_must_use_ty(cx, ty, expr, span, descr_post_path) {
185+
has_emitted = true;
186+
}
187+
}
188+
has_emitted
189+
}
190+
_ => false,
191+
}
192+
}
193+
194+
// Returns whether an error has been emitted (and thus another does not need to be later).
195+
fn check_must_use_def(
166196
cx: &LateContext<'_, '_>,
167197
def_id: DefId,
168-
sp: Span,
198+
span: Span,
169199
descr_pre_path: &str,
170200
descr_post_path: &str,
171201
) -> bool {
172202
for attr in cx.tcx.get_attrs(def_id).iter() {
173203
if attr.check_name(sym::must_use) {
174204
let msg = format!("unused {}`{}`{} that must be used",
175205
descr_pre_path, cx.tcx.def_path_str(def_id), descr_post_path);
176-
let mut err = cx.struct_span_lint(UNUSED_MUST_USE, sp, &msg);
206+
let mut err = cx.struct_span_lint(UNUSED_MUST_USE, span, &msg);
177207
// check for #[must_use = "..."]
178208
if let Some(note) = attr.value_str() {
179209
err.note(&note.as_str());

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

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#![deny(unused_must_use)]
2+
3+
fn foo() -> (Result<(), ()>, ()) {
4+
(Ok::<(), ()>(()), ())
5+
}
6+
7+
fn main() {
8+
(Ok::<(), ()>(()),); //~ ERROR unused `std::result::Result`
9+
10+
(Ok::<(), ()>(()), 0, Ok::<(), ()>(()), 5);
11+
//~^ ERROR unused `std::result::Result`
12+
//~^^ ERROR unused `std::result::Result`
13+
14+
foo(); //~ ERROR unused `std::result::Result`
15+
16+
((Err::<(), ()>(()), ()), ()); //~ ERROR unused `std::result::Result`
17+
}

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

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
error: unused `std::result::Result` in tuple element 0 that must be used
2+
--> $DIR/must_use-tuple.rs:8:6
3+
|
4+
LL | (Ok::<(), ()>(()),);
5+
| ^^^^^^^^^^^^^^^^
6+
|
7+
note: lint level defined here
8+
--> $DIR/must_use-tuple.rs:1:9
9+
|
10+
LL | #![deny(unused_must_use)]
11+
| ^^^^^^^^^^^^^^^
12+
= note: this `Result` may be an `Err` variant, which should be handled
13+
14+
error: unused `std::result::Result` in tuple element 0 that must be used
15+
--> $DIR/must_use-tuple.rs:10:6
16+
|
17+
LL | (Ok::<(), ()>(()), 0, Ok::<(), ()>(()), 5);
18+
| ^^^^^^^^^^^^^^^^
19+
|
20+
= note: this `Result` may be an `Err` variant, which should be handled
21+
22+
error: unused `std::result::Result` in tuple element 2 that must be used
23+
--> $DIR/must_use-tuple.rs:10:27
24+
|
25+
LL | (Ok::<(), ()>(()), 0, Ok::<(), ()>(()), 5);
26+
| ^^^^^^^^^^^^^^^^
27+
|
28+
= note: this `Result` may be an `Err` variant, which should be handled
29+
30+
error: unused `std::result::Result` in tuple element 0 that must be used
31+
--> $DIR/must_use-tuple.rs:14:5
32+
|
33+
LL | foo();
34+
| ^^^^^^
35+
|
36+
= note: this `Result` may be an `Err` variant, which should be handled
37+
38+
error: unused `std::result::Result` in tuple element 0 that must be used
39+
--> $DIR/must_use-tuple.rs:16:6
40+
|
41+
LL | ((Err::<(), ()>(()), ()), ());
42+
| ^^^^^^^^^^^^^^^^^^^^^^^
43+
|
44+
= note: this `Result` may be an `Err` variant, which should be handled
45+
46+
error: aborting due to 5 previous errors
47+

0 commit comments

Comments
 (0)