Auto merge of rust-lang#106884 - clubby789:fieldless-enum-debug, r=michaelwoerister

Simplify `derive(Debug)` output for fieldless enums

Fixes rust-lang#106875

Author: bors

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))
         }

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)
             })),

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(),

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))),
     }];
 

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)
         })),

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(),
-            unify_fieldless_variants: false,
+            fieldless_variants_strategy:
+                FieldlessVariantsStrategy::SpecializeIfAllVariantsFieldless,
             combine_substructure: combine_substructure(Box::new(|a, b, c| {
                 show_substructure(a, b, c)
             })),
@@ -43,16 +45,18 @@ 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),
+        AllFieldlessEnum(enum_def) => return show_fieldless_enum(cx, span, enum_def, substr),
         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();
 
@@ -173,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::<Vec<_>>();
+    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]))
+}

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)
             })),

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) => {

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)
             })),

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<CombineSubstructureFunc<'a>>,
 }
 
+/// 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<FieldInfo>),
 
+    /// 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<ast::Arm> = 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`"),
     }
 }

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)
             })),

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]