Skip to content

Commit 758866d

Browse files
authored
Rollup merge of #145500 - JonathanBrouwer:must_use_target, r=jdonszelmann
Port must_use to the new target checking This PR ports `must_use` to the new target checking logic This also adds a tool-only suggestion to remove attributes on invalid targets, as to not immediately undo the work of #145274 r? `@jdonszelmann`
2 parents 3a3c4db + d5dc797 commit 758866d

15 files changed

+535
-365
lines changed

compiler/rustc_attr_parsing/messages.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ attr_parsing_empty_attribute =
1212
1313
attr_parsing_invalid_target = `#[{$name}]` attribute cannot be used on {$target}
1414
.help = `#[{$name}]` can {$only}be applied to {$applied}
15+
.suggestion = remove the attribute
1516
attr_parsing_invalid_target_lint = `#[{$name}]` attribute cannot be used on {$target}
1617
.warn = {-attr_parsing_previously_accepted}
1718
.help = `#[{$name}]` can {$only}be applied to {$applied}
19+
.suggestion = remove the attribute
1820
1921
attr_parsing_empty_confusables =
2022
expected at least one confusable name

compiler/rustc_attr_parsing/src/attributes/must_use.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
use rustc_errors::DiagArgValue;
22
use rustc_feature::{AttributeTemplate, template};
33
use rustc_hir::attrs::AttributeKind;
4+
use rustc_hir::{MethodKind, Target};
45
use rustc_span::{Symbol, sym};
56

67
use crate::attributes::{AttributeOrder, OnDuplicate, SingleAttributeParser};
7-
use crate::context::{ALL_TARGETS, AcceptContext, AllowedTargets, Stage};
8+
use crate::context::MaybeWarn::{Allow, Error};
9+
use crate::context::{AcceptContext, AllowedTargets, Stage};
810
use crate::parser::ArgParser;
911
use crate::session_diagnostics;
1012
pub(crate) struct MustUseParser;
@@ -13,7 +15,21 @@ impl<S: Stage> SingleAttributeParser<S> for MustUseParser {
1315
const PATH: &[Symbol] = &[sym::must_use];
1416
const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepOutermost;
1517
const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::WarnButFutureError;
16-
const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(ALL_TARGETS); //FIXME Still checked fully in `check_attr.rs`
18+
const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowListWarnRest(&[
19+
Allow(Target::Fn),
20+
Allow(Target::Enum),
21+
Allow(Target::Struct),
22+
Allow(Target::Union),
23+
Allow(Target::Method(MethodKind::Trait { body: false })),
24+
Allow(Target::Method(MethodKind::Trait { body: true })),
25+
Allow(Target::Method(MethodKind::Inherent)),
26+
Allow(Target::ForeignFn),
27+
// `impl Trait` in return position can trip
28+
// `unused_must_use` if `Trait` is marked as
29+
// `#[must_use]`
30+
Allow(Target::Trait),
31+
Error(Target::WherePredicate),
32+
]);
1733
const TEMPLATE: AttributeTemplate = template!(
1834
Word, NameValueStr: "reason",
1935
"https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute"

compiler/rustc_attr_parsing/src/lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ pub fn emit_attribute_lint<L: LintEmitter>(lint: &AttributeLint<HirId>, lint_emi
5353
target: target.plural_name(),
5454
applied: applied.clone(),
5555
only,
56+
attr_span: *span,
5657
},
5758
),
5859
}

compiler/rustc_attr_parsing/src/session_diagnostics.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,13 +489,16 @@ pub(crate) struct InvalidTargetLint {
489489
pub target: &'static str,
490490
pub applied: String,
491491
pub only: &'static str,
492+
#[suggestion(code = "", applicability = "machine-applicable", style = "tool-only")]
493+
pub attr_span: Span,
492494
}
493495

494496
#[derive(Diagnostic)]
495497
#[help]
496498
#[diag(attr_parsing_invalid_target)]
497499
pub(crate) struct InvalidTarget {
498500
#[primary_span]
501+
#[suggestion(code = "", applicability = "machine-applicable", style = "tool-only")]
499502
pub span: Span,
500503
pub name: Symbol,
501504
pub target: &'static str,

compiler/rustc_passes/messages.ftl

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,10 +432,6 @@ passes_must_not_suspend =
432432
`must_not_suspend` attribute should be applied to a struct, enum, union, or trait
433433
.label = is not a struct, enum, union, or trait
434434
435-
passes_must_use_no_effect =
436-
`#[must_use]` has no effect when applied to {$target}
437-
.suggestion = remove the attribute
438-
439435
passes_no_link =
440436
attribute should be applied to an `extern crate` item
441437
.label = not an `extern crate` item

compiler/rustc_passes/src/check_attr.rs

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
194194
Attribute::Parsed(AttributeKind::MayDangle(attr_span)) => {
195195
self.check_may_dangle(hir_id, *attr_span)
196196
}
197-
Attribute::Parsed(AttributeKind::MustUse { span, .. }) => {
198-
self.check_must_use(hir_id, *span, target)
199-
}
200197
&Attribute::Parsed(AttributeKind::CustomMir(dialect, phase, attr_span)) => {
201198
self.check_custom_mir(dialect, phase, attr_span)
202199
}
@@ -249,7 +246,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
249246
| AttributeKind::Coverage (..)
250247
| AttributeKind::ShouldPanic { .. }
251248
| AttributeKind::Coroutine(..)
252-
| AttributeKind::Linkage(..),
249+
| AttributeKind::Linkage(..)
250+
| AttributeKind::MustUse { .. },
253251
) => { /* do nothing */ }
254252
Attribute::Unparsed(attr_item) => {
255253
style = Some(attr_item.style);
@@ -1263,41 +1261,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
12631261
}
12641262
}
12651263

