Skip to content

Port #[must_use] to new attribute parsing infrastructure #142780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions compiler/rustc_attr_data_structures/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,17 @@ pub enum AttributeKind {

/// Represents `#[rustc_macro_transparency]`.
MacroTransparency(Transparency),

/// Represents `#[must_use]`.
MustUse {
span: Span,
/// must_use can optionally have a reason: `#[must_use = "reason this must be used"]`
reason: Option<Symbol>,
},

/// Represents `#[optimize(size|speed)]`
Optimize(OptimizeAttr, Span),

/// Represents [`#[repr]`](https://doc.rust-lang.org/stable/reference/type-layout.html#representations).
Repr(ThinVec<(ReprAttr, Span)>),

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_attr_parsing/src/attributes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub(crate) mod confusables;
pub(crate) mod deprecation;
pub(crate) mod inline;
pub(crate) mod lint_helpers;
pub(crate) mod must_use;
pub(crate) mod repr;
pub(crate) mod stability;
pub(crate) mod transparency;
Expand Down
40 changes: 40 additions & 0 deletions compiler/rustc_attr_parsing/src/attributes/must_use.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use rustc_attr_data_structures::AttributeKind;
use rustc_errors::DiagArgValue;
use rustc_feature::{AttributeTemplate, template};
use rustc_span::{Symbol, sym};

use crate::attributes::{AttributeOrder, OnDuplicate, SingleAttributeParser};
use crate::context::{AcceptContext, Stage};
use crate::parser::ArgParser;
use crate::session_diagnostics;

pub(crate) struct MustUseParser;

