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 3f50ecc

Browse files
clubby789fmease
clubby789
authored and
fmease
committedApr 24, 2024
Improve diagnostics for #[skip] 
1 parent e9c3a64 commit 3f50ecc

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
@@ -110,6 +110,12 @@ builtin_macros_derive_path_args_list = traits in `#[derive(...)]` don't accept a
110110
builtin_macros_derive_path_args_value = traits in `#[derive(...)]` don't accept values
111111
.suggestion = remove the value
112112
113+
builtin_macros_derive_skip_bad_argument = incorrect usage of the `#[skip]` attribute
114+
.note = the `#[skip]` attribute accepts an optional list of traits
115+
.help = try using `#[skip]` or `#[skip(Trait)]`
116+
117+
builtin_macros_derive_skip_unsupported = the `#[skip]` attribute does not support this trait
118+
113119
builtin_macros_env_not_defined = environment variable `{$var}` not defined at compile time
114120
.cargo = Cargo sets build script variables at run time. Use `std::env::var({$var_expr})` instead
115121
.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
@@ -1583,17 +1583,17 @@ impl<'a> TraitDef<'a> {
15831583
let mut skipped_derives = SkippedDerives::None;
15841584
let skip_enabled = cx.ecfg.features.derive_skip
15851585
|| struct_field.span.allows_unstable(sym::derive_skip);
1586-
for skip_attr in attr::filter_by_name(&struct_field.attrs, sym::skip) {
1586+
for attr in attr::filter_by_name(&struct_field.attrs, sym::skip) {
15871587
if !skip_enabled {
15881588
rustc_session::parse::feature_err(
15891589
&cx.sess,
15901590
sym::derive_skip,
1591-
skip_attr.span,
1591+
attr.span,
15921592
"the `#[skip]` attribute is experimental",
15931593
)
15941594
.emit();
15951595
}
1596-
let Some(skip_attr) = ast::Attribute::meta_kind(skip_attr) else {
1596+
let Some(skip_attr) = ast::Attribute::meta_kind(attr) else {
15971597
unreachable!()
15981598
};
15991599

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

‎compiler/rustc_builtin_macros/src/errors.rs

+9
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,15 @@ pub(crate) struct DerivePathArgsValue {
295295
pub(crate) span: Span,
296296
}
297297

298+
#[derive(Diagnostic)]
299+
#[diag(builtin_macros_derive_skip_bad_argument)]
300+
pub(crate) struct DeriveSkipBadArgument {
301+
#[note]
302+
#[help]
303+
#[primary_span]
304+
pub(crate) span: Span,
305+
}
306+
298307
#[derive(Diagnostic)]
299308
#[diag(builtin_macros_no_default_variant)]
300309
#[help]

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

+3
Original file line numberDiff line numberDiff line change
@@ -347,5 +347,8 @@ pub(super) fn builtin(sess: &Session, diagnostic: BuiltinLintDiag, diag: &mut Di
347347
"reduce the glob import's visibility or increase visibility of imported items",
348348
);
349349
}
350+
BuiltinLintDiagnostics::DeriveSkipUnsupported { traits } => {
351+
db.help(format!("the supported traits are {traits}"));
352+
}
350353
}
351354
}

‎compiler/rustc_lint_defs/src/builtin.rs

+29
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ declare_lint_pass! {
116116
UNSTABLE_NAME_COLLISIONS,
117117
UNSTABLE_SYNTAX_PRE_EXPANSION,
118118
UNSUPPORTED_CALLING_CONVENTIONS,
119+
UNSUPPORTED_DERIVE_SKIP,
119120
UNUSED_ASSIGNMENTS,
120121
UNUSED_ASSOCIATED_TYPE_BOUNDS,
121122
UNUSED_ATTRIBUTES,
@@ -4741,3 +4742,31 @@ declare_lint! {
47414742
};
47424743
crate_level_only
47434744
}
4745+
4746+
declare_lint! {
4747+
/// The `unsupported_derive_skip` lint detects the use of `#[skip]` specifying
4748+
/// an unsupported trait.
4749+
///
4750+
/// ### Example
4751+
///
4752+
/// ```rust,compile_fail
4753+
/// # #![allow(unused)]
4754+
/// #[derive(Clone)]
4755+
/// struct Unsupported {
4756+
/// #[skip(Clone)]
4757+
/// field: usize,
4758+
/// }
4759+
/// ```
4760+
///
4761+
/// {{produces}}
4762+
///
4763+
/// ### Explanation
4764+
///
4765+
/// The `#[skip]` attribute allows ignoring a field during the derivation
4766+
/// of specific traits. This is not supported for other traits, e.g. it wouldd
4767+
/// not be possible to clone a structure without cloning all fields..
4768+
pub UNSUPPORTED_DERIVE_SKIP,
4769+
Warn,
4770+
"an unsupported trait was passed to `#[skip]`",
4771+
@feature_gate = sym::derive_skip;
4772+
}

‎compiler/rustc_lint_defs/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,9 @@ pub enum BuiltinLintDiag {
664664
span: Span,
665665
max_vis: String,
666666
},
667+
DeriveSkipUnsupported {
668+
traits: String,
669+
},
667670
}
668671

669672
/// 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.