Skip to content

Commit b2cc4d5

Browse files
committed
must_use: make the check for trivial types cleaner
This removes `Suppressed` as a must use cause that could be nested in another (we did not ever return this, but now it's encoded in the types). I also renamed Suppressed->Trvial and added some docs to clear confusion.
1 parent 696a105 commit b2cc4d5

1 file changed

Lines changed: 73 additions & 35 deletions

File tree

compiler/rustc_lint/src/unused/must_use.rs

Lines changed: 73 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,39 @@ declare_lint! {
8787

8888
declare_lint_pass!(UnusedResults => [UNUSED_MUST_USE, UNUSED_RESULTS]);
8989

90+
/// Must the type be used?
91+
#[derive(Debug)]
92+
pub enum IsTyMustUse {
93+
/// Yes, `MustUsePath` contains an explanation for why the type must be used.
94+
/// This will result in `unused_must_use` lint.
95+
Yes(MustUsePath),
96+
/// No, an ordinary type that may be ignored.
97+
/// This will result in `unused_results` lint.
98+
No,
99+
/// No, the type is trivial and thus should always be ignored.
100+
/// (this suppresses `unused_results` lint)
101+
Trivial,
102+
}
103+
104+
impl IsTyMustUse {
105+
fn map(self, f: impl FnOnce(MustUsePath) -> MustUsePath) -> Self {
106+
match self {
107+
Self::Yes(must_use_path) => Self::Yes(f(must_use_path)),
108+
_ => self,
109+
}
110+
}
111+
112+
fn yes(self) -> Option<MustUsePath> {
113+
match self {
114+
Self::Yes(must_use_path) => Some(must_use_path),
115+
_ => None,
116+
}
117+
}
118+
}
119+
90120
/// A path through a type to a `must_use` source. Contains useful info for the lint.
91121
#[derive(Debug)]
92122
pub enum MustUsePath {
93-
/// Suppress must_use checking.
94-
Suppressed,
95123
/// The root of the normal `must_use` lint with an optional message.
96124
Def(Span, DefId, Option<Symbol>),
97125
Boxed(Box<Self>),
@@ -114,15 +142,15 @@ pub fn is_ty_must_use<'tcx>(
114142
ty: Ty<'tcx>,
115143
expr: &hir::Expr<'_>,
116144
span: Span,
117-
) -> Option<MustUsePath> {
145+
) -> IsTyMustUse {
118146
if ty.is_unit() {
119-
return Some(MustUsePath::Suppressed);
147+
return IsTyMustUse::Trivial;
120148
}
121149
let parent_mod_did = cx.tcx.parent_module(expr.hir_id).to_def_id();
122150
let is_uninhabited =
123151
|t: Ty<'tcx>| !t.is_inhabited_from(cx.tcx, parent_mod_did, cx.typing_env());
124152
if is_uninhabited(ty) {
125-
return Some(MustUsePath::Suppressed);
153+
return IsTyMustUse::Trivial;
126154
}
127155

128156
match *ty.kind() {
@@ -140,17 +168,19 @@ pub fn is_ty_must_use<'tcx>(
140168
&& args.type_at(0).is_unit()
141169
&& is_uninhabited(args.type_at(1)) =>
142170
{
143-
Some(MustUsePath::Suppressed)
171+
IsTyMustUse::Trivial
144172
}
145173
// Suppress warnings on `ControlFlow<Uninhabited, ()>` (e.g. `ControlFlow<!, ()>`).
146174
ty::Adt(def, args)
147175
if cx.tcx.is_diagnostic_item(sym::ControlFlow, def.did())
148176
&& args.type_at(1).is_unit()
149177
&& is_uninhabited(args.type_at(0)) =>
150178
{
151-
Some(MustUsePath::Suppressed)
179+
IsTyMustUse::Trivial
180+
}
181+
ty::Adt(def, _) => {
182+
is_def_must_use(cx, def.did(), span).map_or(IsTyMustUse::No, IsTyMustUse::Yes)
152183
}
153-
ty::Adt(def, _) => is_def_must_use(cx, def.did(), span),
154184
ty::Alias(ty::Opaque | ty::Projection, ty::AliasTy { def_id: def, .. }) => {
155185
elaborate(cx.tcx, cx.tcx.explicit_item_self_bounds(def).iter_identity_copied())
156186
// We only care about self bounds for the impl-trait
@@ -168,16 +198,20 @@ pub fn is_ty_must_use<'tcx>(
168198
}
169199
})
170200
.map(|inner| MustUsePath::Opaque(Box::new(inner)))
201+
.map_or(IsTyMustUse::No, IsTyMustUse::Yes)
171202
}
172-
ty::Dynamic(binders, _) => binders.iter().find_map(|predicate| {
173-
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder() {
174-
let def_id = trait_ref.def_id;
175-
is_def_must_use(cx, def_id, span)
176-
.map(|inner| MustUsePath::TraitObject(Box::new(inner)))
177-
} else {
178-
None
179-
}
180-
}),
203+
ty::Dynamic(binders, _) => binders
204+
.iter()
205+
.find_map(|predicate| {
206+
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder() {
207+
let def_id = trait_ref.def_id;
208+
is_def_must_use(cx, def_id, span)
209+
.map(|inner| MustUsePath::TraitObject(Box::new(inner)))
210+
} else {
211+
None
212+
}
213+
})
214+
.map_or(IsTyMustUse::No, IsTyMustUse::Yes),
181215
ty::Tuple(tys) => {
182216
let elem_exprs = if let hir::ExprKind::Tup(elem_exprs) = expr.kind {
183217
debug_assert_eq!(elem_exprs.len(), tys.len());
@@ -194,35 +228,38 @@ pub fn is_ty_must_use<'tcx>(
194228
.zip(elem_exprs)
195229
.enumerate()
196230
.filter_map(|(i, (ty, expr))| {
197-
is_ty_must_use(cx, ty, expr, expr.span).map(|path| (i, path))
231+
is_ty_must_use(cx, ty, expr, expr.span).yes().map(|path| (i, path))
198232
})
199233
.collect::<Vec<_>>();
200234

201235
if !nested_must_use.is_empty() {
202-
Some(MustUsePath::TupleElement(nested_must_use))
236+
IsTyMustUse::Yes(MustUsePath::TupleElement(nested_must_use))
203237
} else {
204-
None
238+
IsTyMustUse::No
205239
}
206240
}
207241
ty::Array(ty, len) => match len.try_to_target_usize(cx.tcx) {
208242
// If the array is empty we don't lint, to avoid false positives
209-
Some(0) | None => None,
243+
Some(0) | None => IsTyMustUse::No,
210244
// If the array is definitely non-empty, we can do `#[must_use]` checking.
211245
Some(len) => is_ty_must_use(cx, ty, expr, span)
212246
.map(|inner| MustUsePath::Array(Box::new(inner), len)),
213247
},
214-
ty::Closure(..) | ty::CoroutineClosure(..) => Some(MustUsePath::Closure(span)),
248+
ty::Closure(..) | ty::CoroutineClosure(..) => IsTyMustUse::Yes(MustUsePath::Closure(span)),
215249
ty::Coroutine(def_id, ..) => {
216250
// async fn should be treated as "implementor of `Future`"
217-
let must_use = if cx.tcx.coroutine_is_async(def_id) {
218-
let def_id = cx.tcx.lang_items().future_trait()?;
219-
is_def_must_use(cx, def_id, span).map(|inner| MustUsePath::Opaque(Box::new(inner)))
251+
if cx.tcx.coroutine_is_async(def_id)
252+
&& let Some(def_id) = cx.tcx.lang_items().future_trait()
253+
{
254+
IsTyMustUse::Yes(MustUsePath::Opaque(Box::new(
255+
is_def_must_use(cx, def_id, span)
256+
.expect("future trait is marked as `#[must_use]`"),
257+
)))
220258
} else {
221-
None
222-
};
223-
must_use.or(Some(MustUsePath::Coroutine(span)))
259+
IsTyMustUse::Yes(MustUsePath::Coroutine(span))
260+
}
224261
}
225-
_ => None,
262+
_ => IsTyMustUse::No,
226263
}
227264
}
228265

