Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 048a438

Browse files
clubby789clubby789
committedFeb 16, 2024
Improve diagnostics for #[skip] 
1 parent 88ed493 commit 048a438

File tree

8 files changed

+137
-18
lines changed

8 files changed

+137
-18
lines changed
 

‎compiler/rustc_builtin_macros/messages.ftl

+6
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,12 @@ builtin_macros_derive_path_args_list = traits in `#[derive(...)]` don't accept a
108108
builtin_macros_derive_path_args_value = traits in `#[derive(...)]` don't accept values
109109
.suggestion = remove the value
110110
111+
builtin_macros_derive_skip_bad_argument = incorrect usage of the `#[skip]` attribute
112+
.note = the `#[skip]` attribute accepts an optional list of traits
113+
.help = try using `#[skip]` or `#[skip(Trait)]`
114+
115+
builtin_macros_derive_skip_unsupported = the `#[skip]` attribute does not support this trait
116+
111117
builtin_macros_env_not_defined = environment variable `{$var}` not defined at compile time
112118
.cargo = Cargo sets build script variables at run time. Use `std::env::var({$var_expr})` instead
113119
.custom = use `std::env::var({$var_expr})` to read the variable at run time

‎compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

+28-6
Original file line numberDiff line numberDiff line change
@@ -1582,17 +1582,17 @@ impl<'a> TraitDef<'a> {
15821582
let mut skipped_derives = SkippedDerives::None;
15831583
let skip_enabled = cx.ecfg.features.derive_skip
15841584
|| struct_field.span.allows_unstable(sym::derive_skip);
1585-
for skip_attr in attr::filter_by_name(&struct_field.attrs, sym::skip) {
1585+
for attr in attr::filter_by_name(&struct_field.attrs, sym::skip) {
15861586
if !skip_enabled {
15871587
rustc_session::parse::feature_err(
15881588
&cx.sess,
15891589
sym::derive_skip,
1590-
skip_attr.span,
1590+
attr.span,
15911591
"the `#[skip]` attribute is experimental",
15921592
)
15931593
.emit();
15941594
}
1595-
let Some(skip_attr) = ast::Attribute::meta_kind(skip_attr) else {
1595+
let Some(skip_attr) = ast::Attribute::meta_kind(attr) else {
15961596
unreachable!()
15971597
};
15981598

@@ -1604,20 +1604,42 @@ impl<'a> TraitDef<'a> {
16041604
}
16051605
ast::MetaItemKind::List(items) => {
16061606
for item in items {
1607+
let span = item.span();
16071608
let ast::NestedMetaItem::MetaItem(ast::MetaItem {
16081609
path,
16091610
kind: ast::MetaItemKind::Word,
16101611
..
16111612
}) = item
16121613
else {
1613-
cx.dcx().span_err(item.span(), "incorrect skip argument");
1614+
cx.dcx().emit_err(errors::DeriveSkipBadArgument {
1615+
span,
1616+
});
16141617
continue;
16151618
};
1616-
skipped_derives.add(path.segments[0].ident.name);
1619+
let name = path.segments[0].ident;
1620+
const SUPPORTED_TRAITS: [Symbol; 5] = [
1621+
sym::PartialEq,
1622+
sym::PartialOrd,
1623+
sym::Ord,
1624+
sym::Hash,
1625+
sym::Debug,
1626+
];
1627+
if SUPPORTED_TRAITS.contains(&name.name) {
1628+
skipped_derives.add(path.segments[0].ident.name)
1629+
} else {
1630+
let traits = SUPPORTED_TRAITS.iter().map(|s| format!("`{s}`")).collect::<Vec<_>>().join(", ");
1631+
cx.parse_sess().buffer_lint_with_diagnostic(
1632+
rustc_session::lint::builtin::UNSUPPORTED_DERIVE_SKIP,
1633+
span,
1634+
cx.current_expansion.lint_node_id,
1635+
crate::fluent_generated::builtin_macros_derive_skip_unsupported,
1636+
rustc_session::lint::BuiltinLintDiagnostics::DeriveSkipUnsupported { traits },
1637+
)
1638+
}
16171639
}
16181640
}
16191641
ast::MetaItemKind::NameValue(lit) => {
1620-
cx.dcx().span_err(lit.span, "invalid skip attribute");
1642+
cx.dcx().emit_err(errors::DeriveSkipBadArgument { span: lit.span });
16211643
}
16221644
}
16231645
}

‎compiler/rustc_builtin_macros/src/errors.rs

+9
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,15 @@ pub(crate) struct DerivePathArgsValue {
316316
pub(crate) span: Span,
317317
}
318318

319+
#[derive(Diagnostic)]
320+
#[diag(builtin_macros_derive_skip_bad_argument)]
321+
pub(crate) struct DeriveSkipBadArgument {
322+
#[note]
323+
#[help]
324+
#[primary_span]
325+
pub(crate) span: Span,
326+
}
327+
319328
#[derive(Diagnostic)]
320329
#[diag(builtin_macros_no_default_variant)]
321330
#[help]

‎compiler/rustc_lint/src/context/diagnostics.rs

+3
Original file line numberDiff line numberDiff line change
@@ -583,5 +583,8 @@ pub(super) fn builtin(
583583
db.span_note(span, format!("the most public imported item is `{max_vis}`"));
584584
db.help("reduce the glob import's visibility or increase visibility of imported items");
585585
}
586+
BuiltinLintDiagnostics::DeriveSkipUnsupported { traits } => {
587+
db.help(format!("the supported traits are {traits}"));
588+
}
586589
}
587590
}

