Skip to content

Commit b11a231

Browse files
JonathanBrouwerJonathanBrouwer
committed
Port #[must_use] to new attribute parsing infrastructure
Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>
1 parent 15c701f commit b11a231

File tree

20 files changed

+153
-69
lines changed

20 files changed

+153
-69
lines changed

‎compiler/rustc_attr_data_structures/src/attributes.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,17 @@ pub enum AttributeKind {
233233

234234
/// Represents `#[rustc_macro_transparency]`.
235235
MacroTransparency(Transparency),
236+
237+
/// Represents `#[must_use]`.
238+
MustUse {
239+
span: Span,
240+
/// must_use can optionally have a reason: `#[must_use = "reason this must be used"]`
241+
reason: Option<Symbol>,
242+
},
243+
236244
/// Represents `#[optimize(size|speed)]`
237245
Optimize(OptimizeAttr, Span),
246+
238247
/// Represents [`#[repr]`](https://doc.rust-lang.org/stable/reference/type-layout.html#representations).
239248
Repr(ThinVec<(ReprAttr, Span)>),
240249

‎compiler/rustc_attr_parsing/src/attributes/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ pub(crate) mod confusables;
3333
pub(crate) mod deprecation;
3434
pub(crate) mod inline;
3535
pub(crate) mod lint_helpers;
36+
pub(crate) mod must_use;
3637
pub(crate) mod repr;
3738
pub(crate) mod stability;
3839
pub(crate) mod transparency;

‎compiler/rustc_attr_parsing/src/attributes/must_use.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
use rustc_attr_data_structures::AttributeKind;
2+
use rustc_errors::DiagArgValue;
3+
use rustc_feature::{AttributeTemplate, template};
4+
use rustc_span::{Symbol, sym};
5+
6+
use crate::attributes::{AttributeOrder, OnDuplicate, SingleAttributeParser};
7+
use crate::context::{AcceptContext, Stage};
8+
use crate::parser::ArgParser;
9+
use crate::session_diagnostics;
10+
11+
pub(crate) struct MustUseParser;
12+
13+
impl<S: Stage> SingleAttributeParser<S> for MustUseParser {
14+
const PATH: &[Symbol] = &[sym::must_use];
15+
const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepLast;
16+
const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::WarnButFutureError;
17+
const TEMPLATE: AttributeTemplate = template!(Word, NameValueStr: "reason");
18+
19+
fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
20+
Some(AttributeKind::MustUse {
21+
span: cx.attr_span,
22+
reason: match args {
23+
ArgParser::NoArgs => None,
24+
ArgParser::NameValue(name_value) => name_value.value_as_str(),
25+
ArgParser::List(_) => {
26+
let suggestions =
27+
<Self as SingleAttributeParser<S>>::TEMPLATE.suggestions(false, "must_use");
28+
cx.emit_err(session_diagnostics::MustUseIllFormedAttributeInput {
29+
num_suggestions: suggestions.len(),
30+
suggestions: DiagArgValue::StrListSepByAnd(
31+
suggestions.into_iter().map(|s| format!("`{s}`").into()).collect(),
32+
),
33+
span: cx.attr_span,
34+
});
35+
return None;
36+
}
37+
},
38+
})
39+
}
40+
}