impl<S: Stage> SingleAttributeParser<S> for MustUseParser {
const PATH: &[Symbol] = &[sym::must_use];
const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepLast;
const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::WarnButFutureError;
const TEMPLATE: AttributeTemplate = template!(Word, NameValueStr: "reason");

fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> {
Some(AttributeKind::MustUse {
span: cx.attr_span,
reason: match args {
ArgParser::NoArgs => None,
ArgParser::NameValue(name_value) => name_value.value_as_str(),
ArgParser::List(_) => {
let suggestions =
<Self as SingleAttributeParser<S>>::TEMPLATE.suggestions(false, "must_use");
cx.emit_err(session_diagnostics::MustUseIllFormedAttributeInput {
num_suggestions: suggestions.len(),
suggestions: DiagArgValue::StrListSepByAnd(
suggestions.into_iter().map(|s| format!("`{s}`").into()).collect(),
),
span: cx.attr_span,
});
return None;
}
},
})
}
}
2 changes: 2 additions & 0 deletions compiler/rustc_attr_parsing/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::attributes::confusables::ConfusablesParser;
use crate::attributes::deprecation::DeprecationParser;
use crate::attributes::inline::{InlineParser, RustcForceInlineParser};
use crate::attributes::lint_helpers::AsPtrParser;
use crate::attributes::must_use::MustUseParser;
use crate::attributes::repr::{AlignParser, ReprParser};
use crate::attributes::stability::{
BodyStabilityParser, ConstStabilityIndirectParser, ConstStabilityParser, StabilityParser,
Expand Down Expand Up @@ -110,6 +111,7 @@ attribute_parsers!(
Single<ConstStabilityIndirectParser>,
Single<DeprecationParser>,
Single<InlineParser>,
Single<MustUseParser>,
Single<OptimizeParser>,
Single<RustcForceInlineParser>,
Single<TransparencyParser>,
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_attr_parsing/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,15 @@ pub(crate) struct IllFormedAttributeInput {
pub suggestions: DiagArgValue,
}

#[derive(Diagnostic)]
#[diag(attr_parsing_ill_formed_attribute_input)]
pub(crate) struct MustUseIllFormedAttributeInput {
#[primary_span]
pub span: Span,
pub num_suggestions: usize,
pub suggestions: DiagArgValue,
}

#[derive(Diagnostic)]
#[diag(attr_parsing_stability_outside_std, code = E0734)]
pub(crate) struct StabilityOutsideStd {
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::iter;

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

fn is_def_must_use(cx: &LateContext<'_>, def_id: DefId, span: Span) -> Option<MustUsePath> {
if let Some(attr) = cx.tcx.get_attr(def_id, sym::must_use) {
if let Some(reason) = find_attr!(
cx.tcx.get_all_attrs(def_id),
AttributeKind::MustUse { reason, .. } => reason
) {
// check for #[must_use = "..."]
let reason = attr.value_str();
Some(MustUsePath::Def(span, def_id, reason))
Some(MustUsePath::Def(span, def_id, *reason))
} else {
None
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_parse/src/validate_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ fn emit_malformed_attribute(
| sym::deprecated
| sym::optimize
| sym::cold
| sym::must_use
) {
return;
}
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
Attribute::Parsed(AttributeKind::AsPtr(attr_span)) => {
self.check_applied_to_fn_or_method(hir_id, *attr_span, span, target)
}
Attribute::Parsed(AttributeKind::MustUse { span, .. }) => {
self.check_must_use(hir_id, *span, target)
}
Attribute::Unparsed(_) => {
match attr.path().as_slice() {
[sym::diagnostic, sym::do_not_recommend, ..] => {
Expand Down Expand Up @@ -235,7 +238,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
| [sym::const_trait, ..] => self.check_must_be_applied_to_trait(attr, span, target),
[sym::collapse_debuginfo, ..] => self.check_collapse_debuginfo(attr, span, target),
[sym::must_not_suspend, ..] => self.check_must_not_suspend(attr, span, target),
[sym::must_use, ..] => self.check_must_use(hir_id, attr, target),
[sym::may_dangle, ..] => self.check_may_dangle(hir_id, attr),
[sym::rustc_pass_by_value, ..] => self.check_pass_by_value(attr, span, target),
[sym::rustc_allow_incoherent_impl, ..] => {
Expand Down Expand Up @@ -688,7 +690,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
AttributeKind::Deprecation { .. }
| AttributeKind::Repr { .. }
| AttributeKind::Align { .. }
| AttributeKind::Cold(..),
| AttributeKind::Cold(..)
| AttributeKind::MustUse { .. },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added MustUse here but not removed it from the ALLOW_LIST above, because you also seem to not have done this for the other attributes. Is this correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just rewritten the naked attribute, so this shouldn't be necessary anymore. but dw, I'll do the rebase on there

) => {
continue;
}
Expand Down Expand Up @@ -1563,7 +1566,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}

/// Warns against some misuses of `#[must_use]`
fn check_must_use(&self, hir_id: HirId, attr: &Attribute, target: Target) {
fn check_must_use(&self, hir_id: HirId, attr_span: Span, target: Target) {
if matches!(
target,
Target::Fn
Expand Down Expand Up @@ -1603,7 +1606,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
self.tcx.emit_node_span_lint(
UNUSED_ATTRIBUTES,
hir_id,
attr.span(),
attr_span,
errors::MustUseNoEffect { article, target },
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/rustdoc-json-types/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ pub type FxHashMap<K, V> = HashMap<K, V>; // re-export for use in src/librustdoc
// will instead cause conflicts. See #94591 for more. (This paragraph and the "Latest feature" line
// are deliberately not in a doc comment, because they need not be in public docs.)
//
// Latest feature: improve handling of generic args
pub const FORMAT_VERSION: u32 = 51;
// Latest feature: Pretty printing of must_use attributes changed
pub const FORMAT_VERSION: u32 = 52;

/// The root of the emitted JSON blob.
///
Expand Down
30 changes: 17 additions & 13 deletions src/tools/clippy/clippy_lints/src/functions/must_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@ use clippy_utils::ty::is_must_use_ty;
use clippy_utils::visitors::for_each_expr_without_closures;
use clippy_utils::{return_ty, trait_ref_of_method};
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
use rustc_attr_data_structures::AttributeKind;
use rustc_span::Symbol;
use rustc_attr_data_structures::find_attr;

use core::ops::ControlFlow;

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

pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
let attrs = cx.tcx.hir_attrs(item.hir_id());
let attr = cx.tcx.get_attr(item.owner_id, sym::must_use);
let attr = find_attr!(cx.tcx.hir_attrs(item.hir_id()), AttributeKind::MustUse { span, reason } => (span, reason));
if let hir::ItemKind::Fn {
ref sig,
body: ref body_id,
Expand All @@ -31,8 +34,8 @@ pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>
{
let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id);
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
if let Some(attr) = attr {
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, sig);
if let Some((attr_span, reason)) = attr {
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, *attr_span, *reason, attrs, sig);
} else if is_public && !is_proc_macro(attrs) && !attrs.iter().any(|a| a.has_name(sym::no_mangle)) {
check_must_use_candidate(
cx,
Expand All @@ -52,9 +55,9 @@ pub(super) fn check_impl_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Imp
let is_public = cx.effective_visibilities.is_exported(item.owner_id.def_id);
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());
let attrs = cx.tcx.hir_attrs(item.hir_id());
let attr = cx.tcx.get_attr(item.owner_id, sym::must_use);
if let Some(attr) = attr {
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, sig);
let attr = find_attr!(cx.tcx.hir_attrs(item.hir_id()), AttributeKind::MustUse { span, reason } => (span, reason));
if let Some((attr_span, reason)) = attr {
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, *attr_span, *reason, attrs, sig);
} else if is_public && !is_proc_macro(attrs) && trait_ref_of_method(cx, item.owner_id).is_none() {
check_must_use_candidate(
cx,
Expand All @@ -75,9 +78,9 @@ pub(super) fn check_trait_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Tr
let fn_header_span = item.span.with_hi(sig.decl.output.span().hi());

let attrs = cx.tcx.hir_attrs(item.hir_id());
let attr = cx.tcx.get_attr(item.owner_id, sym::must_use);
if let Some(attr) = attr {
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, attr, attrs, sig);
let attr = find_attr!(cx.tcx.hir_attrs(item.hir_id()), AttributeKind::MustUse { span, reason } => (span, reason));
if let Some((attr_span, reason)) = attr {
check_needless_must_use(cx, sig.decl, item.owner_id, item.span, fn_header_span, *attr_span, *reason, attrs, sig);
} else if let hir::TraitFn::Provided(eid) = *eid {
let body = cx.tcx.hir_body(eid);
if attr.is_none() && is_public && !is_proc_macro(attrs) {
Expand All @@ -103,7 +106,8 @@ fn check_needless_must_use(
item_id: hir::OwnerId,
item_span: Span,
fn_header_span: Span,
attr: &Attribute,
attr_span: Span,
reason: Option<Symbol>,
attrs: &[Attribute],
sig: &FnSig<'_>,
) {
Expand All @@ -119,7 +123,7 @@ fn check_needless_must_use(
"this unit-returning function has a `#[must_use]` attribute",
|diag| {
diag.span_suggestion(
attr.span(),
attr_span,
"remove the attribute",
"",
Applicability::MachineApplicable,
Expand All @@ -137,11 +141,11 @@ fn check_needless_must_use(
MUST_USE_UNIT,
fn_header_span,
"this unit-returning function has a `#[must_use]` attribute",
Some(attr.span()),
Some(attr_span),
"remove `must_use`",
);
}
} else if attr.value_str().is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) {
} else if reason.is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) {
// Ignore async functions unless Future::Output type is a must_use type
if sig.header.is_async() {
let infcx = cx.tcx.infer_ctxt().build(cx.typing_mode());
Expand Down
9 changes: 7 additions & 2 deletions src/tools/clippy/clippy_lints/src/return_self_not_must_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use rustc_hir::intravisit::FnKind;
use rustc_hir::{Body, FnDecl, OwnerId, TraitItem, TraitItemKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::declare_lint_pass;
use rustc_span::{Span, sym};
use rustc_span::{Span};
use rustc_attr_data_structures::AttributeKind;
use rustc_attr_data_structures::find_attr;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -74,7 +76,10 @@ fn check_method(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_def: LocalDefId, spa
// We only show this warning for public exported methods.
&& cx.effective_visibilities.is_exported(fn_def)
// We don't want to emit this lint if the `#[must_use]` attribute is already there.
&& !cx.tcx.hir_attrs(owner_id.into()).iter().any(|attr| attr.has_name(sym::must_use))
&& !find_attr!(
cx.tcx.hir_attrs(owner_id.into()),
AttributeKind::MustUse { .. }
)
&& cx.tcx.visibility(fn_def.to_def_id()).is_public()
&& let ret_ty = return_ty(cx, owner_id)
&& let self_arg = nth_arg(cx, owner_id, 0)
Expand Down
5 changes: 4 additions & 1 deletion src/tools/clippy/clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1886,7 +1886,10 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
_ => None,
};

did.is_some_and(|did| cx.tcx.has_attr(did, sym::must_use))
did.is_some_and(|did| find_attr!(
cx.tcx.get_all_attrs(did),
AttributeKind::MustUse { ..}
))
}

/// Checks if a function's body represents the identity function. Looks for bodies of the form:
Expand Down
16 changes: 12 additions & 4 deletions src/tools/clippy/clippy_utils/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ use rustc_trait_selection::traits::{Obligation, ObligationCause};
use std::assert_matches::debug_assert_matches;
use std::collections::hash_map::Entry;
use std::iter;
use rustc_attr_data_structures::find_attr;
use rustc_attr_data_structures::AttributeKind;

use crate::path_res;
use crate::paths::{PathNS, lookup_path_str};
Expand Down Expand Up @@ -326,8 +328,14 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
// Returns whether the type has #[must_use] attribute
pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
match ty.kind() {
ty::Adt(adt, _) => cx.tcx.has_attr(adt.did(), sym::must_use),
ty::Foreign(did) => cx.tcx.has_attr(*did, sym::must_use),
ty::Adt(adt, _) => find_attr!(
cx.tcx.get_all_attrs(adt.did()),
AttributeKind::MustUse { ..}
),
ty::Foreign(did) => find_attr!(
cx.tcx.get_all_attrs(*did),
AttributeKind::MustUse { ..}
),
ty::Slice(ty) | ty::Array(ty, _) | ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => {
// for the Array case we don't need to care for the len == 0 case
// because we don't want to lint functions returning empty arrays
Expand All @@ -337,7 +345,7 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
ty::Alias(ty::Opaque, AliasTy { def_id, .. }) => {
for (predicate, _) in cx.tcx.explicit_item_self_bounds(def_id).skip_binder() {
if let ty::ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder()
&& cx.tcx.has_attr(trait_predicate.trait_ref.def_id, sym::must_use)
&& find_attr!(cx.tcx.get_all_attrs(trait_predicate.trait_ref.def_id), AttributeKind::MustUse { ..})
{
return true;
}
Expand All @@ -347,7 +355,7 @@ pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
ty::Dynamic(binder, _, _) => {
for predicate in *binder {
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder()
&& cx.tcx.has_attr(trait_ref.def_id, sym::must_use)
&& find_attr!(cx.tcx.get_all_attrs(trait_ref.def_id), AttributeKind::MustUse { ..})
{
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions tests/rustdoc-json/attrs/must_use.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#![no_std]

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

//@ is "$.index[?(@.name=='explicit_message')].attrs" '["#[must_use = \"does nothing if you do not use it\"]"]'
//@ is "$.index[?(@.name=='explicit_message')].attrs" '["#[attr = MustUse {reason: \"does nothing if you do not use it\"}]"]'
#[must_use = "does nothing if you do not use it"]
pub fn explicit_message() -> impl Iterator<Item = i64> {}
4 changes: 4 additions & 0 deletions tests/ui/attributes/malformed-must_use.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#[must_use()] //~ ERROR valid forms for the attribute are `#[must_use = "reason"]` and `#[must_use]`
struct Test;

fn main() {}
8 changes: 8 additions & 0 deletions tests/ui/attributes/malformed-must_use.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: valid forms for the attribute are `#[must_use = "reason"]` and `#[must_use]`
--> $DIR/malformed-must_use.rs:1:1
|
LL | #[must_use()]
| ^^^^^^^^^^^^^

error: aborting due to 1 previous error

Loading
Loading