‎compiler/rustc_lint_defs/src/builtin.rs

+29
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ declare_lint_pass! {
113113
UNSTABLE_NAME_COLLISIONS,
114114
UNSTABLE_SYNTAX_PRE_EXPANSION,
115115
UNSUPPORTED_CALLING_CONVENTIONS,
116+
UNSUPPORTED_DERIVE_SKIP,
116117
UNUSED_ASSIGNMENTS,
117118
UNUSED_ASSOCIATED_TYPE_BOUNDS,
118119
UNUSED_ATTRIBUTES,
@@ -4569,3 +4570,31 @@ declare_lint! {
45694570
reference: "issue #120192 <https://github.com/rust-lang/rust/issues/120192>",
45704571
};
45714572
}
4573+
4574+
declare_lint! {
4575+
/// The `unsupported_derive_skip` lint detects the use of `#[skip]` specifying
4576+
/// an unsupported trait.
4577+
///
4578+
/// ### Example
4579+
///
4580+
/// ```rust,compile_fail
4581+
/// # #![allow(unused)]
4582+
/// #[derive(Clone)]
4583+
/// struct Unsupported {
4584+
/// #[skip(Clone)]
4585+
/// field: usize,
4586+
/// }
4587+
/// ```
4588+
///
4589+
/// {{produces}}
4590+
///
4591+
/// ### Explanation
4592+
///
4593+
/// The `#[skip]` attribute allows ignoring a field during the derivation
4594+
/// of specific traits. This is not supported for other traits, e.g. it wouldd
4595+
/// not be possible to clone a structure without cloning all fields..
4596+
pub UNSUPPORTED_DERIVE_SKIP,
4597+
Warn,
4598+
"an unsupported trait was passed to `#[skip]`",
4599+
@feature_gate = sym::derive_skip;
4600+
}

‎compiler/rustc_lint_defs/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,9 @@ pub enum BuiltinLintDiagnostics {
660660
span: Span,
661661
max_vis: String,
662662
},
663+
DeriveSkipUnsupported {
664+
traits: String,
665+
},
663666
}
664667

665668
/// Lints that are buffered up early on in the `Session` before the

‎tests/ui/deriving/skip/derive-skip-invalid.rs

+17-4
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,24 @@
22
#![feature(derive_skip)]
33