1266-
/// Warns against some misuses of `#[must_use]`
1267-
fn check_must_use(&self, hir_id: HirId, attr_span: Span, target: Target) {
1268-
if matches!(
1269-
target,
1270-
Target::Fn
1271-
| Target::Enum
1272-
| Target::Struct
1273-
| Target::Union
1274-
| Target::Method(MethodKind::Trait { body: false } | MethodKind::Inherent)
1275-
| Target::ForeignFn
1276-
// `impl Trait` in return position can trip
1277-
// `unused_must_use` if `Trait` is marked as
1278-
// `#[must_use]`
1279-
| Target::Trait
1280-
) {
1281-
return;
1282-
}
1283-
1284-
// `#[must_use]` can be applied to a trait method definition with a default body
1285-
if let Target::Method(MethodKind::Trait { body: true }) = target
1286-
&& let parent_def_id = self.tcx.hir_get_parent_item(hir_id).def_id
1287-
&& let containing_item = self.tcx.hir_expect_item(parent_def_id)
1288-
&& let hir::ItemKind::Trait(..) = containing_item.kind
1289-
{
1290-
return;
1291-
}
1292-
1293-
self.tcx.emit_node_span_lint(
1294-
UNUSED_ATTRIBUTES,
1295-
hir_id,
1296-
attr_span,
1297-
errors::MustUseNoEffect { target: target.plural_name(), attr_span },
1298-
);
1299-
}
1300-
13011264
/// Checks if `#[must_not_suspend]` is applied to a struct, enum, union, or trait.
13021265
fn check_must_not_suspend(&self, attr: &Attribute, span: Span, target: Target) {
13031266
match target {

compiler/rustc_passes/src/errors.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -358,14 +358,6 @@ pub(crate) struct BothFfiConstAndPure {
358358
pub attr_span: Span,
359359
}
360360

361-
#[derive(LintDiagnostic)]
362-
#[diag(passes_must_use_no_effect)]
363-
pub(crate) struct MustUseNoEffect {
364-
pub target: &'static str,
365-
#[suggestion(code = "", applicability = "machine-applicable", style = "tool-only")]
366-
pub attr_span: Span,
367-
}
368-
369361
#[derive(Diagnostic)]
370362
#[diag(passes_must_not_suspend)]
371363
pub(crate) struct MustNotSuspend {

tests/ui/attributes/rustc_confusables.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,5 @@ impl Bar {
4545
#[rustc_confusables("blah")]
4646
//~^ ERROR attribute cannot be used on
4747
//~| HELP can only be applied to
48+
//~| HELP remove the attribute
4849
fn not_inherent_impl_method() {}

tests/ui/extern/issue-47725.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
//~^ WARN attribute cannot be used on
55
//~| WARN previously accepted
66
//~| HELP can be applied to
7+
//~| HELP remove the attribute
78
struct Foo;
89

910
#[link_name = "foobar"]
1011
//~^ WARN attribute cannot be used on
1112
//~| WARN previously accepted
1213
//~| HELP can be applied to
14+
//~| HELP remove the attribute
1315
extern "C" {
1416
fn foo() -> u32;
1517
}
@@ -19,6 +21,7 @@ extern "C" {
1921
//~| HELP must be of the form
2022
//~| WARN attribute cannot be used on
2123
//~| WARN previously accepted
24+
//~| HELP remove the attribute
2225
//~| HELP can be applied to
2326
//~| NOTE for more information, visit
2427
extern "C" {

tests/ui/extern/issue-47725.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0539]: malformed `link_name` attribute input
2-
--> $DIR/issue-47725.rs:17:1
2+
--> $DIR/issue-47725.rs:19:1
33
|
44
LL | #[link_name]
55
| ^^^^^^^^^^^^ help: must be of the form: `#[link_name = "name"]`
@@ -21,7 +21,7 @@ LL | #![warn(unused_attributes)]
2121
| ^^^^^^^^^^^^^^^^^
2222

2323
warning: `#[link_name]` attribute cannot be used on foreign modules
24-
--> $DIR/issue-47725.rs:9:1
24+
--> $DIR/issue-47725.rs:10:1
2525
|
2626
LL | #[link_name = "foobar"]
2727
| ^^^^^^^^^^^^^^^^^^^^^^^
@@ -30,7 +30,7 @@ LL | #[link_name = "foobar"]
3030
= help: `#[link_name]` can be applied to foreign functions, foreign statics
3131

3232
warning: `#[link_name]` attribute cannot be used on foreign modules
33-
--> $DIR/issue-47725.rs:17:1
33+
--> $DIR/issue-47725.rs:19:1
3434
|
3535
LL | #[link_name]
3636
| ^^^^^^^^^^^^

0 commit comments

Comments
 (0)