Unnamed repository; edit this file 'description' to name the repository.
Fix some things with builtin derives
1. Err on unions on derive where it's required.
2. Err on `#[derive(Default)]` on enums without `#[default]` variant.
3. Don't add where bounds `T: Default` when expanding `Default` on enums (structs need that, enums not).
Also, because I was annoyed by that, in minicore, add a way to filter on multiple flags in the line-filter (`// :`). This is required for the `Debug` and `Hash` derives, because the derive should be in the prelude but the trait not.
| -rw-r--r-- | crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs | 80 | ||||
| -rw-r--r-- | crates/hir-expand/src/builtin/derive_macro.rs | 140 | ||||
| -rw-r--r-- | crates/ide-completion/src/tests/attribute.rs | 4 | ||||
| -rw-r--r-- | crates/test-utils/src/fixture.rs | 22 | ||||
| -rw-r--r-- | crates/test-utils/src/minicore.rs | 16 |
5 files changed, 190 insertions, 72 deletions
diff --git a/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs b/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs index 777953d3f2..0013c2a256 100644 --- a/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs +++ b/crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs @@ -746,3 +746,83 @@ struct Struct9<#[pointee] T, U>(T) where T: ?Sized; 623..690: `derive(CoercePointee)` requires `T` to be marked `?Sized`"#]], ); } + +#[test] +fn union_derive() { + check_errors( + r#" +//- minicore: clone, copy, default, fmt, hash, ord, eq, derive + +#[derive(Copy)] +union Foo1 { _v: () } +#[derive(Clone)] +union Foo2 { _v: () } +#[derive(Default)] +union Foo3 { _v: () } +#[derive(Debug)] +union Foo4 { _v: () } +#[derive(Hash)] +union Foo5 { _v: () } +#[derive(Ord)] +union Foo6 { _v: () } +#[derive(PartialOrd)] +union Foo7 { _v: () } +#[derive(Eq)] +union Foo8 { _v: () } +#[derive(PartialEq)] +union Foo9 { _v: () } + "#, + expect![[r#" + 78..118: this trait cannot be derived for unions + 119..157: this trait cannot be derived for unions + 158..195: this trait cannot be derived for unions + 196..232: this trait cannot be derived for unions + 233..276: this trait cannot be derived for unions + 313..355: this trait cannot be derived for unions"#]], + ); +} + +#[test] +fn default_enum_without_default_attr() { + check_errors( + r#" +//- minicore: default, derive + +#[derive(Default)] +enum Foo { + Bar, +} + "#, + expect!["1..41: `#[derive(Default)]` on enum with no `#[default]`"], + ); +} + +#[test] +fn generic_enum_default() { + check( + r#" +//- minicore: default, derive + +#[derive(Default)] +enum Foo<T> { + Bar(T), + #[default] + Baz, +} +"#, + expect![[r#" + +#[derive(Default)] +enum Foo<T> { + Bar(T), + #[default] + Baz, +} + +impl <T, > $crate::default::Default for Foo<T, > where { + fn default() -> Self { + Foo::Baz + } +}"#]], + ); +} diff --git a/crates/hir-expand/src/builtin/derive_macro.rs b/crates/hir-expand/src/builtin/derive_macro.rs index d135584a08..15e68ff95c 100644 --- a/crates/hir-expand/src/builtin/derive_macro.rs +++ b/crates/hir-expand/src/builtin/derive_macro.rs @@ -458,6 +458,7 @@ fn expand_simple_derive( invoc_span: Span, tt: &tt::TopSubtree, trait_path: tt::TopSubtree, + allow_unions: bool, make_trait_body: impl FnOnce(&BasicAdtInfo) -> tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let info = match parse_adt(db, tt, invoc_span) { @@ -469,6 +470,12 @@ fn expand_simple_derive( ); } }; + if !allow_unions && matches!(info.shape, AdtShape::Union) { + return ExpandResult::new( + tt::TopSubtree::empty(tt::DelimSpan::from_single(invoc_span)), + ExpandError::other(invoc_span, "this trait cannot be derived for unions"), + ); + } ExpandResult::ok(expand_simple_derive_with_parsed( invoc_span, info, @@ -535,7 +542,14 @@ fn copy_expand( tt: &tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let krate = dollar_crate(span); - expand_simple_derive(db, span, tt, quote! {span => #krate::marker::Copy }, |_| quote! {span =>}) + expand_simple_derive( + db, + span, + tt, + quote! {span => #krate::marker::Copy }, + true, + |_| quote! {span =>}, + ) } fn clone_expand( @@ -544,7 +558,7 @@ fn clone_expand( tt: &tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let krate = dollar_crate(span); - expand_simple_derive(db, span, tt, quote! {span => #krate::clone::Clone }, |adt| { + expand_simple_derive(db, span, tt, quote! {span => #krate::clone::Clone }, true, |adt| { if matches!(adt.shape, AdtShape::Union) { let star = tt::Punct { char: '*', spacing: ::tt::Spacing::Alone, span }; return quote! {span => @@ -599,41 +613,63 @@ fn default_expand( tt: &tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let krate = &dollar_crate(span); - expand_simple_derive(db, span, tt, quote! {span => #krate::default::Default }, |adt| { - let body = match &adt.shape { - AdtShape::Struct(fields) => { - let name = &adt.name; - fields.as_pattern_map( - quote!(span =>#name), + let adt = match parse_adt(db, tt, span) { + Ok(info) => info, + Err(e) => { + return ExpandResult::new( + tt::TopSubtree::empty(tt::DelimSpan { open: span, close: span }), + e, + ); + } + }; + let (body, constrain_to_trait) = match &adt.shape { + AdtShape::Struct(fields) => { + let name = &adt.name; + let body = fields.as_pattern_map( + quote!(span =>#name), + span, + |_| quote!(span =>#krate::default::Default::default()), + ); + (body, true) + } + AdtShape::Enum { default_variant, variants } => { + if let Some(d) = default_variant { + let (name, fields) = &variants[*d]; + let adt_name = &adt.name; + let body = fields.as_pattern_map( + quote!(span =>#adt_name :: #name), span, |_| quote!(span =>#krate::default::Default::default()), - ) + ); + (body, false) + } else { + return ExpandResult::new( + tt::TopSubtree::empty(tt::DelimSpan::from_single(span)), + ExpandError::other(span, "`#[derive(Default)]` on enum with no `#[default]`"), + ); } - AdtShape::Enum { default_variant, variants } => { - if let Some(d) = default_variant { - let (name, fields) = &variants[*d]; - let adt_name = &adt.name; - fields.as_pattern_map( - quote!(span =>#adt_name :: #name), - span, - |_| quote!(span =>#krate::default::Default::default()), - ) - } else { - // FIXME: Return expand error here - quote!(span =>) + } + AdtShape::Union => { + return ExpandResult::new( + tt::TopSubtree::empty(tt::DelimSpan::from_single(span)), + ExpandError::other(span, "this trait cannot be derived for unions"), + ); + } + }; + ExpandResult::ok(expand_simple_derive_with_parsed( + span, + adt, + quote! {span => #krate::default::Default }, + |_adt| { + quote! {span => + fn default() -> Self { + #body } } - AdtShape::Union => { - // FIXME: Return expand error here - quote!(span =>) - } - }; - quote! {span => - fn default() -> Self { - #body - } - } - }) + }, + constrain_to_trait, + tt::TopSubtree::empty(tt::DelimSpan::from_single(span)), + )) } fn debug_expand( @@ -642,7 +678,7 @@ fn debug_expand( tt: &tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let krate = &dollar_crate(span); - expand_simple_derive(db, span, tt, quote! {span => #krate::fmt::Debug }, |adt| { + expand_simple_derive(db, span, tt, quote! {span => #krate::fmt::Debug }, false, |adt| { let for_variant = |name: String, v: &VariantShape| match v { VariantShape::Struct(fields) => { let for_fields = fields.iter().map(|it| { @@ -697,10 +733,7 @@ fn debug_expand( } }) .collect(), - AdtShape::Union => { - // FIXME: Return expand error here - vec![] - } + AdtShape::Union => unreachable!(), }; quote! {span => fn fmt(&self, f: &mut #krate::fmt::Formatter) -> #krate::fmt::Result { @@ -718,11 +751,7 @@ fn hash_expand( tt: &tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let krate = &dollar_crate(span); - expand_simple_derive(db, span, tt, quote! {span => #krate::hash::Hash }, |adt| { - if matches!(adt.shape, AdtShape::Union) { - // FIXME: Return expand error here - return quote! {span =>}; - } + expand_simple_derive(db, span, tt, quote! {span => #krate::hash::Hash }, false, |adt| { if matches!(&adt.shape, AdtShape::Enum { variants, .. } if variants.is_empty()) { let star = tt::Punct { char: '*', spacing: ::tt::Spacing::Alone, span }; return quote! {span => @@ -769,7 +798,14 @@ fn eq_expand( tt: &tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let krate = dollar_crate(span); - expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::Eq }, |_| quote! {span =>}) + expand_simple_derive( + db, + span, + tt, + quote! {span => #krate::cmp::Eq }, + true, + |_| quote! {span =>}, + ) } fn partial_eq_expand( @@ -778,11 +814,7 @@ fn partial_eq_expand( tt: &tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let krate = dollar_crate(span); - expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::PartialEq }, |adt| { - if matches!(adt.shape, AdtShape::Union) { - // FIXME: Return expand error here - return quote! {span =>}; - } + expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::PartialEq }, false, |adt| { let name = &adt.name; let (self_patterns, other_patterns) = self_and_other_patterns(adt, name, span); @@ -854,7 +886,7 @@ fn ord_expand( tt: &tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let krate = &dollar_crate(span); - expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::Ord }, |adt| { + expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::Ord }, false, |adt| { fn compare( krate: &tt::Ident, left: tt::TopSubtree, @@ -873,10 +905,6 @@ fn ord_expand( } } } - if matches!(adt.shape, AdtShape::Union) { - // FIXME: Return expand error here - return quote!(span =>); - } let (self_patterns, other_patterns) = self_and_other_patterns(adt, &adt.name, span); let arms = izip!(self_patterns, other_patterns, adt.shape.field_names(span)).map( |(pat1, pat2, fields)| { @@ -916,7 +944,7 @@ fn partial_ord_expand( tt: &tt::TopSubtree, ) -> ExpandResult<tt::TopSubtree> { let krate = &dollar_crate(span); - expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::PartialOrd }, |adt| { + expand_simple_derive(db, span, tt, quote! {span => #krate::cmp::PartialOrd }, false, |adt| { fn compare( krate: &tt::Ident, left: tt::TopSubtree, @@ -935,10 +963,6 @@ fn partial_ord_expand( } } } - if matches!(adt.shape, AdtShape::Union) { - // FIXME: Return expand error here - return quote!(span =>); - } let left = quote!(span =>#krate::intrinsics::discriminant_value(self)); let right = quote!(span =>#krate::intrinsics::discriminant_value(other)); diff --git a/crates/ide-completion/src/tests/attribute.rs b/crates/ide-completion/src/tests/attribute.rs index 411902f111..46a3630045 100644 --- a/crates/ide-completion/src/tests/attribute.rs +++ b/crates/ide-completion/src/tests/attribute.rs @@ -878,6 +878,7 @@ mod derive { expect![[r#" de Clone macro Clone de Clone, Copy + de Debug macro Debug de Default macro Default de PartialEq macro PartialEq de PartialEq, Eq @@ -900,6 +901,7 @@ mod derive { expect![[r#" de Clone macro Clone de Clone, Copy + de Debug macro Debug de Default macro Default de Eq de Eq, PartialOrd, Ord @@ -921,6 +923,7 @@ mod derive { expect![[r#" de Clone macro Clone de Clone, Copy + de Debug macro Debug de Default macro Default de Eq de Eq, PartialOrd, Ord @@ -942,6 +945,7 @@ mod derive { expect![[r#" de Clone macro Clone de Clone, Copy + de Debug macro Debug de Default macro Default de PartialOrd de PartialOrd, Ord diff --git a/crates/test-utils/src/fixture.rs b/crates/test-utils/src/fixture.rs index 1d821e96e5..e830c6a7cf 100644 --- a/crates/test-utils/src/fixture.rs +++ b/crates/test-utils/src/fixture.rs @@ -435,14 +435,16 @@ impl MiniCore { continue; } - let mut active_line_region = false; - let mut inactive_line_region = false; + let mut active_line_region = 0; + let mut inactive_line_region = 0; if let Some(idx) = trimmed.find("// :!") { - inactive_line_region = true; - inactive_regions.push(&trimmed[idx + "// :!".len()..]); + let regions = trimmed[idx + "// :!".len()..].split(", "); + inactive_line_region += regions.clone().count(); + inactive_regions.extend(regions); } else if let Some(idx) = trimmed.find("// :") { - active_line_region = true; - active_regions.push(&trimmed[idx + "// :".len()..]); + let regions = trimmed[idx + "// :".len()..].split(", "); + active_line_region += regions.clone().count(); + active_regions.extend(regions); } let mut keep = true; @@ -462,11 +464,11 @@ impl MiniCore { if keep { buf.push_str(line); } - if active_line_region { - active_regions.pop().unwrap(); + if active_line_region > 0 { + active_regions.drain(active_regions.len() - active_line_region..); } - if inactive_line_region { - inactive_regions.pop().unwrap(); + if inactive_line_region > 0 { + inactive_regions.drain(inactive_regions.len() - active_line_region..); } } diff --git a/crates/test-utils/src/minicore.rs b/crates/test-utils/src/minicore.rs index d48063fb86..c79aaba054 100644 --- a/crates/test-utils/src/minicore.rs +++ b/crates/test-utils/src/minicore.rs @@ -228,8 +228,11 @@ pub mod hash { } // region:derive - #[rustc_builtin_macro] - pub macro Hash($item:item) {} + pub(crate) mod derive { + #[rustc_builtin_macro] + pub macro Hash($item:item) {} + } + pub use derive::Hash; // endregion:derive } // endregion:hash @@ -1264,8 +1267,11 @@ pub mod fmt { } // region:derive - #[rustc_builtin_macro] - pub macro Debug($item:item) {} + pub(crate) mod derive { + #[rustc_builtin_macro] + pub macro Debug($item:item) {} + } + pub use derive::Debug; // endregion:derive // region:builtin_impls @@ -1931,6 +1937,8 @@ pub mod prelude { panic, // :panic result::Result::{self, Err, Ok}, // :result str::FromStr, // :str + fmt::derive::Debug, // :fmt, derive + hash::derive::Hash, // :hash, derive }; } |