‎compiler/rustc_attr_parsing/src/context.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use crate::attributes::confusables::ConfusablesParser;
2020
use crate::attributes::deprecation::DeprecationParser;
2121
use crate::attributes::inline::{InlineParser, RustcForceInlineParser};
2222
use crate::attributes::lint_helpers::AsPtrParser;
23+
use crate::attributes::must_use::MustUseParser;
2324
use crate::attributes::repr::{AlignParser, ReprParser};
2425
use crate::attributes::stability::{
2526
BodyStabilityParser, ConstStabilityIndirectParser, ConstStabilityParser, StabilityParser,
@@ -110,6 +111,7 @@ attribute_parsers!(
110111
Single<ConstStabilityIndirectParser>,
111112
Single<DeprecationParser>,
112113
Single<InlineParser>,
114+
Single<MustUseParser>,
113115
Single<OptimizeParser>,
114116
Single<RustcForceInlineParser>,
115117
Single<TransparencyParser>,

‎compiler/rustc_attr_parsing/src/session_diagnostics.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,15 @@ pub(crate) struct IllFormedAttributeInput {
436436
pub suggestions: DiagArgValue,
437437
}
438438

439+
#[derive(Diagnostic)]
440+
#[diag(attr_parsing_ill_formed_attribute_input)]
441+
pub(crate) struct MustUseIllFormedAttributeInput {
442+
#[primary_span]
443+
pub span: Span,
444+
pub num_suggestions: usize,
445+
pub suggestions: DiagArgValue,
446+
}
447+
439448
#[derive(Diagnostic)]
440449
#[diag(attr_parsing_stability_outside_std, code = E0734)]
441450
pub(crate) struct StabilityOutsideStd {

‎compiler/rustc_lint/src/unused.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::iter;
22

33
use rustc_ast::util::{classify, parser};
44
use rustc_ast::{self as ast, ExprKind, HasAttrs as _, StmtKind};
5+
use rustc_attr_data_structures::{AttributeKind, find_attr};
56
use rustc_errors::{MultiSpan, pluralize};
67
use rustc_hir::def::{DefKind, Res};
78
use rustc_hir::def_id::DefId;
@@ -368,10 +369,12 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
368369
}
369370

370371
fn is_def_must_use(cx: &LateContext<'_>, def_id: DefId, span: Span) -> Option<MustUsePath> {
371-
if let Some(attr) = cx.tcx.get_attr(def_id, sym::must_use) {
372+
if let Some(reason) = find_attr!(
373+
cx.tcx.get_all_attrs(def_id),
374+
AttributeKind::MustUse { reason, .. } => reason
375+
) {
372376
// check for #[must_use = "..."]
373-
let reason = attr.value_str();
374-
Some(MustUsePath::Def(span, def_id, reason))
377+
Some(MustUsePath::Def(span, def_id, *reason))
375378
} else {
376379
None
377380
}

‎compiler/rustc_parse/src/validate_attr.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ fn emit_malformed_attribute(
293293
| sym::deprecated
294294
| sym::optimize
295295
| sym::cold
296+
| sym::must_use
296297
) {
297298
return;
298299
}

‎compiler/rustc_passes/src/check_attr.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
163163
Attribute::Parsed(AttributeKind::AsPtr(attr_span)) => {
164164
self.check_applied_to_fn_or_method(hir_id, *attr_span, span, target)
165165
}
166+
Attribute::Parsed(AttributeKind::MustUse { span, .. }) => {
167+
self.check_must_use(hir_id, *span, target)
168+
}
166169
Attribute::Unparsed(_) => {
167170
match attr.path().as_slice() {
168171
[sym::diagnostic, sym::do_not_recommend, ..] => {
@@ -235,7 +238,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
235238
| [sym::const_trait, ..] => self.check_must_be_applied_to_trait(attr, span, target),
236239
[sym::collapse_debuginfo, ..] => self.check_collapse_debuginfo(attr, span, target),
237240
[sym::must_not_suspend, ..] => self.check_must_not_suspend(attr, span, target),
238-
[sym::must_use, ..] => self.check_must_use(hir_id, attr, target),
239241
[sym::may_dangle, ..] => self.check_may_dangle(hir_id, attr),
240242
[sym::rustc_pass_by_value, ..] => self.check_pass_by_value(attr, span, target),
241243
[sym::rustc_allow_incoherent_impl, ..] => {
@@ -688,7 +690,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
688690
AttributeKind::Deprecation { .. }
689691
| AttributeKind::Repr { .. }
690692
| AttributeKind::Align { .. }
691-
| AttributeKind::Cold(..),
693+
| AttributeKind::Cold(..)
694+
| AttributeKind::MustUse { .. },
692695
) => {
693696
continue;
694697
}
@@ -1563,7 +1566,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
15631566
}
15641567

15651568
/// Warns against some misuses of `#[must_use]`
1566-
fn check_must_use(&self, hir_id: HirId, attr: &Attribute, target: Target) {
1569+
fn check_must_use(&self, hir_id: HirId, attr_span: Span, target: Target) {
15671570
if matches!(
15681571
target,
15691572
Target::Fn
@@ -1603,7 +1606,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
16031606
self.tcx.emit_node_span_lint(
16041607
UNUSED_ATTRIBUTES,
16051608
hir_id,
1606-
attr.span(),
1609+
attr_span,
16071610
errors::MustUseNoEffect { article, target },
16081611
);
16091612
}

‎src/rustdoc-json-types/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ pub type FxHashMap<K, V> = HashMap<K, V>; // re-export for use in src/librustdoc
3737
// will instead cause conflicts. See #94591 for more. (This paragraph and the "Latest feature" line
3838
// are deliberately not in a doc comment, because they need not be in public docs.)
3939
//
40-
// Latest feature: Pretty printing of cold attributes changed
41-
pub const FORMAT_VERSION: u32 = 50;
40+
// Latest feature: Pretty printing of must_use attributes changed
41+
pub const FORMAT_VERSION: u32 = 51;
4242

4343
/// The root of the emitted JSON blob.
4444
///

‎src/tools/clippy/clippy_lints/src/functions/must_use.rs

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,17 @@ use clippy_utils::ty::is_must_use_ty;
1515
use clippy_utils::visitors::for_each_expr_without_closures;
1616
use clippy_utils::{return_ty, trait_ref_of_method};
1717
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
18+
use rustc_attr_data_structures::AttributeKind;
19+
use rustc_span::Symbol;
20+
use rustc_attr_data_structures::find_attr;
1821

1922
use core::ops::ControlFlow;
2023

2124
use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT};
2225

2326
pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
2427
let attrs = cx.tcx.hir_attrs(item.hir_id());
25-
let attr = cx.tcx.get_attr(item.owner_id, sym::must_use);
28+
let attr = find_attr!(cx.tcx.hir_attrs(item.hir_id()), AttributeKind::MustUse { span, reason } => (span, reason));
2629
if let hir::ItemKind::Fn {
2730
ref sig,
2831
body: ref body_id,
@@ -31,8 +34,8 @@ pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>
3134
{
3235
let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id);
3336
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
34-
if let Some(attr) = attr {
35-
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, sig);
37+
if let Some((attr_span, reason)) = attr {
38+
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, *attr_span, *reason, attrs, sig);
3639
} else if is_public && !is_proc_macro(attrs) && !attrs.iter().any(|a| a.has_name(sym::no_mangle)) {
3740
check_must_use_candidate(
3841
cx,
@@ -52,9 +55,9 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp
5255
let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id);
5356
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
5457
let attrs = cx.tcx.hir_attrs(item.hir_id());
55-
let attr = cx.tcx.get_attr(item.owner_id, sym::must_use);
56-
if let Some(attr) = attr {
57-
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, sig);
58+
let attr = find_attr!(cx.tcx.hir_attrs(item.hir_id()), AttributeKind::MustUse { span, reason } => (span, reason));
59+
if let Some((attr_span, reason)) = attr {
60+
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, *attr_span, *reason, attrs, sig);
5861
} else if is_public && !is_proc_macro(attrs) && trait_ref_of_method(cx, item.owner_id).is_none() {
5962
check_must_use_candidate(
6063
cx,
@@ -75,9 +78,9 @@ pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Tr
7578
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
7679

7780
let attrs = cx.tcx.hir_attrs(item.hir_id());
78-
let attr = cx.tcx.get_attr(item.owner_id, sym::must_use);
79-
if let Some(attr) = attr {
80-
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, sig);
81+
let attr = find_attr!(cx.tcx.hir_attrs(item.hir_id()), AttributeKind::MustUse { span, reason } => (span, reason));
82+
if let Some((attr_span, reason)) = attr {
83+
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, *attr_span, *reason, attrs, sig);
8184
} else if let hir::TraitFn::Provided(eid) = *eid {
8285
let body = cx.tcx.hir_body(eid);
8386
if attr.is_none() && is_public && !is_proc_macro(attrs) {
@@ -103,7 +106,8 @@ fn check_needless_must_use(
103106
item_id: hir::OwnerId,
104107
item_span: Span,
105108
fn_header_span: Span,
106-
attr: &Attribute,
109+
attr_span: Span,
110+
reason: Option<Symbol>,
107111
attrs: &[Attribute],
108112
sig: &FnSig<'_>,
109113
) {
@@ -119,7 +123,7 @@ fn check_needless_must_use(
119123
"this unit-returning function has a `#[must_use]` attribute",
120124
|diag| {
121125
diag.span_suggestion(
122-
attr.span(),
126+
attr_span,
123127
"remove the attribute",
124128
"",
125129
Applicability::MachineApplicable,
@@ -137,11 +141,11 @@ fn check_needless_must_use(
137141
MUST_USE_UNIT,
138142
fn_header_span,
139143
"this unit-returning function has a `#[must_use]` attribute",
140-
Some(attr.span()),
144+
Some(attr_span),
141145
"remove `must_use`",
142146
);
143147
}
144-
} else if attr.value_str().is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) {
148+
} else if reason.is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) {
145149
// Ignore async functions unless Future::Output type is a must_use type
146150
if sig.header.is_async() {
147151
let infcx = cx.tcx.infer_ctxt().build(cx.typing_mode());

‎src/tools/clippy/clippy_lints/src/return_self_not_must_use.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ use rustc_hir::intravisit::FnKind;
66
use rustc_hir::{Body, FnDecl, OwnerId, TraitItem, TraitItemKind};
77
use rustc_lint::{LateContext, LateLintPass, LintContext};
88
use rustc_session::declare_lint_pass;
9-
use rustc_span::{Span, sym};
9+
use rustc_span::{Span};
10+
use rustc_attr_data_structures::AttributeKind;
11+
use rustc_attr_data_structures::find_attr;
1012

1113
declare_clippy_lint! {
1214
/// ### What it does
@@ -74,7 +76,10 @@ fn check_method(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_def: LocalDefId, spa
7476
// We only show this warning for public exported methods.
7577
&& cx.effective_visibilities.is_exported(fn_def)
7678
// We don't want to emit this lint if the `#[must_use]` attribute is already there.
77-
&& !cx.tcx.hir_attrs(owner_id.into()).iter().any(|attr| attr.has_name(sym::must_use))
79+
&& !find_attr!(
80+
cx.tcx.hir_attrs(owner_id.into()),
81+
AttributeKind::MustUse { .. }
82+
)
7883
&& cx.tcx.visibility(fn_def.to_def_id()).is_public()
7984
&& let ret_ty = return_ty(cx, owner_id)
8085
&& let self_arg = nth_arg(cx, owner_id, 0)

‎src/tools/clippy/clippy_utils/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1886,7 +1886,10 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
18861886
_ => None,
18871887
};
18881888

1889-
did.is_some_and(|did| cx.tcx.has_attr(did, sym::must_use))
1889+
did.is_some_and(|did| find_attr!(
1890+
cx.tcx.get_all_attrs(did),
1891+
AttributeKind::MustUse { ..}
1892+
))
18901893
}
18911894

18921895
/// Checks if a function's body represents the identity function. Looks for bodies of the form:

‎src/tools/clippy/clippy_utils/src/ty/mod.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ use rustc_trait_selection::traits::{Obligation, ObligationCause};
3131
use std::assert_matches::debug_assert_matches;
3232
use std::collections::hash_map::Entry;
3333
use std::iter;
34+
use rustc_attr_data_structures::find_attr;
35+
use rustc_attr_data_structures::AttributeKind;
3436

3537
use crate::path_res;
3638
use crate::paths::{PathNS, lookup_path_str};
@@ -326,8 +328,14 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
326328
// Returns whether the type has #[must_use] attribute
327329
pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
328330
match ty.kind() {
329-
ty::Adt(adt, _) => cx.tcx.has_attr(adt.did(), sym::must_use),
330-
ty::Foreign(did) => cx.tcx.has_attr(*did, sym::must_use),
331+
ty::Adt(adt, _) => find_attr!(
332+
cx.tcx.get_all_attrs(adt.did()),
333+
AttributeKind::MustUse { ..}
334+
),
335+
ty::Foreign(did) => find_attr!(
336+
cx.tcx.get_all_attrs(*did),
337+
AttributeKind::MustUse { ..}
338+
),
331339
ty::Slice(ty) | ty::Array(ty, _) | ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => {
332340
// for the Array case we don't need to care for the len == 0 case
333341
// because we don't want to lint functions returning empty arrays
@@ -337,7 +345,7 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
337345
ty::Alias(ty::Opaque, AliasTy { def_id, .. }) => {
338346
for (predicate, _) in cx.tcx.explicit_item_self_bounds(def_id).skip_binder() {
339347
if let ty::ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder()
340-
&& cx.tcx.has_attr(trait_predicate.trait_ref.def_id, sym::must_use)
348+
&& find_attr!(cx.tcx.get_all_attrs(trait_predicate.trait_ref.def_id), AttributeKind::MustUse { ..})
341349
{
342350
return true;
343351
}
@@ -347,7 +355,7 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
347355
ty::Dynamic(binder, _, _) => {
348356
for predicate in *binder {
349357
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder()
350-
&& cx.tcx.has_attr(trait_ref.def_id, sym::must_use)
358+
&& find_attr!(cx.tcx.get_all_attrs(trait_ref.def_id), AttributeKind::MustUse { ..})
351359
{
352360
return true;
353361
}

‎tests/rustdoc-json/attrs/must_use.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
#![no_std]
22

3-
//@ is "$.index[?(@.name=='example')].attrs" '["#[must_use]"]'
3+
//@ is "$.index[?(@.name=='example')].attrs" '["#[attr = MustUse]"]'
44
#[must_use]
55
pub fn example() -> impl Iterator<Item = i64> {}
66

7-
//@ is "$.index[?(@.name=='explicit_message')].attrs" '["#[must_use = \"does nothing if you do not use it\"]"]'
7+
//@ is "$.index[?(@.name=='explicit_message')].attrs" '["#[attr = MustUse {reason: \"does nothing if you do not use it\"}]"]'
88
#[must_use = "does nothing if you do not use it"]
99
pub fn explicit_message() -> impl Iterator<Item = i64> {}

‎tests/ui/attributes/malformed-must_use.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#[must_use()] //~ ERROR valid forms for the attribute are `#[must_use = "reason"]` and `#[must_use]`
2+
struct Test;
3+
4+
fn main() {}

‎tests/ui/attributes/malformed-must_use.stderr

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: valid forms for the attribute are `#[must_use = "reason"]` and `#[must_use]`
2+
--> $DIR/malformed-must_use.rs:1:1
3+
|
4+
LL | #[must_use()]
5+
| ^^^^^^^^^^^^^
6+
7+
error: aborting due to 1 previous error
8+

0 commit comments

Comments
 (0)