Skip to content

Simplify derive(Debug) output for fieldless enums #106884

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.

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 2 commits into from
Jan 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions compiler/rustc_builtin_macros/src/deriving/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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))
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
}];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})),
Expand Down
54 changes: 51 additions & 3 deletions compiler/rustc_builtin_macros/src/deriving/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)
})),
Expand All @@ -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();

Expand Down Expand Up @@ -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]))
}
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/decodable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/encodable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})),
Expand Down
90 changes: 61 additions & 29 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<'_>,
Expand All @@ -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.
Expand Down Expand Up @@ -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(
Expand All @@ -1187,7 +1218,7 @@ impl<'a> MethodDef<'a> {
nonselflike_args,
&EnumMatching(0, 1, &variants[0], Vec::new()),
);
};
}
}

// These arms are of the form:
Expand All @@ -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.)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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`"),
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/deriving/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})),
Expand Down
11 changes: 6 additions & 5 deletions tests/ui/deriving/deriving-all-codegen.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down