From 97cf1713d19d01666bf4613fae51eacce3974640 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Sat, 14 Jan 2023 21:16:25 +0000 Subject: [PATCH 1/2] Add enum for fieldless unification --- .../src/deriving/clone.rs | 6 +- .../src/deriving/cmp/eq.rs | 2 +- .../src/deriving/cmp/ord.rs | 2 +- .../src/deriving/cmp/partial_eq.rs | 2 +- .../src/deriving/cmp/partial_ord.rs | 2 +- .../src/deriving/debug.rs | 9 +- .../src/deriving/decodable.rs | 2 +- .../src/deriving/default.rs | 2 +- .../src/deriving/encodable.rs | 2 +- .../src/deriving/generic/mod.rs | 90 +++++++++++++------ .../rustc_builtin_macros/src/deriving/hash.rs | 2 +- 11 files changed, 78 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/clone.rs b/compiler/rustc_builtin_macros/src/deriving/clone.rs index d59b3b8c86d..bc7e8b6a223 100644 --- a/compiler/rustc_builtin_macros/src/deriving/clone.rs +++ b/compiler/rustc_builtin_macros/src/deriving/clone.rs @@ -82,7 +82,7 @@ pub fn expand_deriving_clone( nonself_args: Vec::new(), ret_ty: Self_, attributes: attrs, - unify_fieldless_variants: false, + fieldless_variants_strategy: FieldlessVariantsStrategy::Default, combine_substructure: substructure, }], associated_types: Vec::new(), @@ -177,7 +177,9 @@ fn cs_clone( all_fields = af; vdata = &variant.data; } - EnumTag(..) => cx.span_bug(trait_span, &format!("enum tags in `derive({})`", name,)), + EnumTag(..) | AllFieldlessEnum(..) => { + cx.span_bug(trait_span, &format!("enum tags in `derive({})`", name,)) + } StaticEnum(..) | StaticStruct(..) => { cx.span_bug(trait_span, &format!("associated function in `derive({})`", name)) } diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs index f861d47ed40..3e994f037ad 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs @@ -36,7 +36,7 @@ pub fn expand_deriving_eq( nonself_args: vec![], ret_ty: Unit, attributes: attrs, - unify_fieldless_variants: true, + fieldless_variants_strategy: FieldlessVariantsStrategy::Unify, combine_substructure: combine_substructure(Box::new(|a, b, c| { cs_total_eq_assert(a, b, c) })), diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs index 96d18c7afb9..a926fca4e65 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs @@ -29,7 +29,7 @@ pub fn expand_deriving_ord( nonself_args: vec![(self_ref(), sym::other)], ret_ty: Path(path_std!(cmp::Ordering)), attributes: attrs, - unify_fieldless_variants: true, + fieldless_variants_strategy: FieldlessVariantsStrategy::Unify, combine_substructure: combine_substructure(Box::new(|a, b, c| cs_cmp(a, b, c))), }], associated_types: Vec::new(), diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs index 7f95551fc48..9051fe0b28a 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs @@ -76,7 +76,7 @@ pub fn expand_deriving_partial_eq( nonself_args: vec![(self_ref(), sym::other)], ret_ty: Path(path_local!(bool)), attributes: attrs, - unify_fieldless_variants: true, + fieldless_variants_strategy: FieldlessVariantsStrategy::Unify, combine_substructure: combine_substructure(Box::new(|a, b, c| cs_eq(a, b, c))), }]; diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs index 5c4e5b7f816..c9dc8921262 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs @@ -28,7 +28,7 @@ pub fn expand_deriving_partial_ord( nonself_args: vec![(self_ref(), sym::other)], ret_ty, attributes: attrs, - unify_fieldless_variants: true, + fieldless_variants_strategy: FieldlessVariantsStrategy::Unify, combine_substructure: combine_substructure(Box::new(|cx, span, substr| { cs_partial_cmp(cx, span, substr) })), diff --git a/compiler/rustc_builtin_macros/src/deriving/debug.rs b/compiler/rustc_builtin_macros/src/deriving/debug.rs index 5b1b7e6804c..a586964d0c3 100644 --- a/compiler/rustc_builtin_macros/src/deriving/debug.rs +++ b/compiler/rustc_builtin_macros/src/deriving/debug.rs @@ -31,7 +31,7 @@ pub fn expand_deriving_debug( nonself_args: vec![(fmtr, sym::f)], ret_ty: Path(path_std!(fmt::Result)), attributes: ast::AttrVec::new(), - unify_fieldless_variants: false, + fieldless_variants_strategy: FieldlessVariantsStrategy::Default, combine_substructure: combine_substructure(Box::new(|a, b, c| { show_substructure(a, b, c) })), @@ -43,16 +43,17 @@ pub fn expand_deriving_debug( } fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr { + // We want to make sure we have the ctxt set so that we can use unstable methods + let span = cx.with_def_site_ctxt(span); + let (ident, vdata, fields) = match substr.fields { Struct(vdata, fields) => (substr.type_ident, *vdata, fields), EnumMatching(_, _, v, fields) => (v.ident, &v.data, fields), - EnumTag(..) | StaticStruct(..) | StaticEnum(..) => { + AllFieldlessEnum(..) | EnumTag(..) | StaticStruct(..) | StaticEnum(..) => { cx.span_bug(span, "nonsensical .fields in `#[derive(Debug)]`") } }; - // We want to make sure we have the ctxt set so that we can use unstable methods - let span = cx.with_def_site_ctxt(span); let name = cx.expr_str(span, ident.name); let fmt = substr.nonselflike_args[0].clone(); diff --git a/compiler/rustc_builtin_macros/src/deriving/decodable.rs b/compiler/rustc_builtin_macros/src/deriving/decodable.rs index 62af02c2bb4..5f9519dad1b 100644 --- a/compiler/rustc_builtin_macros/src/deriving/decodable.rs +++ b/compiler/rustc_builtin_macros/src/deriving/decodable.rs @@ -49,7 +49,7 @@ pub fn expand_deriving_rustc_decodable( PathKind::Std, )), attributes: ast::AttrVec::new(), - unify_fieldless_variants: false, + fieldless_variants_strategy: FieldlessVariantsStrategy::Default, combine_substructure: combine_substructure(Box::new(|a, b, c| { decodable_substructure(a, b, c, krate) })), diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index eb66c4a69a6..18270747296 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/compiler/rustc_builtin_macros/src/deriving/default.rs @@ -34,7 +34,7 @@ pub fn expand_deriving_default( nonself_args: Vec::new(), ret_ty: Self_, attributes: attrs, - unify_fieldless_variants: false, + fieldless_variants_strategy: FieldlessVariantsStrategy::Default, combine_substructure: combine_substructure(Box::new(|cx, trait_span, substr| { match substr.fields { StaticStruct(_, fields) => { diff --git a/compiler/rustc_builtin_macros/src/deriving/encodable.rs b/compiler/rustc_builtin_macros/src/deriving/encodable.rs index 68bc0ff2ec0..2afeed927ac 100644 --- a/compiler/rustc_builtin_macros/src/deriving/encodable.rs +++ b/compiler/rustc_builtin_macros/src/deriving/encodable.rs @@ -133,7 +133,7 @@ pub fn expand_deriving_rustc_encodable( PathKind::Std, )), attributes: AttrVec::new(), - unify_fieldless_variants: false, + fieldless_variants_strategy: FieldlessVariantsStrategy::Default, combine_substructure: combine_substructure(Box::new(|a, b, c| { encodable_substructure(a, b, c, krate) })), diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index beac591bfc8..17b7ac0eba1 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -222,14 +222,27 @@ pub struct MethodDef<'a> { pub attributes: ast::AttrVec, - /// Can we combine fieldless variants for enums into a single match arm? - /// If true, indicates that the trait operation uses the enum tag in some - /// way. - pub unify_fieldless_variants: bool, + pub fieldless_variants_strategy: FieldlessVariantsStrategy, pub combine_substructure: RefCell>, } +/// How to handle fieldless enum variants. +#[derive(PartialEq)] +pub enum FieldlessVariantsStrategy { + /// Combine fieldless variants into a single match arm. + /// This assumes that relevant information has been handled + /// by looking at the enum's discriminant. + Unify, + /// Don't do anything special about fieldless variants. They are + /// handled like any other variant. + Default, + /// If all variants of the enum are fieldless, expand the special + /// `AllFieldLessEnum` substructure, so that the entire enum can be handled + /// at once. + SpecializeIfAllVariantsFieldless, +} + /// All the data about the data structure/method being derived upon. pub struct Substructure<'a> { /// ident of self @@ -264,9 +277,14 @@ pub enum StaticFields { /// A summary of the possible sets of fields. pub enum SubstructureFields<'a> { - /// A non-static method with `Self` is a struct. + /// A non-static method where `Self` is a struct. Struct(&'a ast::VariantData, Vec), + /// A non-static method handling the entire enum at once + /// (after it has been determined that none of the enum + /// variants has any fields). + AllFieldlessEnum(&'a ast::EnumDef), + /// Matching variants of the enum: variant index, variant count, ast::Variant, /// fields: the field name is only non-`None` in the case of a struct /// variant. @@ -1086,8 +1104,8 @@ impl<'a> MethodDef<'a> { /// ``` /// Creates a tag check combined with a match for a tuple of all /// `selflike_args`, with an arm for each variant with fields, possibly an - /// arm for each fieldless variant (if `!unify_fieldless_variants` is not - /// true), and possibly a default arm. + /// arm for each fieldless variant (if `unify_fieldless_variants` is not + /// `Unify`), and possibly a default arm. fn expand_enum_method_body<'b>( &self, cx: &mut ExtCtxt<'_>, @@ -1101,7 +1119,8 @@ impl<'a> MethodDef<'a> { let variants = &enum_def.variants; // Traits that unify fieldless variants always use the tag(s). - let uses_tags = self.unify_fieldless_variants; + let unify_fieldless_variants = + self.fieldless_variants_strategy == FieldlessVariantsStrategy::Unify; // There is no sensible code to be generated for *any* deriving on a // zero-variant enum. So we just generate a failing expression. @@ -1161,23 +1180,35 @@ impl<'a> MethodDef<'a> { // match is necessary. let all_fieldless = variants.iter().all(|v| v.data.fields().is_empty()); if all_fieldless { - if uses_tags && variants.len() > 1 { - // If the type is fieldless and the trait uses the tag and - // there are multiple variants, we need just an operation on - // the tag(s). - let (tag_field, mut tag_let_stmts) = get_tag_pieces(cx); - let mut tag_check = self.call_substructure_method( - cx, - trait_, - type_ident, - nonselflike_args, - &EnumTag(tag_field, None), - ); - tag_let_stmts.append(&mut tag_check.0); - return BlockOrExpr(tag_let_stmts, tag_check.1); - } - - if variants.len() == 1 { + if variants.len() > 1 { + match self.fieldless_variants_strategy { + FieldlessVariantsStrategy::Unify => { + // If the type is fieldless and the trait uses the tag and + // there are multiple variants, we need just an operation on + // the tag(s). + let (tag_field, mut tag_let_stmts) = get_tag_pieces(cx); + let mut tag_check = self.call_substructure_method( + cx, + trait_, + type_ident, + nonselflike_args, + &EnumTag(tag_field, None), + ); + tag_let_stmts.append(&mut tag_check.0); + return BlockOrExpr(tag_let_stmts, tag_check.1); + } + FieldlessVariantsStrategy::SpecializeIfAllVariantsFieldless => { + return self.call_substructure_method( + cx, + trait_, + type_ident, + nonselflike_args, + &AllFieldlessEnum(enum_def), + ); + } + FieldlessVariantsStrategy::Default => (), + } + } else if variants.len() == 1 { // If there is a single variant, we don't need an operation on // the tag(s). Just use the most degenerate result. return self.call_substructure_method( @@ -1187,7 +1218,7 @@ impl<'a> MethodDef<'a> { nonselflike_args, &EnumMatching(0, 1, &variants[0], Vec::new()), ); - }; + } } // These arms are of the form: @@ -1198,7 +1229,7 @@ impl<'a> MethodDef<'a> { let mut match_arms: Vec = variants .iter() .enumerate() - .filter(|&(_, v)| !(self.unify_fieldless_variants && v.data.fields().is_empty())) + .filter(|&(_, v)| !(unify_fieldless_variants && v.data.fields().is_empty())) .map(|(index, variant)| { // A single arm has form (&VariantK, &VariantK, ...) => BodyK // (see "Final wrinkle" note below for why.) @@ -1249,7 +1280,7 @@ impl<'a> MethodDef<'a> { // Add a default arm to the match, if necessary. let first_fieldless = variants.iter().find(|v| v.data.fields().is_empty()); let default = match first_fieldless { - Some(v) if self.unify_fieldless_variants => { + Some(v) if unify_fieldless_variants => { // We need a default case that handles all the fieldless // variants. The index and actual variant aren't meaningful in // this case, so just use dummy values. @@ -1296,7 +1327,7 @@ impl<'a> MethodDef<'a> { // If the trait uses the tag and there are multiple variants, we need // to add a tag check operation before the match. Otherwise, the match // is enough. - if uses_tags && variants.len() > 1 { + if unify_fieldless_variants && variants.len() > 1 { let (tag_field, mut tag_let_stmts) = get_tag_pieces(cx); // Combine a tag check with the match. @@ -1580,5 +1611,6 @@ where } } StaticEnum(..) | StaticStruct(..) => cx.span_bug(trait_span, "static function in `derive`"), + AllFieldlessEnum(..) => cx.span_bug(trait_span, "fieldless enum in `derive`"), } } diff --git a/compiler/rustc_builtin_macros/src/deriving/hash.rs b/compiler/rustc_builtin_macros/src/deriving/hash.rs index c136bb7141a..f8570d8f86a 100644 --- a/compiler/rustc_builtin_macros/src/deriving/hash.rs +++ b/compiler/rustc_builtin_macros/src/deriving/hash.rs @@ -33,7 +33,7 @@ pub fn expand_deriving_hash( nonself_args: vec![(Ref(Box::new(Path(arg)), Mutability::Mut), sym::state)], ret_ty: Unit, attributes: AttrVec::new(), - unify_fieldless_variants: true, + fieldless_variants_strategy: FieldlessVariantsStrategy::Unify, combine_substructure: combine_substructure(Box::new(|a, b, c| { hash_substructure(a, b, c) })), From 95a824c02c3f9a50bb4fa49e7af7d3a5a2faf51e Mon Sep 17 00:00:00 2001 From: clubby789 Date: Thu, 19 Jan 2023 15:52:29 +0000 Subject: [PATCH 2/2] Special case `derive(Debug)` for fieldless enums --- .../src/deriving/debug.rs | 51 ++++++++++++++++++- tests/ui/deriving/deriving-all-codegen.stdout | 11 ++-- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/debug.rs b/compiler/rustc_builtin_macros/src/deriving/debug.rs index a586964d0c3..e0f487e8648 100644 --- a/compiler/rustc_builtin_macros/src/deriving/debug.rs +++ b/compiler/rustc_builtin_macros/src/deriving/debug.rs @@ -2,6 +2,7 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use crate::deriving::path_std; +use ast::EnumDef; use rustc_ast::{self as ast, MetaItem}; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{sym, Ident, Symbol}; @@ -31,7 +32,8 @@ pub fn expand_deriving_debug( nonself_args: vec![(fmtr, sym::f)], ret_ty: Path(path_std!(fmt::Result)), attributes: ast::AttrVec::new(), - fieldless_variants_strategy: FieldlessVariantsStrategy::Default, + fieldless_variants_strategy: + FieldlessVariantsStrategy::SpecializeIfAllVariantsFieldless, combine_substructure: combine_substructure(Box::new(|a, b, c| { show_substructure(a, b, c) })), @@ -49,7 +51,8 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_> let (ident, vdata, fields) = match substr.fields { Struct(vdata, fields) => (substr.type_ident, *vdata, fields), EnumMatching(_, _, v, fields) => (v.ident, &v.data, fields), - AllFieldlessEnum(..) | EnumTag(..) | StaticStruct(..) | StaticEnum(..) => { + AllFieldlessEnum(enum_def) => return show_fieldless_enum(cx, span, enum_def, substr), + EnumTag(..) | StaticStruct(..) | StaticEnum(..) => { cx.span_bug(span, "nonsensical .fields in `#[derive(Debug)]`") } }; @@ -174,3 +177,47 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_> BlockOrExpr::new_mixed(stmts, Some(expr)) } } + +/// Special case for enums with no fields. Builds: +/// ```text +/// impl ::core::fmt::Debug for A { +/// fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { +/// ::core::fmt::Formatter::write_str(f, +/// match self { +/// A::A => "A", +/// A::B() => "B", +/// A::C {} => "C", +/// }) +/// } +/// } +/// ``` +fn show_fieldless_enum( + cx: &mut ExtCtxt<'_>, + span: Span, + def: &EnumDef, + substr: &Substructure<'_>, +) -> BlockOrExpr { + let fmt = substr.nonselflike_args[0].clone(); + let arms = def + .variants + .iter() + .map(|v| { + let variant_path = cx.path(span, vec![substr.type_ident, v.ident]); + let pat = match &v.data { + ast::VariantData::Tuple(fields, _) => { + debug_assert!(fields.is_empty()); + cx.pat_tuple_struct(span, variant_path, vec![]) + } + ast::VariantData::Struct(fields, _) => { + debug_assert!(fields.is_empty()); + cx.pat_struct(span, variant_path, vec![]) + } + ast::VariantData::Unit(_) => cx.pat_path(span, variant_path), + }; + cx.arm(span, pat, cx.expr_str(span, v.ident.name)) + }) + .collect::>(); + let name = cx.expr_match(span, cx.expr_self(span), arms); + let fn_path_write_str = cx.std_path(&[sym::fmt, sym::Formatter, sym::write_str]); + BlockOrExpr::new_expr(cx.expr_call_global(span, fn_path_write_str, vec![fmt, name])) +} diff --git a/tests/ui/deriving/deriving-all-codegen.stdout b/tests/ui/deriving/deriving-all-codegen.stdout index a63cbd4ca7e..e6ee11a783b 100644 --- a/tests/ui/deriving/deriving-all-codegen.stdout +++ b/tests/ui/deriving/deriving-all-codegen.stdout @@ -731,11 +731,12 @@ impl ::core::marker::Copy for Fieldless { } #[automatically_derived] impl ::core::fmt::Debug for Fieldless { fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { - match self { - Fieldless::A => ::core::fmt::Formatter::write_str(f, "A"), - Fieldless::B => ::core::fmt::Formatter::write_str(f, "B"), - Fieldless::C => ::core::fmt::Formatter::write_str(f, "C"), - } + ::core::fmt::Formatter::write_str(f, + match self { + Fieldless::A => "A", + Fieldless::B => "B", + Fieldless::C => "C", + }) } } #[automatically_derived]