Skip to content

fix: Fix some things with builtin derives #20167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 4, 2025
Merged

fix: Fix some things with builtin derives #20167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions crates/hir-def/src/macro_expansion_tests/builtin_derive_macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}"#]],
);
}
140 changes: 82 additions & 58 deletions crates/hir-expand/src/builtin/derive_macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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 =>
Expand Down Expand Up @@ -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(
Expand All @@ -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| {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 =>
Expand Down Expand Up @@ -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(
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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)| {
Expand Down Expand Up @@ -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,
Expand All @@ -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));

Expand Down
4 changes: 4 additions & 0 deletions crates/ide-completion/src/tests/attribute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
22 changes: 12 additions & 10 deletions crates/test-utils/src/fixture.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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..);
}
}

Expand Down
Loading