Skip to content

Commit b17cbe2

Browse files
JonathanBrouwerJonathanBrouwer
committed
Port #[must_use] to new attribute parsing infrastructure
Signed-off-by: Jonathan Brouwer <[email protected]>
1 parent 3b97f13 commit b17cbe2

File tree

13 files changed

+109
-44
lines changed

13 files changed

+109
-44
lines changed

‎compiler/rustc_attr_data_structures/src/attributes.rs

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

231231
/// Represents `#[rustc_macro_transparency]`.
232232
MacroTransparency(Transparency),
233+
234+
/// Represents `#[must_use]`.
235+
MustUse {
236+
span: Span,
237+
/// must_use can optionally have a reason: `#[must_use = "reason this must be used"]`
238+
reason: Option<Symbol>,
239+
},
240+
233241
/// Represents `#[optimize(size|speed)]`
234242
Optimize(OptimizeAttr, Span),
243+
235244
/// Represents [`#[repr]`](https://doc.rust-lang.org/stable/reference/type-layout.html#representations).
236245
Repr(ThinVec<(ReprAttr, Span)>),
237246

‎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: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
use rustc_attr_data_structures::AttributeKind;
2+
use rustc_feature::{AttributeTemplate, template};
3+
use rustc_span::{Symbol, sym};
4+
5+
use crate::attributes::{AttributeOrder, OnDuplicate, SingleAttributeParser};
6+
use crate::context::{AcceptContext, Stage};
7+
use crate::parser::ArgParser;
8+
9+
pub(crate) struct MustUseParser;
10+
11+
impl<S: Stage> SingleAttributeParser<S> for MustUseParser {
12+
const PATH: &[Symbol] = &[sym::must_use];
13+
const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepLast;
14+
const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::WarnButFutureError;
15+
const TEMPLATE: AttributeTemplate = template!(Word, NameValueStr: "Reason");
16+
17+
fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
18+
Some(AttributeKind::MustUse {
19+
span: cx.attr_span,
20+
reason: match args {
21+
ArgParser::NoArgs => None,
22+
ArgParser::NameValue(name_value) => name_value.value_as_str(),
23+
ArgParser::List(_) => todo!(),
24+
},
25+
})
26+
}
27+
}

‎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,
@@ -109,6 +110,7 @@ attribute_parsers!(
109110
Single<ConstStabilityIndirectParser>,
110111
Single<DeprecationParser>,
111112
Single<InlineParser>,
113+
Single<MustUseParser>,
112114
Single<OptimizeParser>,
113115
Single<RustcForceInlineParser>,
114116
Single<TransparencyParser>,

‎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_passes/src/check_attr.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
161161
Attribute::Parsed(AttributeKind::AsPtr(attr_span)) => {
162162
self.check_applied_to_fn_or_method(hir_id, *attr_span, span, target)
163163
}
164+
Attribute::Parsed(AttributeKind::MustUse { span, .. }) => {
165+
self.check_must_use(hir_id, *span, target)
166+
}
164167
Attribute::Unparsed(_) => {
165168
match attr.path().as_slice() {
166169
[sym::diagnostic, sym::do_not_recommend, ..] => {
@@ -233,7 +236,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
233236
| [sym::const_trait, ..] => self.check_must_be_applied_to_trait(attr, span, target),
234237
[sym::collapse_debuginfo, ..] => self.check_collapse_debuginfo(attr, span, target),
235238
[sym::must_not_suspend, ..] => self.check_must_not_suspend(attr, span, target),
236-
[sym::must_use, ..] => self.check_must_use(hir_id, attr, target),
237239
[sym::may_dangle, ..] => self.check_may_dangle(hir_id, attr),
238240
[sym::rustc_pass_by_value, ..] => self.check_pass_by_value(attr, span, target),
239241
[sym::rustc_allow_incoherent_impl, ..] => {
@@ -688,7 +690,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
688690
Attribute::Parsed(
689691
AttributeKind::Deprecation { .. }
690692
| AttributeKind::Repr { .. }
691-
| AttributeKind::Align { .. },
693+
| AttributeKind::Align { .. }
694+
| AttributeKind::MustUse { .. },
692695
) => {
693696
continue;
694697
}
@@ -1560,7 +1563,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
15601563
}
15611564

15621565
/// Warns against some misuses of `#[must_use]`
1563-
fn check_must_use(&self, hir_id: HirId, attr: &Attribute, target: Target) {
1566+
fn check_must_use(&self, hir_id: HirId, attr_span: Span, target: Target) {
15641567
if matches!(
15651568
target,
15661569
Target::Fn
@@ -1600,7 +1603,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
16001603
self.tcx.emit_node_span_lint(
16011604
UNUSED_ATTRIBUTES,
16021605
hir_id,
1603-
attr.span(),
1606+
attr_span,
16041607
errors::MustUseNoEffect { article, target },
16051608
);
16061609
}

‎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 optimize attributes changed
41-
pub const FORMAT_VERSION: u32 = 49;
40+
// Latest feature: Pretty printing of must_use attributes changed
41+
pub const FORMAT_VERSION: u32 = 50;
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/lint/unused/unused-attr-duplicate.stderr

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,19 +65,6 @@ LL | #[should_panic]
6565
| ^^^^^^^^^^^^^^^
6666
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
6767

68-
error: unused attribute
69-
--> $DIR/unused-attr-duplicate.rs:60:1
70-
|
71-
LL | #[must_use = "some message"]
72-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
73-
|
74-
note: attribute also specified here
75-
--> $DIR/unused-attr-duplicate.rs:59:1
76-
|
77-
LL | #[must_use]
78-
| ^^^^^^^^^^^
79-
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
80-
8168
error: unused attribute
8269
--> $DIR/unused-attr-duplicate.rs:66:1
8370
|
@@ -276,6 +263,19 @@ note: attribute also specified here
276263
LL | #[macro_export]
277264
| ^^^^^^^^^^^^^^^
278265

266+
error: unused attribute
267+
--> $DIR/unused-attr-duplicate.rs:60:1
268+
|
269+
LL | #[must_use = "some message"]
270+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
271+
|
272+
note: attribute also specified here
273+
--> $DIR/unused-attr-duplicate.rs:59:1
274+
|
275+
LL | #[must_use]
276+
| ^^^^^^^^^^^
277+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
278+
279279
error: unused attribute
280280
--> $DIR/unused-attr-duplicate.rs:74:1
281281
|

0 commit comments

Comments
 (0)