@@ -269,17 +306,18 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
269306
let ty = cx.typeck_results().expr_ty(expr);
270307

271308
let must_use_result = is_ty_must_use(cx, ty, expr, expr.span);
272-
let type_lint_emitted_or_suppressed = match must_use_result {
273-
Some(path) => {
309+
let type_lint_emitted_or_trivial = match must_use_result {
310+
IsTyMustUse::Yes(path) => {
274311
emit_must_use_untranslated(cx, &path, "", "", 1, false, expr_is_from_block);
275312
true
276313
}
277-
None => false,
314+
IsTyMustUse::Trivial => true,
315+
IsTyMustUse::No => false,
278316
};
279317

280318
let fn_warned = check_fn_must_use(cx, expr, expr_is_from_block);
281319

282-
if !fn_warned && type_lint_emitted_or_suppressed {
320+
if !fn_warned && type_lint_emitted_or_trivial {
283321
// We don't warn about unused unit or uninhabited types.
284322
// (See https://github.com/rust-lang/rust/issues/43806 for details.)
285323
return;
@@ -349,7 +387,8 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
349387
op_warned = true;
350388
}
351389

352-
if !(type_lint_emitted_or_suppressed || fn_warned || op_warned) {
390+
// Only emit unused results lint if we haven't emitted any of the more specific lints and the expression type is non trivial.
391+
if !(type_lint_emitted_or_trivial || fn_warned || op_warned) {
353392
cx.emit_span_lint(UNUSED_RESULTS, s.span, UnusedResult { ty });
354393
}
355394
}
@@ -431,7 +470,6 @@ fn emit_must_use_untranslated(
431470
let plural_suffix = pluralize!(plural_len);
432471

433472
match path {
434-
MustUsePath::Suppressed => {}
435473
MustUsePath::Boxed(path) => {
436474
let descr_pre = &format!("{descr_pre}boxed ");
437475
emit_must_use_untranslated(

0 commit comments

Comments
 (0)