From c70147fd66e08962ab06adf12eb6a41bc1ea7f08 Mon Sep 17 00:00:00 2001 From: Jacob Pratt Date: Tue, 29 Jun 2021 20:22:52 -0400 Subject: [PATCH 1/4] Permit deriving default on enums with `#[default]` --- .../src/deriving/default.rs | 197 +++++++++++++++--- compiler/rustc_feature/src/active.rs | 3 + compiler/rustc_span/src/symbol.rs | 1 + library/core/src/default.rs | 3 +- src/test/ui/deriving/deriving-default-enum.rs | 19 ++ src/test/ui/deriving/deriving-with-helper.rs | 5 +- src/test/ui/error-codes/E0665.rs | 8 - src/test/ui/error-codes/E0665.stderr | 11 - .../feature-gate-derive_default_enum.rs | 7 + .../feature-gate-derive_default_enum.stderr | 13 ++ src/test/ui/macros/macros-nonfatal-errors.rs | 57 ++++- .../ui/macros/macros-nonfatal-errors.stderr | 119 +++++++++-- src/test/ui/resolve/issue-2356.rs | 2 +- 13 files changed, 371 insertions(+), 74 deletions(-) create mode 100644 src/test/ui/deriving/deriving-default-enum.rs delete mode 100644 src/test/ui/error-codes/E0665.rs delete mode 100644 src/test/ui/error-codes/E0665.stderr create mode 100644 src/test/ui/feature-gates/feature-gate-derive_default_enum.rs create mode 100644 src/test/ui/feature-gates/feature-gate-derive_default_enum.stderr diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index 980be3a005039..7c1dc5e536555 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/compiler/rustc_builtin_macros/src/deriving/default.rs @@ -2,11 +2,15 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use rustc_ast::ptr::P; +use rustc_ast::EnumDef; +use rustc_ast::VariantData; use rustc_ast::{Expr, MetaItem}; -use rustc_errors::struct_span_err; +use rustc_errors::Applicability; use rustc_expand::base::{Annotatable, DummyResult, ExtCtxt}; +use rustc_span::symbol::Ident; use rustc_span::symbol::{kw, sym}; use rustc_span::Span; +use smallvec::SmallVec; pub fn expand_deriving_default( cx: &mut ExtCtxt<'_>, @@ -34,8 +38,25 @@ pub fn expand_deriving_default( attributes: attrs, is_unsafe: false, unify_fieldless_variants: false, - combine_substructure: combine_substructure(Box::new(|a, b, c| { - default_substructure(a, b, c) + combine_substructure: combine_substructure(Box::new(|cx, trait_span, substr| { + match substr.fields { + StaticStruct(_, fields) => { + default_struct_substructure(cx, trait_span, substr, fields) + } + StaticEnum(enum_def, _) => { + if !cx.sess.features_untracked().derive_default_enum { + rustc_session::parse::feature_err( + cx.parse_sess(), + sym::derive_default_enum, + span, + "deriving `Default` on enums is experimental", + ) + .emit(); + } + default_enum_substructure(cx, trait_span, enum_def) + } + _ => cx.span_bug(trait_span, "method in `derive(Default)`"), + } })), }], associated_types: Vec::new(), @@ -43,44 +64,158 @@ pub fn expand_deriving_default( trait_def.expand(cx, mitem, item, push) } -fn default_substructure( +fn default_struct_substructure( cx: &mut ExtCtxt<'_>, trait_span: Span, substr: &Substructure<'_>, + summary: &StaticFields, ) -> P { // Note that `kw::Default` is "default" and `sym::Default` is "Default"! let default_ident = cx.std_path(&[kw::Default, sym::Default, kw::Default]); let default_call = |span| cx.expr_call_global(span, default_ident.clone(), Vec::new()); - match *substr.fields { - StaticStruct(_, ref summary) => match *summary { - Unnamed(ref fields, is_tuple) => { - if !is_tuple { - cx.expr_ident(trait_span, substr.type_ident) - } else { - let exprs = fields.iter().map(|sp| default_call(*sp)).collect(); - cx.expr_call_ident(trait_span, substr.type_ident, exprs) - } - } - Named(ref fields) => { - let default_fields = fields - .iter() - .map(|&(ident, span)| cx.field_imm(span, ident, default_call(span))) - .collect(); - cx.expr_struct_ident(trait_span, substr.type_ident, default_fields) + match summary { + Unnamed(ref fields, is_tuple) => { + if !is_tuple { + cx.expr_ident(trait_span, substr.type_ident) + } else { + let exprs = fields.iter().map(|sp| default_call(*sp)).collect(); + cx.expr_call_ident(trait_span, substr.type_ident, exprs) } - }, - StaticEnum(..) => { - struct_span_err!( - &cx.sess.parse_sess.span_diagnostic, - trait_span, - E0665, - "`Default` cannot be derived for enums, only structs" - ) + } + Named(ref fields) => { + let default_fields = fields + .iter() + .map(|&(ident, span)| cx.field_imm(span, ident, default_call(span))) + .collect(); + cx.expr_struct_ident(trait_span, substr.type_ident, default_fields) + } + } +} + +fn default_enum_substructure( + cx: &mut ExtCtxt<'_>, + trait_span: Span, + enum_def: &EnumDef, +) -> P { + let default_variant = match extract_default_variant(cx, enum_def, trait_span) { + Ok(value) => value, + Err(()) => return DummyResult::raw_expr(trait_span, true), + }; + + // At this point, we know that there is exactly one variant with a `#[default]` attribute. The + // attribute hasn't yet been validated. + + if let Err(()) = validate_default_attribute(cx, default_variant) { + return DummyResult::raw_expr(trait_span, true); + } + + // We now know there is exactly one unit variant with exactly one `#[default]` attribute. + + cx.expr_path(cx.path( + default_variant.span, + vec![Ident::new(kw::SelfUpper, default_variant.span), default_variant.ident], + )) +} + +fn extract_default_variant<'a>( + cx: &mut ExtCtxt<'_>, + enum_def: &'a EnumDef, + trait_span: Span, +) -> Result<&'a rustc_ast::Variant, ()> { + let default_variants: SmallVec<[_; 1]> = enum_def + .variants + .iter() + .filter(|variant| cx.sess.contains_name(&variant.attrs, kw::Default)) + .collect(); + + let variant = match default_variants.as_slice() { + [variant] => variant, + [] => { + cx.struct_span_err(trait_span, "no default declared") + .help("make a unit variant default by placing `#[default]` above it") + .emit(); + + return Err(()); + } + [first, rest @ ..] => { + cx.struct_span_err(trait_span, "multiple declared defaults") + .span_label(first.span, "first default") + .span_labels(rest.iter().map(|variant| variant.span), "additional default") + .note("only one variant can be default") + .emit(); + + return Err(()); + } + }; + + if !matches!(variant.data, VariantData::Unit(..)) { + cx.struct_span_err(variant.ident.span, "`#[default]` may only be used on unit variants") + .help("consider a manual implementation of `Default`") + .emit(); + + return Err(()); + } + + if let Some(non_exhaustive_attr) = cx.sess.find_by_name(&variant.attrs, sym::non_exhaustive) { + cx.struct_span_err(variant.ident.span, "default variant must be exhaustive") + .span_label(non_exhaustive_attr.span, "declared `#[non_exhaustive]` here") + .help("consider a manual implementation of `Default`") .emit(); - // let compilation continue - DummyResult::raw_expr(trait_span, true) + + return Err(()); + } + + Ok(variant) +} + +fn validate_default_attribute( + cx: &mut ExtCtxt<'_>, + default_variant: &rustc_ast::Variant, +) -> Result<(), ()> { + let attrs: SmallVec<[_; 1]> = + cx.sess.filter_by_name(&default_variant.attrs, kw::Default).collect(); + + let attr = match attrs.as_slice() { + [attr] => attr, + [] => cx.bug( + "this method must only be called with a variant that has a `#[default]` attribute", + ), + [first, rest @ ..] => { + // FIXME(jhpratt) Do we want to perform this check? It doesn't exist + // for `#[inline]`, `#[non_exhaustive]`, and presumably others. + + let suggestion_text = + if rest.len() == 1 { "try removing this" } else { "try removing these" }; + + cx.struct_span_err(default_variant.ident.span, "multiple `#[default]` attributes") + .note("only one `#[default]` attribute is needed") + .span_label(first.span, "`#[default]` used here") + .span_label(rest[0].span, "`#[default]` used again here") + .span_help(rest.iter().map(|attr| attr.span).collect::>(), suggestion_text) + // This would otherwise display the empty replacement, hence the otherwise + // repetitive `.span_help` call above. + .tool_only_multipart_suggestion( + suggestion_text, + rest.iter().map(|attr| (attr.span, String::new())).collect(), + Applicability::MachineApplicable, + ) + .emit(); + + return Err(()); } - _ => cx.span_bug(trait_span, "method in `derive(Default)`"), + }; + if !attr.is_word() { + cx.struct_span_err(attr.span, "`#[default]` attribute does not accept a value") + .span_suggestion_hidden( + attr.span, + "try using `#[default]`", + "#[default]".into(), + Applicability::MaybeIncorrect, + ) + .emit(); + + return Err(()); } + Ok(()) } diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 803e4a2e59def..41351c1f9389b 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -683,6 +683,9 @@ declare_features! ( /// Infer generic args for both consts and types. (active, generic_arg_infer, "1.55.0", Some(85077), None), + /// Allows `#[derive(Default)]` and `#[default]` on enums. + (active, derive_default_enum, "1.56.0", Some(86985), None), + // ------------------------------------------------------------------------- // feature-group-end: actual feature gates // ------------------------------------------------------------------------- diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 536ebdef426b8..5fc773e431c8b 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -489,6 +489,7 @@ symbols! { deref_mut, deref_target, derive, + derive_default_enum, destructuring_assignment, diagnostic, direct, diff --git a/library/core/src/default.rs b/library/core/src/default.rs index fd7159d35fa7f..0fe88222805e5 100644 --- a/library/core/src/default.rs +++ b/library/core/src/default.rs @@ -161,7 +161,8 @@ pub fn default() -> T { } /// Derive macro generating an impl of the trait `Default`. -#[rustc_builtin_macro] +#[cfg_attr(not(bootstrap), rustc_builtin_macro(Default, attributes(default)))] +#[cfg_attr(bootstrap, rustc_builtin_macro)] #[stable(feature = "builtin_macro_prelude", since = "1.38.0")] #[allow_internal_unstable(core_intrinsics)] pub macro Default($item:item) { diff --git a/src/test/ui/deriving/deriving-default-enum.rs b/src/test/ui/deriving/deriving-default-enum.rs new file mode 100644 index 0000000000000..931ff1a5847d4 --- /dev/null +++ b/src/test/ui/deriving/deriving-default-enum.rs @@ -0,0 +1,19 @@ +// run-pass + +#![feature(derive_default_enum)] + +// nb: does not impl Default +#[derive(Debug, PartialEq)] +struct NotDefault; + +#[derive(Debug, Default, PartialEq)] +enum Foo { + #[default] + Alpha, + #[allow(dead_code)] + Beta(NotDefault), +} + +fn main() { + assert_eq!(Foo::default(), Foo::Alpha); +} diff --git a/src/test/ui/deriving/deriving-with-helper.rs b/src/test/ui/deriving/deriving-with-helper.rs index ea74a15624c60..d8f0b27a2e5f6 100644 --- a/src/test/ui/deriving/deriving-with-helper.rs +++ b/src/test/ui/deriving/deriving-with-helper.rs @@ -5,6 +5,7 @@ #![feature(lang_items)] #![feature(no_core)] #![feature(rustc_attrs)] +#![feature(derive_default_enum)] #![no_core] @@ -30,7 +31,7 @@ mod default { trait Sized {} #[derive(Default)] -struct S { +enum S { #[default] // OK - field: u8, + Foo, } diff --git a/src/test/ui/error-codes/E0665.rs b/src/test/ui/error-codes/E0665.rs deleted file mode 100644 index cfd42bd9aac32..0000000000000 --- a/src/test/ui/error-codes/E0665.rs +++ /dev/null @@ -1,8 +0,0 @@ -#[derive(Default)] //~ ERROR E0665 -enum Food { - Sweet, - Salty, -} - -fn main() { -} diff --git a/src/test/ui/error-codes/E0665.stderr b/src/test/ui/error-codes/E0665.stderr deleted file mode 100644 index bb8b3906ca6b0..0000000000000 --- a/src/test/ui/error-codes/E0665.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0665]: `Default` cannot be derived for enums, only structs - --> $DIR/E0665.rs:1:10 - | -LL | #[derive(Default)] - | ^^^^^^^ - | - = note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0665`. diff --git a/src/test/ui/feature-gates/feature-gate-derive_default_enum.rs b/src/test/ui/feature-gates/feature-gate-derive_default_enum.rs new file mode 100644 index 0000000000000..05a5d4e14223a --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-derive_default_enum.rs @@ -0,0 +1,7 @@ +#[derive(Default)] //~ ERROR deriving `Default` on enums is experimental +enum Foo { + #[default] + Alpha, +} + +fn main() {} diff --git a/src/test/ui/feature-gates/feature-gate-derive_default_enum.stderr b/src/test/ui/feature-gates/feature-gate-derive_default_enum.stderr new file mode 100644 index 0000000000000..58dd4d508a709 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-derive_default_enum.stderr @@ -0,0 +1,13 @@ +error[E0658]: deriving `Default` on enums is experimental + --> $DIR/feature-gate-derive_default_enum.rs:1:10 + | +LL | #[derive(Default)] + | ^^^^^^^ + | + = note: see issue #86985 for more information + = help: add `#![feature(derive_default_enum)]` to the crate attributes to enable + = note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/macros/macros-nonfatal-errors.rs b/src/test/ui/macros/macros-nonfatal-errors.rs index 0a496c9dc3d33..bb5f4d089bc1f 100644 --- a/src/test/ui/macros/macros-nonfatal-errors.rs +++ b/src/test/ui/macros/macros-nonfatal-errors.rs @@ -5,9 +5,62 @@ #![feature(asm, llvm_asm)] #![feature(trace_macros, concat_idents)] +#![feature(derive_default_enum)] -#[derive(Default)] //~ ERROR -enum OrDeriveThis {} +#[derive(Default)] //~ ERROR no default declared +enum NoDeclaredDefault { + Foo, + Bar, +} + +#[derive(Default)] //~ ERROR multiple declared defaults +enum MultipleDefaults { + #[default] + Foo, + #[default] + Bar, + #[default] + Baz, +} + +#[derive(Default)] +enum ExtraDeriveTokens { + #[default = 1] //~ ERROR `#[default]` attribute does not accept a value + Foo, +} + +#[derive(Default)] +enum TwoDefaultAttrs { + #[default] + #[default] + Foo, //~ERROR multiple `#[default]` attributes + Bar, +} + +#[derive(Default)] +enum ManyDefaultAttrs { + #[default] + #[default] + #[default] + #[default] + Foo, //~ERROR multiple `#[default]` attributes + Bar, +} + +#[derive(Default)] +enum DefaultHasFields { + #[default] + Foo {}, //~ ERROR `#[default]` may only be used on unit variants + Bar, +} + +#[derive(Default)] +enum NonExhaustiveDefault { + #[default] + #[non_exhaustive] + Foo, //~ ERROR default variant must be exhaustive + Bar, +} fn main() { asm!(invalid); //~ ERROR diff --git a/src/test/ui/macros/macros-nonfatal-errors.stderr b/src/test/ui/macros/macros-nonfatal-errors.stderr index 14058f8663933..4bb5e9b816950 100644 --- a/src/test/ui/macros/macros-nonfatal-errors.stderr +++ b/src/test/ui/macros/macros-nonfatal-errors.stderr @@ -1,49 +1,133 @@ -error[E0665]: `Default` cannot be derived for enums, only structs - --> $DIR/macros-nonfatal-errors.rs:9:10 +error: no default declared + --> $DIR/macros-nonfatal-errors.rs:10:10 | LL | #[derive(Default)] | ^^^^^^^ | + = help: make a unit variant default by placing `#[default]` above it = note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) +error: multiple declared defaults + --> $DIR/macros-nonfatal-errors.rs:16:10 + | +LL | #[derive(Default)] + | ^^^^^^^ +... +LL | Foo, + | --- first default +LL | #[default] +LL | Bar, + | --- additional default +LL | #[default] +LL | Baz, + | --- additional default + | + = note: only one variant can be default + = note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: `#[default]` attribute does not accept a value + --> $DIR/macros-nonfatal-errors.rs:28:5 + | +LL | #[default = 1] + | ^^^^^^^^^^^^^^ + | + = help: try using `#[default]` + +error: multiple `#[default]` attributes + --> $DIR/macros-nonfatal-errors.rs:36:5 + | +LL | #[default] + | ---------- `#[default]` used here +LL | #[default] + | ---------- `#[default]` used again here +LL | Foo, + | ^^^ + | + = note: only one `#[default]` attribute is needed +help: try removing this + --> $DIR/macros-nonfatal-errors.rs:35:5 + | +LL | #[default] + | ^^^^^^^^^^ + +error: multiple `#[default]` attributes + --> $DIR/macros-nonfatal-errors.rs:46:5 + | +LL | #[default] + | ---------- `#[default]` used here +LL | #[default] + | ---------- `#[default]` used again here +... +LL | Foo, + | ^^^ + | + = note: only one `#[default]` attribute is needed +help: try removing these + --> $DIR/macros-nonfatal-errors.rs:43:5 + | +LL | #[default] + | ^^^^^^^^^^ +LL | #[default] + | ^^^^^^^^^^ +LL | #[default] + | ^^^^^^^^^^ + +error: `#[default]` may only be used on unit variants + --> $DIR/macros-nonfatal-errors.rs:53:5 + | +LL | Foo {}, + | ^^^ + | + = help: consider a manual implementation of `Default` + +error: default variant must be exhaustive + --> $DIR/macros-nonfatal-errors.rs:61:5 + | +LL | #[non_exhaustive] + | ----------------- declared `#[non_exhaustive]` here +LL | Foo, + | ^^^ + | + = help: consider a manual implementation of `Default` + error: asm template must be a string literal - --> $DIR/macros-nonfatal-errors.rs:13:10 + --> $DIR/macros-nonfatal-errors.rs:66:10 | LL | asm!(invalid); | ^^^^^^^ error: inline assembly must be a string literal - --> $DIR/macros-nonfatal-errors.rs:14:15 + --> $DIR/macros-nonfatal-errors.rs:67:15 | LL | llvm_asm!(invalid); | ^^^^^^^ error: concat_idents! requires ident args. - --> $DIR/macros-nonfatal-errors.rs:16:5 + --> $DIR/macros-nonfatal-errors.rs:69:5 | LL | concat_idents!("not", "idents"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:18:17 + --> $DIR/macros-nonfatal-errors.rs:71:17 | LL | option_env!(invalid); | ^^^^^^^ error: expected string literal - --> $DIR/macros-nonfatal-errors.rs:19:10 + --> $DIR/macros-nonfatal-errors.rs:72:10 | LL | env!(invalid); | ^^^^^^^ error: expected string literal - --> $DIR/macros-nonfatal-errors.rs:20:10 + --> $DIR/macros-nonfatal-errors.rs:73:10 | LL | env!(foo, abr, baz); | ^^^ error: environment variable `RUST_HOPEFULLY_THIS_DOESNT_EXIST` not defined - --> $DIR/macros-nonfatal-errors.rs:21:5 + --> $DIR/macros-nonfatal-errors.rs:74:5 | LL | env!("RUST_HOPEFULLY_THIS_DOESNT_EXIST"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -51,7 +135,7 @@ LL | env!("RUST_HOPEFULLY_THIS_DOESNT_EXIST"); = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info) error: format argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:23:13 + --> $DIR/macros-nonfatal-errors.rs:76:13 | LL | format!(invalid); | ^^^^^^^ @@ -62,19 +146,19 @@ LL | format!("{}", invalid); | ^^^^^ error: argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:25:14 + --> $DIR/macros-nonfatal-errors.rs:78:14 | LL | include!(invalid); | ^^^^^^^ error: argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:27:18 + --> $DIR/macros-nonfatal-errors.rs:80:18 | LL | include_str!(invalid); | ^^^^^^^ error: couldn't read $DIR/i'd be quite surprised if a file with this name existed: $FILE_NOT_FOUND_MSG (os error 2) - --> $DIR/macros-nonfatal-errors.rs:28:5 + --> $DIR/macros-nonfatal-errors.rs:81:5 | LL | include_str!("i'd be quite surprised if a file with this name existed"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -82,13 +166,13 @@ LL | include_str!("i'd be quite surprised if a file with this name existed") = note: this error originates in the macro `include_str` (in Nightly builds, run with -Z macro-backtrace for more info) error: argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:29:20 + --> $DIR/macros-nonfatal-errors.rs:82:20 | LL | include_bytes!(invalid); | ^^^^^^^ error: couldn't read $DIR/i'd be quite surprised if a file with this name existed: $FILE_NOT_FOUND_MSG (os error 2) - --> $DIR/macros-nonfatal-errors.rs:30:5 + --> $DIR/macros-nonfatal-errors.rs:83:5 | LL | include_bytes!("i'd be quite surprised if a file with this name existed"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -96,11 +180,10 @@ LL | include_bytes!("i'd be quite surprised if a file with this name existed = note: this error originates in the macro `include_bytes` (in Nightly builds, run with -Z macro-backtrace for more info) error: trace_macros! accepts only `true` or `false` - --> $DIR/macros-nonfatal-errors.rs:32:5 + --> $DIR/macros-nonfatal-errors.rs:85:5 | LL | trace_macros!(invalid); | ^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 15 previous errors +error: aborting due to 21 previous errors -For more information about this error, try `rustc --explain E0665`. diff --git a/src/test/ui/resolve/issue-2356.rs b/src/test/ui/resolve/issue-2356.rs index f7b2dd13dcb64..fe9bf4d443e72 100644 --- a/src/test/ui/resolve/issue-2356.rs +++ b/src/test/ui/resolve/issue-2356.rs @@ -29,7 +29,7 @@ impl Clone for Cat { impl Default for Cat { fn default() -> Self { default(); - //~^ ERROR cannot find function `default` + //~^ ERROR cannot find function `default` in this scope [E0425] loop {} } } From eef2856eaf63bfaa39b4a124ecc691cf9a0306e8 Mon Sep 17 00:00:00 2001 From: Jacob Pratt Date: Mon, 12 Jul 2021 02:26:14 -0400 Subject: [PATCH 2/4] Add machine-applicable suggestions This avoids the need for tools like rust-analyzer to implement these suggestions themselves. --- .../src/deriving/default.rs | 50 ++++++++++++++++--- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index 7c1dc5e536555..8f672bb7d6529 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/compiler/rustc_builtin_macros/src/deriving/default.rs @@ -132,18 +132,52 @@ fn extract_default_variant<'a>( let variant = match default_variants.as_slice() { [variant] => variant, [] => { - cx.struct_span_err(trait_span, "no default declared") - .help("make a unit variant default by placing `#[default]` above it") - .emit(); + let possible_defaults = enum_def + .variants + .iter() + .filter(|variant| matches!(variant.data, VariantData::Unit(..))) + .filter(|variant| !cx.sess.contains_name(&variant.attrs, sym::non_exhaustive)); + + let mut diag = cx.struct_span_err(trait_span, "no default declared"); + diag.help("make a unit variant default by placing `#[default]` above it"); + for variant in possible_defaults { + // Suggest making each unit variant default. + diag.tool_only_span_suggestion( + variant.span, + &format!("make `{}` default", variant.ident), + format!("#[default] {}", variant.ident), + Applicability::MaybeIncorrect, + ); + } + diag.emit(); return Err(()); } [first, rest @ ..] => { - cx.struct_span_err(trait_span, "multiple declared defaults") - .span_label(first.span, "first default") - .span_labels(rest.iter().map(|variant| variant.span), "additional default") - .note("only one variant can be default") - .emit(); + let mut diag = cx.struct_span_err(trait_span, "multiple declared defaults"); + diag.span_label(first.span, "first default"); + diag.span_labels(rest.iter().map(|variant| variant.span), "additional default"); + diag.note("only one variant can be default"); + for variant in &default_variants { + // Suggest making each variant already tagged default. + let suggestion = default_variants + .iter() + .filter_map(|v| { + if v.ident == variant.ident { + None + } else { + Some((cx.sess.find_by_name(&v.attrs, kw::Default)?.span, String::new())) + } + }) + .collect(); + + diag.tool_only_multipart_suggestion( + &format!("make `{}` default", variant.ident), + suggestion, + Applicability::MaybeIncorrect, + ); + } + diag.emit(); return Err(()); } From 5ae2371cebbe9c82603739980eea20bad6dadcdb Mon Sep 17 00:00:00 2001 From: Jacob Pratt Date: Thu, 8 Jul 2021 18:26:48 -0400 Subject: [PATCH 3/4] Indicate E0665 is no longer emitted --- compiler/rustc_error_codes/src/error_codes/E0665.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0665.md b/compiler/rustc_error_codes/src/error_codes/E0665.md index a15f4021c712e..ae54d6d15798d 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0665.md +++ b/compiler/rustc_error_codes/src/error_codes/E0665.md @@ -1,8 +1,10 @@ +#### Note: this error code is no longer emitted by the compiler. + The `Default` trait was derived on an enum. Erroneous code example: -```compile_fail,E0665 +```compile_fail #[derive(Default)] enum Food { Sweet, From 72465b0a32e689cd6a61120479c13b38281108a7 Mon Sep 17 00:00:00 2001 From: Jacob Pratt Date: Sat, 17 Jul 2021 01:05:57 -0400 Subject: [PATCH 4/4] Prohibit `#[default]` in invalid places --- .../src/deriving/default.rs | 40 ++++++++- src/test/ui/macros/macros-nonfatal-errors.rs | 34 +++++++- .../ui/macros/macros-nonfatal-errors.stderr | 86 +++++++++++++------ 3 files changed, 131 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index 8f672bb7d6529..8c53094b62496 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/compiler/rustc_builtin_macros/src/deriving/default.rs @@ -2,6 +2,7 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use rustc_ast::ptr::P; +use rustc_ast::walk_list; use rustc_ast::EnumDef; use rustc_ast::VariantData; use rustc_ast::{Expr, MetaItem}; @@ -19,6 +20,8 @@ pub fn expand_deriving_default( item: &Annotatable, push: &mut dyn FnMut(Annotatable), ) { + item.visit_with(&mut DetectNonVariantDefaultAttr { cx }); + let inline = cx.meta_word(span, sym::inline); let attrs = vec![cx.attribute(inline)]; let trait_def = TraitDef { @@ -184,9 +187,12 @@ fn extract_default_variant<'a>( }; if !matches!(variant.data, VariantData::Unit(..)) { - cx.struct_span_err(variant.ident.span, "`#[default]` may only be used on unit variants") - .help("consider a manual implementation of `Default`") - .emit(); + cx.struct_span_err( + variant.ident.span, + "the `#[default]` attribute may only be used on unit enum variants", + ) + .help("consider a manual implementation of `Default`") + .emit(); return Err(()); } @@ -253,3 +259,31 @@ fn validate_default_attribute( } Ok(()) } + +struct DetectNonVariantDefaultAttr<'a, 'b> { + cx: &'a ExtCtxt<'b>, +} + +impl<'a, 'b> rustc_ast::visit::Visitor<'a> for DetectNonVariantDefaultAttr<'a, 'b> { + fn visit_attribute(&mut self, attr: &'a rustc_ast::Attribute) { + if attr.has_name(kw::Default) { + self.cx + .struct_span_err( + attr.span, + "the `#[default]` attribute may only be used on unit enum variants", + ) + .emit(); + } + + rustc_ast::visit::walk_attribute(self, attr); + } + fn visit_variant(&mut self, v: &'a rustc_ast::Variant) { + self.visit_ident(v.ident); + self.visit_vis(&v.vis); + self.visit_variant_data(&v.data); + walk_list!(self, visit_anon_const, &v.disr_expr); + for attr in &v.attrs { + rustc_ast::visit::walk_attribute(self, attr); + } + } +} diff --git a/src/test/ui/macros/macros-nonfatal-errors.rs b/src/test/ui/macros/macros-nonfatal-errors.rs index bb5f4d089bc1f..faacc6f081d31 100644 --- a/src/test/ui/macros/macros-nonfatal-errors.rs +++ b/src/test/ui/macros/macros-nonfatal-errors.rs @@ -5,8 +5,40 @@ #![feature(asm, llvm_asm)] #![feature(trace_macros, concat_idents)] +#![feature(stmt_expr_attributes, arbitrary_enum_discriminant)] #![feature(derive_default_enum)] +#[derive(Default)] +struct DefaultInnerAttrStruct { + #[default] //~ ERROR the `#[default]` attribute may only be used on unit enum variants + foo: (), +} + +#[derive(Default)] +struct DefaultInnerAttrTupleStruct(#[default] ()); +//~^ ERROR the `#[default]` attribute may only be used on unit enum variants + +#[derive(Default)] +#[default] //~ ERROR the `#[default]` attribute may only be used on unit enum variants +struct DefaultOuterAttrStruct {} + +#[derive(Default)] +#[default] //~ ERROR the `#[default]` attribute may only be used on unit enum variants +enum DefaultOuterAttrEnum { + #[default] + Foo, +} + +#[rustfmt::skip] // needs some work to handle this case +#[repr(u8)] +#[derive(Default)] +enum AttrOnInnerExpression { + Foo = #[default] 0, //~ ERROR the `#[default]` attribute may only be used on unit enum variants + Bar([u8; #[default] 1]), //~ ERROR the `#[default]` attribute may only be used on unit enum variants + #[default] + Baz, +} + #[derive(Default)] //~ ERROR no default declared enum NoDeclaredDefault { Foo, @@ -50,7 +82,7 @@ enum ManyDefaultAttrs { #[derive(Default)] enum DefaultHasFields { #[default] - Foo {}, //~ ERROR `#[default]` may only be used on unit variants + Foo {}, //~ ERROR the `#[default]` attribute may only be used on unit enum variants Bar, } diff --git a/src/test/ui/macros/macros-nonfatal-errors.stderr b/src/test/ui/macros/macros-nonfatal-errors.stderr index 4bb5e9b816950..e0ed8522bf652 100644 --- a/src/test/ui/macros/macros-nonfatal-errors.stderr +++ b/src/test/ui/macros/macros-nonfatal-errors.stderr @@ -1,5 +1,41 @@ +error: the `#[default]` attribute may only be used on unit enum variants + --> $DIR/macros-nonfatal-errors.rs:13:5 + | +LL | #[default] + | ^^^^^^^^^^ + +error: the `#[default]` attribute may only be used on unit enum variants + --> $DIR/macros-nonfatal-errors.rs:18:36 + | +LL | struct DefaultInnerAttrTupleStruct(#[default] ()); + | ^^^^^^^^^^ + +error: the `#[default]` attribute may only be used on unit enum variants + --> $DIR/macros-nonfatal-errors.rs:22:1 + | +LL | #[default] + | ^^^^^^^^^^ + +error: the `#[default]` attribute may only be used on unit enum variants + --> $DIR/macros-nonfatal-errors.rs:26:1 + | +LL | #[default] + | ^^^^^^^^^^ + +error: the `#[default]` attribute may only be used on unit enum variants + --> $DIR/macros-nonfatal-errors.rs:36:11 + | +LL | Foo = #[default] 0, + | ^^^^^^^^^^ + +error: the `#[default]` attribute may only be used on unit enum variants + --> $DIR/macros-nonfatal-errors.rs:37:14 + | +LL | Bar([u8; #[default] 1]), + | ^^^^^^^^^^ + error: no default declared - --> $DIR/macros-nonfatal-errors.rs:10:10 + --> $DIR/macros-nonfatal-errors.rs:42:10 | LL | #[derive(Default)] | ^^^^^^^ @@ -8,7 +44,7 @@ LL | #[derive(Default)] = note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) error: multiple declared defaults - --> $DIR/macros-nonfatal-errors.rs:16:10 + --> $DIR/macros-nonfatal-errors.rs:48:10 | LL | #[derive(Default)] | ^^^^^^^ @@ -26,7 +62,7 @@ LL | Baz, = note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info) error: `#[default]` attribute does not accept a value - --> $DIR/macros-nonfatal-errors.rs:28:5 + --> $DIR/macros-nonfatal-errors.rs:60:5 | LL | #[default = 1] | ^^^^^^^^^^^^^^ @@ -34,7 +70,7 @@ LL | #[default = 1] = help: try using `#[default]` error: multiple `#[default]` attributes - --> $DIR/macros-nonfatal-errors.rs:36:5 + --> $DIR/macros-nonfatal-errors.rs:68:5 | LL | #[default] | ---------- `#[default]` used here @@ -45,13 +81,13 @@ LL | Foo, | = note: only one `#[default]` attribute is needed help: try removing this - --> $DIR/macros-nonfatal-errors.rs:35:5 + --> $DIR/macros-nonfatal-errors.rs:67:5 | LL | #[default] | ^^^^^^^^^^ error: multiple `#[default]` attributes - --> $DIR/macros-nonfatal-errors.rs:46:5 + --> $DIR/macros-nonfatal-errors.rs:78:5 | LL | #[default] | ---------- `#[default]` used here @@ -63,7 +99,7 @@ LL | Foo, | = note: only one `#[default]` attribute is needed help: try removing these - --> $DIR/macros-nonfatal-errors.rs:43:5 + --> $DIR/macros-nonfatal-errors.rs:75:5 | LL | #[default] | ^^^^^^^^^^ @@ -72,8 +108,8 @@ LL | #[default] LL | #[default] | ^^^^^^^^^^ -error: `#[default]` may only be used on unit variants - --> $DIR/macros-nonfatal-errors.rs:53:5 +error: the `#[default]` attribute may only be used on unit enum variants + --> $DIR/macros-nonfatal-errors.rs:85:5 | LL | Foo {}, | ^^^ @@ -81,7 +117,7 @@ LL | Foo {}, = help: consider a manual implementation of `Default` error: default variant must be exhaustive - --> $DIR/macros-nonfatal-errors.rs:61:5 + --> $DIR/macros-nonfatal-errors.rs:93:5 | LL | #[non_exhaustive] | ----------------- declared `#[non_exhaustive]` here @@ -91,43 +127,43 @@ LL | Foo, = help: consider a manual implementation of `Default` error: asm template must be a string literal - --> $DIR/macros-nonfatal-errors.rs:66:10 + --> $DIR/macros-nonfatal-errors.rs:98:10 | LL | asm!(invalid); | ^^^^^^^ error: inline assembly must be a string literal - --> $DIR/macros-nonfatal-errors.rs:67:15 + --> $DIR/macros-nonfatal-errors.rs:99:15 | LL | llvm_asm!(invalid); | ^^^^^^^ error: concat_idents! requires ident args. - --> $DIR/macros-nonfatal-errors.rs:69:5 + --> $DIR/macros-nonfatal-errors.rs:101:5 | LL | concat_idents!("not", "idents"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:71:17 + --> $DIR/macros-nonfatal-errors.rs:103:17 | LL | option_env!(invalid); | ^^^^^^^ error: expected string literal - --> $DIR/macros-nonfatal-errors.rs:72:10 + --> $DIR/macros-nonfatal-errors.rs:104:10 | LL | env!(invalid); | ^^^^^^^ error: expected string literal - --> $DIR/macros-nonfatal-errors.rs:73:10 + --> $DIR/macros-nonfatal-errors.rs:105:10 | LL | env!(foo, abr, baz); | ^^^ error: environment variable `RUST_HOPEFULLY_THIS_DOESNT_EXIST` not defined - --> $DIR/macros-nonfatal-errors.rs:74:5 + --> $DIR/macros-nonfatal-errors.rs:106:5 | LL | env!("RUST_HOPEFULLY_THIS_DOESNT_EXIST"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -135,7 +171,7 @@ LL | env!("RUST_HOPEFULLY_THIS_DOESNT_EXIST"); = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info) error: format argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:76:13 + --> $DIR/macros-nonfatal-errors.rs:108:13 | LL | format!(invalid); | ^^^^^^^ @@ -146,19 +182,19 @@ LL | format!("{}", invalid); | ^^^^^ error: argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:78:14 + --> $DIR/macros-nonfatal-errors.rs:110:14 | LL | include!(invalid); | ^^^^^^^ error: argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:80:18 + --> $DIR/macros-nonfatal-errors.rs:112:18 | LL | include_str!(invalid); | ^^^^^^^ error: couldn't read $DIR/i'd be quite surprised if a file with this name existed: $FILE_NOT_FOUND_MSG (os error 2) - --> $DIR/macros-nonfatal-errors.rs:81:5 + --> $DIR/macros-nonfatal-errors.rs:113:5 | LL | include_str!("i'd be quite surprised if a file with this name existed"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -166,13 +202,13 @@ LL | include_str!("i'd be quite surprised if a file with this name existed") = note: this error originates in the macro `include_str` (in Nightly builds, run with -Z macro-backtrace for more info) error: argument must be a string literal - --> $DIR/macros-nonfatal-errors.rs:82:20 + --> $DIR/macros-nonfatal-errors.rs:114:20 | LL | include_bytes!(invalid); | ^^^^^^^ error: couldn't read $DIR/i'd be quite surprised if a file with this name existed: $FILE_NOT_FOUND_MSG (os error 2) - --> $DIR/macros-nonfatal-errors.rs:83:5 + --> $DIR/macros-nonfatal-errors.rs:115:5 | LL | include_bytes!("i'd be quite surprised if a file with this name existed"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -180,10 +216,10 @@ LL | include_bytes!("i'd be quite surprised if a file with this name existed = note: this error originates in the macro `include_bytes` (in Nightly builds, run with -Z macro-backtrace for more info) error: trace_macros! accepts only `true` or `false` - --> $DIR/macros-nonfatal-errors.rs:85:5 + --> $DIR/macros-nonfatal-errors.rs:117:5 | LL | trace_macros!(invalid); | ^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 21 previous errors +error: aborting due to 27 previous errors