44
#[derive(Debug)]
5-
struct KeyVal(#[skip = "Debug"] usize); //~ ERROR invalid skip attribute
5+
struct KeyVal(#[skip = "Debug"] usize);
6+
//~^ ERROR incorrect usage of the `#[skip]` attribute
7+
//~| NOTE the `#[skip]` attribute accepts an optional list of traits
68

79
#[derive(Debug)]
8-
struct BadArg(#[skip("Debug")] usize); //~ ERROR incorrect skip argument
10+
struct BadArg(#[skip("Debug")] usize);
11+
//~^ ERROR incorrect usage of the `#[skip]` attribute
12+
//~| NOTE the `#[skip]` attribute accepts an optional list of traits
913

1014
// FIXME: better error for derives not supporting `skip`
11-
#[derive(Clone)]
12-
struct SkipClone(#[skip] usize); //~ ERROR cannot find attribute `skip` in this scope
15+
// #[derive(Clone)]
16+
// struct SkipClone(#[skip] usize);
17+
18+
// FIXME: derives don't get a useful lint_node_id so the lint is at the crate level
19+
#[derive(Debug, Clone)]
20+
#[deny(unsupported_derive_skip)]
21+
struct SkipClone2(#[skip(Clone)] usize);
22+
//~^ WARN the `#[skip]` attribute does not support this trait
23+
//~| WARN the `#[skip]` attribute does not support this trait
24+
//~| NOTE #[warn(unsupported_derive_skip)]` on by default
25+
//~| NOTE duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

‎tests/ui/deriving/skip/derive-skip-invalid.stderr

+42-8
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,54 @@
1-
error: invalid skip attribute
1+
error: incorrect usage of the `#[skip]` attribute
2+
--> $DIR/derive-skip-invalid.rs:5:24
3+
|
4+
LL | struct KeyVal(#[skip = "Debug"] usize);
5+
| ^^^^^^^
6+
|
7+
note: the `#[skip]` attribute accepts an optional list of traits
8+
--> $DIR/derive-skip-invalid.rs:5:24
9+
|
10+
LL | struct KeyVal(#[skip = "Debug"] usize);
11+
| ^^^^^^^
12+
help: try using `#[skip]` or `#[skip(Trait)]`
213
--> $DIR/derive-skip-invalid.rs:5:24
314
|
415
LL | struct KeyVal(#[skip = "Debug"] usize);
516
| ^^^^^^^
617

7-
error: incorrect skip argument
8-
--> $DIR/derive-skip-invalid.rs:8:22
18+
error: incorrect usage of the `#[skip]` attribute
19+
--> $DIR/derive-skip-invalid.rs:10:22
20+
|
21+
LL | struct BadArg(#[skip("Debug")] usize);
22+
| ^^^^^^^
23+
|
24+
note: the `#[skip]` attribute accepts an optional list of traits
25+
--> $DIR/derive-skip-invalid.rs:10:22
26+
|
27+
LL | struct BadArg(#[skip("Debug")] usize);
28+
| ^^^^^^^
29+
help: try using `#[skip]` or `#[skip(Trait)]`
30+
--> $DIR/derive-skip-invalid.rs:10:22
931
|
1032
LL | struct BadArg(#[skip("Debug")] usize);
1133
| ^^^^^^^
1234

13-
error: cannot find attribute `skip` in this scope
14-
--> $DIR/derive-skip-invalid.rs:12:20
35+
warning: the `#[skip]` attribute does not support this trait
36+
--> $DIR/derive-skip-invalid.rs:21:26
37+
|
38+
LL | struct SkipClone2(#[skip(Clone)] usize);
39+
| ^^^^^
40+
|
41+
= help: the supported traits are `PartialEq`, `PartialOrd`, `Ord`, `Hash`, `Debug`
42+
= note: `#[warn(unsupported_derive_skip)]` on by default
43+
44+
warning: the `#[skip]` attribute does not support this trait
45+
--> $DIR/derive-skip-invalid.rs:21:26
46+
|
47+
LL | struct SkipClone2(#[skip(Clone)] usize);
48+
| ^^^^^
1549
|
16-
LL | struct SkipClone(#[skip] usize);
17-
| ^^^^
50+
= help: the supported traits are `PartialEq`, `PartialOrd`, `Ord`, `Hash`, `Debug`
51+
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
1852

19-
error: aborting due to 3 previous errors
53+
error: aborting due to 2 previous errors; 2 warnings emitted
2054

0 commit comments

Comments
 (0)
Failed to load comments.