From 5762d2385ebdd735b110ab82e80eed6efa7d7c55 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 28 Jun 2022 13:10:36 +1000 Subject: Avoid unnecessary blocks in derive output. By not committing to either block form or expression form until necessary, we can avoid lots of unnecessary blocks. --- .../rustc_builtin_macros/src/deriving/clone.rs | 15 ++- .../rustc_builtin_macros/src/deriving/cmp/eq.rs | 7 +- .../rustc_builtin_macros/src/deriving/cmp/ord.rs | 9 +- .../src/deriving/cmp/partial_eq.rs | 13 ++- .../src/deriving/cmp/partial_ord.rs | 10 +- .../rustc_builtin_macros/src/deriving/debug.rs | 15 ++- .../rustc_builtin_macros/src/deriving/decodable.rs | 7 +- .../rustc_builtin_macros/src/deriving/default.rs | 41 ++++---- .../rustc_builtin_macros/src/deriving/encodable.rs | 14 +-- .../src/deriving/generic/mod.rs | 114 ++++++++++++++------- compiler/rustc_builtin_macros/src/deriving/hash.rs | 12 ++- 11 files changed, 146 insertions(+), 111 deletions(-) (limited to 'compiler') diff --git a/compiler/rustc_builtin_macros/src/deriving/clone.rs b/compiler/rustc_builtin_macros/src/deriving/clone.rs index 1c507678489..b37b82dd089 100644 --- a/compiler/rustc_builtin_macros/src/deriving/clone.rs +++ b/compiler/rustc_builtin_macros/src/deriving/clone.rs @@ -2,8 +2,7 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use crate::deriving::path_std; -use rustc_ast::ptr::P; -use rustc_ast::{self as ast, Expr, Generics, ItemKind, MetaItem, VariantData}; +use rustc_ast::{self as ast, Generics, ItemKind, MetaItem, VariantData}; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::Span; @@ -98,7 +97,7 @@ fn cs_clone_simple( trait_span: Span, substr: &Substructure<'_>, is_union: bool, -) -> P { +) -> BlockOrExpr { let mut stmts = Vec::new(); let mut process_variant = |variant: &VariantData| { for field in variant.fields() { @@ -139,8 +138,7 @@ fn cs_clone_simple( ), } } - stmts.push(cx.stmt_expr(cx.expr_deref(trait_span, cx.expr_self(trait_span)))); - cx.expr_block(cx.block(trait_span, stmts)) + BlockOrExpr::new_mixed(stmts, cx.expr_deref(trait_span, cx.expr_self(trait_span))) } fn cs_clone( @@ -148,7 +146,7 @@ fn cs_clone( cx: &mut ExtCtxt<'_>, trait_span: Span, substr: &Substructure<'_>, -) -> P { +) -> BlockOrExpr { let ctor_path; let all_fields; let fn_path = cx.std_path(&[sym::clone, sym::Clone, sym::clone]); @@ -177,7 +175,7 @@ fn cs_clone( } } - match *vdata { + let expr = match *vdata { VariantData::Struct(..) => { let fields = all_fields .iter() @@ -201,5 +199,6 @@ fn cs_clone( cx.expr_call(trait_span, path, subcalls) } VariantData::Unit(..) => cx.expr_path(ctor_path), - } + }; + BlockOrExpr::new_expr(expr) } diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs index cb2ad283a19..59f8fbc4a08 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs @@ -2,8 +2,7 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use crate::deriving::path_std; -use rustc_ast::ptr::P; -use rustc_ast::{self as ast, Expr, MetaItem}; +use rustc_ast::{self as ast, MetaItem}; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{sym, Ident}; use rustc_span::Span; @@ -52,7 +51,7 @@ fn cs_total_eq_assert( cx: &mut ExtCtxt<'_>, trait_span: Span, substr: &Substructure<'_>, -) -> P { +) -> BlockOrExpr { let mut stmts = Vec::new(); let mut process_variant = |variant: &ast::VariantData| { for field in variant.fields() { @@ -78,5 +77,5 @@ fn cs_total_eq_assert( } _ => cx.span_bug(trait_span, "unexpected substructure in `derive(Eq)`"), } - cx.expr_block(cx.block(trait_span, stmts)) + BlockOrExpr::new_stmts(stmts) } diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs index c7850cd4b4c..4795b72a780 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs @@ -3,7 +3,7 @@ use crate::deriving::generic::*; use crate::deriving::path_std; use rustc_ast::ptr::P; -use rustc_ast::{self as ast, Expr, MetaItem}; +use rustc_ast::{self as ast, MetaItem}; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{sym, Ident}; use rustc_span::Span; @@ -51,7 +51,7 @@ pub fn ordering_collapsed( cx.expr_call_global(span, fn_cmp_path, vec![lft, rgt]) } -pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P { +pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr { let test_id = Ident::new(sym::cmp, span); let equals_path = cx.path_global(span, cx.std_path(&[sym::cmp, sym::Ordering, sym::Equal])); @@ -70,7 +70,7 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P< // cmp => cmp // } // - cs_fold( + let expr = cs_fold( // foldr nests the if-elses correctly, leaving the first field // as the outermost one, and the last as the innermost. false, @@ -107,5 +107,6 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P< cx, span, substr, - ) + ); + BlockOrExpr::new_expr(expr) } 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 ca5ca29eb82..f58e303b8f0 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs @@ -15,8 +15,6 @@ pub fn expand_deriving_partial_eq( item: &Annotatable, push: &mut dyn FnMut(Annotatable), ) { - // structures are equal if all fields are equal, and non equal, if - // any fields are not equal or if the enum variants are different fn cs_op( cx: &mut ExtCtxt<'_>, span: Span, @@ -24,7 +22,7 @@ pub fn expand_deriving_partial_eq( op: BinOpKind, combiner: BinOpKind, base: bool, - ) -> P { + ) -> BlockOrExpr { let op = |cx: &mut ExtCtxt<'_>, span: Span, self_f: P, other_fs: &[P]| { let [other_f] = other_fs else { cx.span_bug(span, "not exactly 2 arguments in `derive(PartialEq)`"); @@ -33,7 +31,7 @@ pub fn expand_deriving_partial_eq( cx.expr_binary(span, op, self_f, other_f.clone()) }; - cs_fold1( + let expr = cs_fold1( true, // use foldl |cx, span, subexpr, self_f, other_fs| { let eq = op(cx, span, self_f, other_fs); @@ -52,13 +50,14 @@ pub fn expand_deriving_partial_eq( cx, span, substr, - ) + ); + BlockOrExpr::new_expr(expr) } - fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P { + fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr { cs_op(cx, span, substr, BinOpKind::Eq, BinOpKind::And, true) } - fn cs_ne(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P { + fn cs_ne(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr { cs_op(cx, span, substr, BinOpKind::Ne, BinOpKind::Or, false) } 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 07db82fee93..891485ab8ff 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs @@ -2,8 +2,7 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use crate::deriving::{path_std, pathvec_std}; -use rustc_ast::ptr::P; -use rustc_ast::{Expr, MetaItem}; +use rustc_ast::MetaItem; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{sym, Ident}; use rustc_span::Span; @@ -48,7 +47,7 @@ pub fn expand_deriving_partial_ord( trait_def.expand(cx, mitem, item, push) } -pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P { +pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr { let test_id = Ident::new(sym::cmp, span); let ordering = cx.path_global(span, cx.std_path(&[sym::cmp, sym::Ordering, sym::Equal])); let ordering_expr = cx.expr_path(ordering.clone()); @@ -69,7 +68,7 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_ // cmp => cmp // } // - cs_fold( + let expr = cs_fold( // foldr nests the if-elses correctly, leaving the first field // as the outermost one, and the last as the innermost. false, @@ -110,5 +109,6 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_ cx, span, substr, - ) + ); + BlockOrExpr::new_expr(expr) } diff --git a/compiler/rustc_builtin_macros/src/deriving/debug.rs b/compiler/rustc_builtin_macros/src/deriving/debug.rs index 1411c60c0bf..e0489828760 100644 --- a/compiler/rustc_builtin_macros/src/deriving/debug.rs +++ b/compiler/rustc_builtin_macros/src/deriving/debug.rs @@ -2,8 +2,7 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use crate::deriving::path_std; -use rustc_ast::ptr::P; -use rustc_ast::{self as ast, Expr, MetaItem}; +use rustc_ast::{self as ast, MetaItem}; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_span::Span; @@ -42,7 +41,7 @@ pub fn expand_deriving_debug( trait_def.expand(cx, mitem, item, push) } -fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P { +fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr { let (ident, vdata, fields) = match substr.fields { Struct(vdata, fields) => (substr.type_ident, *vdata, fields), EnumMatching(_, _, v, fields) => (v.ident, &v.data, fields), @@ -74,7 +73,8 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_> if fields.is_empty() { // Special case for no fields. let fn_path_write_str = cx.std_path(&[sym::fmt, sym::Formatter, sym::write_str]); - cx.expr_call_global(span, fn_path_write_str, vec![fmt, name]) + let expr = cx.expr_call_global(span, fn_path_write_str, vec![fmt, name]); + BlockOrExpr::new_expr(expr) } else if fields.len() <= CUTOFF { // Few enough fields that we can use a specific-length method. let debug = if is_struct { @@ -100,7 +100,8 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_> let field = cx.expr_addr_of(field.span, field); args.push(field); } - cx.expr_call_global(span, fn_path_debug, args) + let expr = cx.expr_call_global(span, fn_path_debug, args); + BlockOrExpr::new_expr(expr) } else { // Enough fields that we must use the any-length method. let mut name_exprs = Vec::with_capacity(fields.len()); @@ -176,8 +177,6 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_> stmts.push(names_let.unwrap()); } stmts.push(values_let); - stmts.push(cx.stmt_expr(expr)); - - cx.expr_block(cx.block(span, stmts)) + BlockOrExpr::new_mixed(stmts, expr) } } diff --git a/compiler/rustc_builtin_macros/src/deriving/decodable.rs b/compiler/rustc_builtin_macros/src/deriving/decodable.rs index 16154fb4d03..b9f2a750822 100644 --- a/compiler/rustc_builtin_macros/src/deriving/decodable.rs +++ b/compiler/rustc_builtin_macros/src/deriving/decodable.rs @@ -62,7 +62,7 @@ fn decodable_substructure( trait_span: Span, substr: &Substructure<'_>, krate: Symbol, -) -> P { +) -> BlockOrExpr { let decoder = substr.nonself_args[0].clone(); let recurse = vec![ Ident::new(krate, trait_span), @@ -74,7 +74,7 @@ fn decodable_substructure( let blkarg = Ident::new(sym::_d, trait_span); let blkdecoder = cx.expr_ident(trait_span, blkarg); - match *substr.fields { + let expr = match *substr.fields { StaticStruct(_, ref summary) => { let nfields = match *summary { Unnamed(ref fields, _) => fields.len(), @@ -173,7 +173,8 @@ fn decodable_substructure( ) } _ => cx.bug("expected StaticEnum or StaticStruct in derive(Decodable)"), - } + }; + BlockOrExpr::new_expr(expr) } /// Creates a decoder for a single enum variant/struct: diff --git a/compiler/rustc_builtin_macros/src/deriving/default.rs b/compiler/rustc_builtin_macros/src/deriving/default.rs index d41b25343b0..90d5cdbc0a0 100644 --- a/compiler/rustc_builtin_macros/src/deriving/default.rs +++ b/compiler/rustc_builtin_macros/src/deriving/default.rs @@ -1,11 +1,10 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; -use rustc_ast::ptr::P; +use rustc_ast as ast; use rustc_ast::walk_list; use rustc_ast::EnumDef; use rustc_ast::VariantData; -use rustc_ast::{Expr, MetaItem}; use rustc_errors::Applicability; use rustc_expand::base::{Annotatable, DummyResult, ExtCtxt}; use rustc_span::symbol::Ident; @@ -16,7 +15,7 @@ use smallvec::SmallVec; pub fn expand_deriving_default( cx: &mut ExtCtxt<'_>, span: Span, - mitem: &MetaItem, + mitem: &ast::MetaItem, item: &Annotatable, push: &mut dyn FnMut(Annotatable), ) { @@ -59,12 +58,12 @@ fn default_struct_substructure( trait_span: Span, substr: &Substructure<'_>, summary: &StaticFields, -) -> P { +) -> BlockOrExpr { // Note that `kw::Default` is "default" and `sym::Default` is "Default"! let default_ident = cx.std_path(&[kw::Default, sym::Default, kw::Default]); let default_call = |span| cx.expr_call_global(span, default_ident.clone(), Vec::new()); - match summary { + let expr = match summary { Unnamed(ref fields, is_tuple) => { if !is_tuple { cx.expr_ident(trait_span, substr.type_ident) @@ -80,31 +79,27 @@ fn default_struct_substructure( .collect(); cx.expr_struct_ident(trait_span, substr.type_ident, default_fields) } - } + }; + BlockOrExpr::new_expr(expr) } fn default_enum_substructure( cx: &mut ExtCtxt<'_>, trait_span: Span, enum_def: &EnumDef, -) -> P { - let Ok(default_variant) = extract_default_variant(cx, enum_def, trait_span) else { - return DummyResult::raw_expr(trait_span, true); +) -> BlockOrExpr { + let expr = if let Ok(default_variant) = extract_default_variant(cx, enum_def, trait_span) + && let Ok(_) = validate_default_attribute(cx, default_variant) + { + // We now know there is exactly one unit variant with exactly one `#[default]` attribute. + cx.expr_path(cx.path( + default_variant.span, + vec![Ident::new(kw::SelfUpper, default_variant.span), default_variant.ident], + )) + } else { + DummyResult::raw_expr(trait_span, true) }; - - // At this point, we know that there is exactly one variant with a `#[default]` attribute. The - // attribute hasn't yet been validated. - - if let Err(()) = validate_default_attribute(cx, default_variant) { - return DummyResult::raw_expr(trait_span, true); - } - - // We now know there is exactly one unit variant with exactly one `#[default]` attribute. - - cx.expr_path(cx.path( - default_variant.span, - vec![Ident::new(kw::SelfUpper, default_variant.span), default_variant.ident], - )) + BlockOrExpr::new_expr(expr) } fn extract_default_variant<'a>( diff --git a/compiler/rustc_builtin_macros/src/deriving/encodable.rs b/compiler/rustc_builtin_macros/src/deriving/encodable.rs index 7dc0584618d..0dfce114bfc 100644 --- a/compiler/rustc_builtin_macros/src/deriving/encodable.rs +++ b/compiler/rustc_builtin_macros/src/deriving/encodable.rs @@ -89,8 +89,7 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use crate::deriving::pathvec_std; -use rustc_ast::ptr::P; -use rustc_ast::{Expr, ExprKind, MetaItem, Mutability}; +use rustc_ast::{ExprKind, MetaItem, Mutability}; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_span::Span; @@ -147,7 +146,7 @@ fn encodable_substructure( trait_span: Span, substr: &Substructure<'_>, krate: Symbol, -) -> P { +) -> BlockOrExpr { let encoder = substr.nonself_args[0].clone(); // throw an underscore in front to suppress unused variable warnings let blkarg = Ident::new(sym::_e, trait_span); @@ -208,7 +207,7 @@ fn encodable_substructure( let fn_emit_struct_path = cx.def_site_path(&[sym::rustc_serialize, sym::Encoder, sym::emit_struct]); - cx.expr_call_global( + let expr = cx.expr_call_global( trait_span, fn_emit_struct_path, vec![ @@ -217,7 +216,8 @@ fn encodable_substructure( cx.expr_usize(trait_span, fields.len()), blk, ], - ) + ); + BlockOrExpr::new_expr(expr) } EnumMatching(idx, _, variant, ref fields) => { @@ -279,12 +279,12 @@ fn encodable_substructure( let blk = cx.lambda1(trait_span, call, blkarg); let fn_emit_enum_path: Vec<_> = cx.def_site_path(&[sym::rustc_serialize, sym::Encoder, sym::emit_enum]); - let ret = cx.expr_call_global( + let expr = cx.expr_call_global( trait_span, fn_emit_enum_path, vec![encoder, cx.expr_str(trait_span, substr.type_ident.name), blk], ); - cx.expr_block(cx.block(trait_span, vec![me, cx.stmt_expr(ret)])) + BlockOrExpr::new_mixed(vec![me], expr) } _ => cx.bug("expected Struct or EnumMatching in derive(Encodable)"), diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 2a9e37081e0..e2d82b181a1 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -296,7 +296,7 @@ pub enum SubstructureFields<'a> { /// Combine the values of all the fields together. The last argument is /// all the fields of all the structures. pub type CombineSubstructureFunc<'a> = - Box, Span, &Substructure<'_>) -> P + 'a>; + Box, Span, &Substructure<'_>) -> BlockOrExpr + 'a>; /// Deal with non-matching enum variants. The slice is the identifiers holding /// the variant index value for each of the `Self` arguments. @@ -314,6 +314,48 @@ struct TypeParameter { ty: P, } +// The code snippets built up for derived code are sometimes used as blocks +// (e.g. in a function body) and sometimes used as expressions (e.g. in a match +// arm). This structure avoids committing to either form until necessary, +// avoiding the insertion of any unnecessary blocks. +// +// The statements come before the expression. +pub struct BlockOrExpr(Vec, Option>); + +impl BlockOrExpr { + pub fn new_stmts(stmts: Vec) -> BlockOrExpr { + BlockOrExpr(stmts, None) + } + + pub fn new_expr(expr: P) -> BlockOrExpr { + BlockOrExpr(vec![], Some(expr)) + } + + pub fn new_mixed(stmts: Vec, expr: P) -> BlockOrExpr { + BlockOrExpr(stmts, Some(expr)) + } + + // Converts it into a block. + fn into_block(mut self, cx: &ExtCtxt<'_>, span: Span) -> P { + if let Some(expr) = self.1 { + self.0.push(cx.stmt_expr(expr)); + } + cx.block(span, self.0) + } + + // Converts it into an expression. + fn into_expr(self, cx: &ExtCtxt<'_>, span: Span) -> P { + if self.0.is_empty() { + match self.1 { + None => cx.expr_block(cx.block(span, vec![])), + Some(expr) => expr, + } + } else { + cx.expr_block(self.into_block(cx, span)) + } + } +} + /// This method helps to extract all the type parameters referenced from a /// type. For a type parameter ``, it looks for either a `TyPath` that /// is not global and starts with `T`, or a `TyQPath`. @@ -827,7 +869,7 @@ impl<'a> MethodDef<'a> { type_ident: Ident, nonself_args: &[P], fields: &SubstructureFields<'_>, - ) -> P { + ) -> BlockOrExpr { let span = trait_.span; let substructure = Substructure { type_ident, nonself_args, fields }; let mut f = self.combine_substructure.borrow_mut(); @@ -902,7 +944,7 @@ impl<'a> MethodDef<'a> { generics: &Generics, explicit_self: Option, arg_types: Vec<(Ident, P)>, - body: P, + body: BlockOrExpr, ) -> P { let span = trait_.span; // Create the generics that aren't for `Self`. @@ -921,7 +963,7 @@ impl<'a> MethodDef<'a> { let method_ident = Ident::new(self.name, span); let fn_decl = cx.fn_decl(args, ast::FnRetTy::Ty(ret_type)); - let body_block = cx.block_expr(body); + let body_block = body.into_block(cx, span); let trait_lo_sp = span.shrink_to_lo(); @@ -986,7 +1028,7 @@ impl<'a> MethodDef<'a> { nonself_args: &[P], use_temporaries: bool, is_packed: bool, - ) -> P { + ) -> BlockOrExpr { let mut raw_fields = Vec::new(); // Vec<[fields of self], [fields of next Self arg], [etc]> let span = trait_.span; let mut patterns = Vec::new(); @@ -1047,16 +1089,14 @@ impl<'a> MethodDef<'a> { ); if !is_packed { - body.span = span; body } else { // Do the let-destructuring. let mut stmts: Vec<_> = iter::zip(self_args, patterns) .map(|(arg_expr, pat)| cx.stmt_let_pat(span, pat, arg_expr.clone())) .collect(); - stmts.push(cx.stmt_expr(body)); - - cx.expr_block(cx.block(span, stmts)) + stmts.extend(std::mem::take(&mut body.0)); + BlockOrExpr(stmts, body.1) } } @@ -1067,7 +1107,7 @@ impl<'a> MethodDef<'a> { struct_def: &VariantData, type_ident: Ident, nonself_args: &[P], - ) -> P { + ) -> BlockOrExpr { let summary = trait_.summarise_struct(cx, struct_def); self.call_substructure_method( @@ -1130,7 +1170,7 @@ impl<'a> MethodDef<'a> { type_ident: Ident, mut self_args: Vec>, nonself_args: &[P], - ) -> P { + ) -> BlockOrExpr { let span = trait_.span; let variants = &enum_def.variants; @@ -1253,13 +1293,9 @@ impl<'a> MethodDef<'a> { // Self arg, assuming all are instances of VariantK. // Build up code associated with such a case. let substructure = EnumMatching(index, variants.len(), variant, field_tuples); - let arm_expr = self.call_substructure_method( - cx, - trait_, - type_ident, - nonself_args, - &substructure, - ); + let arm_expr = self + .call_substructure_method(cx, trait_, type_ident, nonself_args, &substructure) + .into_expr(cx, span); cx.arm(span, single_pat, arm_expr) }) @@ -1271,13 +1307,16 @@ impl<'a> MethodDef<'a> { // The index and actual variant aren't meaningful in this case, // so just use whatever let substructure = EnumMatching(0, variants.len(), v, Vec::new()); - Some(self.call_substructure_method( - cx, - trait_, - type_ident, - nonself_args, - &substructure, - )) + Some( + self.call_substructure_method( + cx, + trait_, + type_ident, + nonself_args, + &substructure, + ) + .into_expr(cx, span), + ) } _ if variants.len() > 1 && self_args.len() > 1 => { // Since we know that all the arguments will match if we reach @@ -1341,13 +1380,15 @@ impl<'a> MethodDef<'a> { } } - let arm_expr = self.call_substructure_method( - cx, - trait_, - type_ident, - nonself_args, - &catch_all_substructure, - ); + let arm_expr = self + .call_substructure_method( + cx, + trait_, + type_ident, + nonself_args, + &catch_all_substructure, + ) + .into_expr(cx, span); // Final wrinkle: the self_args are expressions that deref // down to desired places, but we cannot actually deref @@ -1371,8 +1412,7 @@ impl<'a> MethodDef<'a> { // } let all_match = cx.expr_match(span, match_arg, match_arms); let arm_expr = cx.expr_if(span, discriminant_test, all_match, Some(arm_expr)); - index_let_stmts.push(cx.stmt_expr(arm_expr)); - cx.expr_block(cx.block(span, index_let_stmts)) + BlockOrExpr(index_let_stmts, Some(arm_expr)) } else if variants.is_empty() { // As an additional wrinkle, For a zero-variant enum A, // currently the compiler @@ -1423,7 +1463,7 @@ impl<'a> MethodDef<'a> { // derive Debug on such a type could here generate code // that needs the feature gate enabled.) - deriving::call_unreachable(cx, span) + BlockOrExpr(vec![], Some(deriving::call_unreachable(cx, span))) } else { // Final wrinkle: the self_args are expressions that deref // down to desired places, but we cannot actually deref @@ -1432,7 +1472,7 @@ impl<'a> MethodDef<'a> { // `(*self, *__arg_0, ...)` into `(&*self, &*__arg_0, ...)`. self_args.map_in_place(|self_arg| cx.expr_addr_of(span, self_arg)); let match_arg = cx.expr(span, ast::ExprKind::Tup(self_args)); - cx.expr_match(span, match_arg, match_arms) + BlockOrExpr(vec![], Some(cx.expr_match(span, match_arg, match_arms))) } } @@ -1443,7 +1483,7 @@ impl<'a> MethodDef<'a> { enum_def: &EnumDef, type_ident: Ident, nonself_args: &[P], - ) -> P { + ) -> BlockOrExpr { let summary = enum_def .variants .iter() diff --git a/compiler/rustc_builtin_macros/src/deriving/hash.rs b/compiler/rustc_builtin_macros/src/deriving/hash.rs index 9790449c4b3..c3f7d09886b 100644 --- a/compiler/rustc_builtin_macros/src/deriving/hash.rs +++ b/compiler/rustc_builtin_macros/src/deriving/hash.rs @@ -2,8 +2,7 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use crate::deriving::{self, path_std, pathvec_std}; -use rustc_ast::ptr::P; -use rustc_ast::{Expr, MetaItem, Mutability}; +use rustc_ast::{MetaItem, Mutability}; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::sym; use rustc_span::Span; @@ -45,7 +44,11 @@ pub fn expand_deriving_hash( hash_trait_def.expand(cx, mitem, item, push); } -fn hash_substructure(cx: &mut ExtCtxt<'_>, trait_span: Span, substr: &Substructure<'_>) -> P { +fn hash_substructure( + cx: &mut ExtCtxt<'_>, + trait_span: Span, + substr: &Substructure<'_>, +) -> BlockOrExpr { let [state_expr] = substr.nonself_args else { cx.span_bug(trait_span, "incorrect number of arguments in `derive(Hash)`"); }; @@ -81,6 +84,5 @@ fn hash_substructure(cx: &mut ExtCtxt<'_>, trait_span: Span, substr: &Substructu stmts.extend( fields.iter().map(|FieldInfo { ref self_, span, .. }| call_hash(*span, self_.clone())), ); - - cx.expr_block(cx.block(trait_span, stmts)) + BlockOrExpr::new_stmts(stmts) } -- cgit 1.4.1-3-g733a5 From a7b1d31a9f358747221e8eb9986bb8303d5d6586 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 1 Jul 2022 16:32:20 +1000 Subject: Don't repeat `AssertParamIs{Clone,Eq}` assertions. It's common to see repeated assertions like this in derived `clone` and `eq` methods: ``` let _: ::core::clone::AssertParamIsClone; let _: ::core::clone::AssertParamIsClone; ``` This commit avoids them. --- compiler/rustc_ast/src/ast.rs | 8 +++++++ .../rustc_builtin_macros/src/deriving/clone.rs | 26 +++++++++++++++------- .../rustc_builtin_macros/src/deriving/cmp/eq.rs | 25 ++++++++++++++------- src/test/ui/deriving/deriving-all-codegen.stdout | 13 ----------- 4 files changed, 43 insertions(+), 29 deletions(-) (limited to 'compiler') diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index e5b61d7000a..bb0b8121cac 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2036,6 +2036,14 @@ impl TyKind { pub fn is_unit(&self) -> bool { matches!(self, TyKind::Tup(tys) if tys.is_empty()) } + + pub fn is_simple_path(&self) -> Option { + if let TyKind::Path(None, Path { segments, .. }) = &self && segments.len() == 1 { + Some(segments[0].ident.name) + } else { + None + } + } } /// Syntax used to declare a trait object. diff --git a/compiler/rustc_builtin_macros/src/deriving/clone.rs b/compiler/rustc_builtin_macros/src/deriving/clone.rs index b37b82dd089..0a55d4e0ce0 100644 --- a/compiler/rustc_builtin_macros/src/deriving/clone.rs +++ b/compiler/rustc_builtin_macros/src/deriving/clone.rs @@ -3,6 +3,7 @@ use crate::deriving::generic::*; use crate::deriving::path_std; use rustc_ast::{self as ast, Generics, ItemKind, MetaItem, VariantData}; +use rustc_data_structures::fx::FxHashSet; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::Span; @@ -99,20 +100,29 @@ fn cs_clone_simple( is_union: bool, ) -> BlockOrExpr { let mut stmts = Vec::new(); + let mut seen_type_names = FxHashSet::default(); let mut process_variant = |variant: &VariantData| { for field in variant.fields() { - // let _: AssertParamIsClone; - super::assert_ty_bounds( - cx, - &mut stmts, - field.ty.clone(), - field.span, - &[sym::clone, sym::AssertParamIsClone], - ); + // This basic redundancy checking only prevents duplication of + // assertions like `AssertParamIsClone` where the type is a + // simple name. That's enough to get a lot of cases, though. + if let Some(name) = field.ty.kind.is_simple_path() && !seen_type_names.insert(name) { + // Already produced an assertion for this type. + } else { + // let _: AssertParamIsClone; + super::assert_ty_bounds( + cx, + &mut stmts, + field.ty.clone(), + field.span, + &[sym::clone, sym::AssertParamIsClone], + ); + } } }; if is_union { + // Just a single assertion for unions, that the union impls `Copy`. // let _: AssertParamIsCopy; let self_ty = cx.ty_path(cx.path_ident(trait_span, Ident::with_dummy_span(kw::SelfUpper))); super::assert_ty_bounds( diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs index 59f8fbc4a08..c1a2ebcc146 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs @@ -3,6 +3,7 @@ use crate::deriving::generic::*; use crate::deriving::path_std; use rustc_ast::{self as ast, MetaItem}; +use rustc_data_structures::fx::FxHashSet; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{sym, Ident}; use rustc_span::Span; @@ -53,16 +54,24 @@ fn cs_total_eq_assert( substr: &Substructure<'_>, ) -> BlockOrExpr { let mut stmts = Vec::new(); + let mut seen_type_names = FxHashSet::default(); let mut process_variant = |variant: &ast::VariantData| { for field in variant.fields() { - // let _: AssertParamIsEq; - super::assert_ty_bounds( - cx, - &mut stmts, - field.ty.clone(), - field.span, - &[sym::cmp, sym::AssertParamIsEq], - ); + // This basic redundancy checking only prevents duplication of + // assertions like `AssertParamIsEq` where the type is a + // simple name. That's enough to get a lot of cases, though. + if let Some(name) = field.ty.kind.is_simple_path() && !seen_type_names.insert(name) { + // Already produced an assertion for this type. + } else { + // let _: AssertParamIsEq; + super::assert_ty_bounds( + cx, + &mut stmts, + field.ty.clone(), + field.span, + &[sym::cmp, sym::AssertParamIsEq], + ); + } } }; diff --git a/src/test/ui/deriving/deriving-all-codegen.stdout b/src/test/ui/deriving/deriving-all-codegen.stdout index fb5965858cc..7713627be5e 100644 --- a/src/test/ui/deriving/deriving-all-codegen.stdout +++ b/src/test/ui/deriving/deriving-all-codegen.stdout @@ -95,7 +95,6 @@ struct Point { impl ::core::clone::Clone for Point { #[inline] fn clone(&self) -> Point { - let _: ::core::clone::AssertParamIsClone; let _: ::core::clone::AssertParamIsClone; *self } @@ -152,7 +151,6 @@ impl ::core::cmp::Eq for Point { #[no_coverage] fn assert_receiver_is_total_eq(&self) -> () { let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; } } #[automatically_derived] @@ -291,13 +289,6 @@ impl ::core::cmp::Eq for Big { #[no_coverage] fn assert_receiver_is_total_eq(&self) -> () { let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; } } #[automatically_derived] @@ -773,8 +764,6 @@ enum Mixed { impl ::core::clone::Clone for Mixed { #[inline] fn clone(&self) -> Mixed { - let _: ::core::clone::AssertParamIsClone; - let _: ::core::clone::AssertParamIsClone; let _: ::core::clone::AssertParamIsClone; *self } @@ -871,8 +860,6 @@ impl ::core::cmp::Eq for Mixed { #[no_coverage] fn assert_receiver_is_total_eq(&self) -> () { let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; - let _: ::core::cmp::AssertParamIsEq; } } #[automatically_derived] -- cgit 1.4.1-3-g733a5 From 2c911dc16f2ac3237fca7c7626982e6f6a4d0e33 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 4 Jul 2022 15:08:28 +1000 Subject: Avoid unnecessary 1-tuples in derived code. --- .../src/deriving/generic/mod.rs | 12 ++++- src/test/codegen/consts.rs | 2 +- src/test/ui/deriving/deriving-all-codegen.stdout | 62 +++++++++++----------- 3 files changed, 42 insertions(+), 34 deletions(-) (limited to 'compiler') diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index e2d82b181a1..f3e1176cab3 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -1239,7 +1239,11 @@ impl<'a> MethodDef<'a> { } // Here is the pat = `(&VariantK, &VariantK, ...)` - let single_pat = cx.pat_tuple(span, subpats); + let single_pat = if subpats.len() == 1 { + subpats.pop().unwrap() + } else { + cx.pat_tuple(span, subpats) + }; // For the BodyK, we need to delegate to our caller, // passing it an EnumMatching to indicate which case @@ -1471,7 +1475,11 @@ impl<'a> MethodDef<'a> { // expression; here add a layer of borrowing, turning // `(*self, *__arg_0, ...)` into `(&*self, &*__arg_0, ...)`. self_args.map_in_place(|self_arg| cx.expr_addr_of(span, self_arg)); - let match_arg = cx.expr(span, ast::ExprKind::Tup(self_args)); + let match_arg = if self_args.len() == 1 { + self_args.pop().unwrap() + } else { + cx.expr(span, ast::ExprKind::Tup(self_args)) + }; BlockOrExpr(vec![], Some(cx.expr_match(span, match_arg, match_arms))) } } diff --git a/src/test/codegen/consts.rs b/src/test/codegen/consts.rs index c97223879ca..260d9de8670 100644 --- a/src/test/codegen/consts.rs +++ b/src/test/codegen/consts.rs @@ -10,7 +10,7 @@ // CHECK: @STATIC = {{.*}}, align 4 // This checks the constants from inline_enum_const -// CHECK: @alloc14 = {{.*}}, align 2 +// CHECK: @alloc12 = {{.*}}, align 2 // This checks the constants from {low,high}_align_const, they share the same // constant, but the alignment differs, so the higher one should be used diff --git a/src/test/ui/deriving/deriving-all-codegen.stdout b/src/test/ui/deriving/deriving-all-codegen.stdout index 7713627be5e..763b39a212a 100644 --- a/src/test/ui/deriving/deriving-all-codegen.stdout +++ b/src/test/ui/deriving/deriving-all-codegen.stdout @@ -554,8 +554,8 @@ enum Enum1 { impl ::core::clone::Clone for Enum1 { #[inline] fn clone(&self) -> Enum1 { - match (&*self,) { - (&Enum1::Single { x: ref __self_0 },) => + match &*self { + &Enum1::Single { x: ref __self_0 } => Enum1::Single { x: ::core::clone::Clone::clone(&*__self_0) }, } } @@ -564,8 +564,8 @@ impl ::core::clone::Clone for Enum1 { #[allow(unused_qualifications)] impl ::core::fmt::Debug for Enum1 { fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { - match (&*self,) { - (&Enum1::Single { x: ref __self_0 },) => + match &*self { + &Enum1::Single { x: ref __self_0 } => ::core::fmt::Formatter::debug_struct_field1_finish(f, "Single", "x", &&*__self_0), } @@ -575,8 +575,8 @@ impl ::core::fmt::Debug for Enum1 { #[allow(unused_qualifications)] impl ::core::hash::Hash for Enum1 { fn hash<__H: ::core::hash::Hasher>(&self, state: &mut __H) -> () { - match (&*self,) { - (&Enum1::Single { x: ref __self_0 },) => { + match &*self { + &Enum1::Single { x: ref __self_0 } => { ::core::hash::Hash::hash(&*__self_0, state) } } @@ -669,10 +669,10 @@ impl ::core::marker::Copy for Fieldless { } #[allow(unused_qualifications)] 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"), + 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"), } } } @@ -686,7 +686,7 @@ impl ::core::default::Default for Fieldless { #[allow(unused_qualifications)] impl ::core::hash::Hash for Fieldless { fn hash<__H: ::core::hash::Hasher>(&self, state: &mut __H) -> () { - match (&*self,) { + match &*self { _ => { ::core::hash::Hash::hash(&::core::intrinsics::discriminant_value(self), state) @@ -775,13 +775,13 @@ impl ::core::marker::Copy for Mixed { } #[allow(unused_qualifications)] impl ::core::fmt::Debug for Mixed { fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { - match (&*self,) { - (&Mixed::P,) => ::core::fmt::Formatter::write_str(f, "P"), - (&Mixed::Q,) => ::core::fmt::Formatter::write_str(f, "Q"), - (&Mixed::R(ref __self_0),) => + match &*self { + &Mixed::P => ::core::fmt::Formatter::write_str(f, "P"), + &Mixed::Q => ::core::fmt::Formatter::write_str(f, "Q"), + &Mixed::R(ref __self_0) => ::core::fmt::Formatter::debug_tuple_field1_finish(f, "R", &&*__self_0), - (&Mixed::S { d1: ref __self_0, d2: ref __self_1 },) => + &Mixed::S { d1: ref __self_0, d2: ref __self_1 } => ::core::fmt::Formatter::debug_struct_field2_finish(f, "S", "d1", &&*__self_0, "d2", &&*__self_1), } @@ -797,13 +797,13 @@ impl ::core::default::Default for Mixed { #[allow(unused_qualifications)] impl ::core::hash::Hash for Mixed { fn hash<__H: ::core::hash::Hasher>(&self, state: &mut __H) -> () { - match (&*self,) { - (&Mixed::R(ref __self_0),) => { + match &*self { + &Mixed::R(ref __self_0) => { ::core::hash::Hash::hash(&::core::intrinsics::discriminant_value(self), state); ::core::hash::Hash::hash(&*__self_0, state) } - (&Mixed::S { d1: ref __self_0, d2: ref __self_1 },) => { + &Mixed::S { d1: ref __self_0, d2: ref __self_1 } => { ::core::hash::Hash::hash(&::core::intrinsics::discriminant_value(self), state); ::core::hash::Hash::hash(&*__self_0, state); @@ -943,12 +943,12 @@ enum Fielded { X(u32), Y(bool), Z(Option), } impl ::core::clone::Clone for Fielded { #[inline] fn clone(&self) -> Fielded { - match (&*self,) { - (&Fielded::X(ref __self_0),) => + match &*self { + &Fielded::X(ref __self_0) => Fielded::X(::core::clone::Clone::clone(&*__self_0)), - (&Fielded::Y(ref __self_0),) => + &Fielded::Y(ref __self_0) => Fielded::Y(::core::clone::Clone::clone(&*__self_0)), - (&Fielded::Z(ref __self_0),) => + &Fielded::Z(ref __self_0) => Fielded::Z(::core::clone::Clone::clone(&*__self_0)), } } @@ -957,14 +957,14 @@ impl ::core::clone::Clone for Fielded { #[allow(unused_qualifications)] impl ::core::fmt::Debug for Fielded { fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { - match (&*self,) { - (&Fielded::X(ref __self_0),) => + match &*self { + &Fielded::X(ref __self_0) => ::core::fmt::Formatter::debug_tuple_field1_finish(f, "X", &&*__self_0), - (&Fielded::Y(ref __self_0),) => + &Fielded::Y(ref __self_0) => ::core::fmt::Formatter::debug_tuple_field1_finish(f, "Y", &&*__self_0), - (&Fielded::Z(ref __self_0),) => + &Fielded::Z(ref __self_0) => ::core::fmt::Formatter::debug_tuple_field1_finish(f, "Z", &&*__self_0), } @@ -974,18 +974,18 @@ impl ::core::fmt::Debug for Fielded { #[allow(unused_qualifications)] impl ::core::hash::Hash for Fielded { fn hash<__H: ::core::hash::Hasher>(&self, state: &mut __H) -> () { - match (&*self,) { - (&Fielded::X(ref __self_0),) => { + match &*self { + &Fielded::X(ref __self_0) => { ::core::hash::Hash::hash(&::core::intrinsics::discriminant_value(self), state); ::core::hash::Hash::hash(&*__self_0, state) } - (&Fielded::Y(ref __self_0),) => { + &Fielded::Y(ref __self_0) => { ::core::hash::Hash::hash(&::core::intrinsics::discriminant_value(self), state); ::core::hash::Hash::hash(&*__self_0, state) } - (&Fielded::Z(ref __self_0),) => { + &Fielded::Z(ref __self_0) => { ::core::hash::Hash::hash(&::core::intrinsics::discriminant_value(self), state); ::core::hash::Hash::hash(&*__self_0, state) -- cgit 1.4.1-3-g733a5 From 0ee79f2c5a13cae0ffaa43a21b7f11d1d7ad5316 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 1 Jul 2022 21:05:01 +1000 Subject: Avoid the unnecessary innermost match in `partial_cmp`/`cmp`. We currently do a match on the comparison of every field in a struct or enum variant. But the last field has a degenerate match like this: ``` match ::core::cmp::Ord::cmp(&self.y, &other.y) { ::core::cmp::Ordering::Equal => ::core::cmp::Ordering::Equal, cmp => cmp, }, ``` This commit changes it to this: ``` ::core::cmp::Ord::cmp(&self.y, &other.y), ``` This is fairly straightforward thanks to the existing `cs_fold1` function. The commit also removes the `cs_fold` function which is no longer used. (Note: there is some repetition now in `cs_cmp` and `cs_partial_cmp`. I will remove that in a follow-up PR.) --- .../rustc_builtin_macros/src/deriving/cmp/ord.rs | 21 +++- .../src/deriving/cmp/partial_ord.rs | 22 +++- .../src/deriving/generic/mod.rs | 25 ---- src/test/ui/deriving/deriving-all-codegen.stdout | 128 ++++----------------- 4 files changed, 57 insertions(+), 139 deletions(-) (limited to 'compiler') diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs index 4795b72a780..cb19107ae38 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs @@ -70,7 +70,7 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> Bl // cmp => cmp // } // - let expr = cs_fold( + let expr = cs_fold1( // foldr nests the if-elses correctly, leaving the first field // as the outermost one, and the last as the innermost. false, @@ -79,15 +79,12 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> Bl // ::std::cmp::Ordering::Equal => old, // cmp => cmp // } - let new = { let [other_f] = other_fs else { cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`"); }; - let args = vec![cx.expr_addr_of(span, self_f), cx.expr_addr_of(span, other_f.clone())]; - cx.expr_call_global(span, cmp_path.clone(), args) }; @@ -96,7 +93,21 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> Bl cx.expr_match(span, new, vec![eq_arm, neq_arm]) }, - cx.expr_path(equals_path.clone()), + |cx, args| match args { + Some((span, self_f, other_fs)) => { + let new = { + let [other_f] = other_fs else { + cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`"); + }; + let args = + vec![cx.expr_addr_of(span, self_f), cx.expr_addr_of(span, other_f.clone())]; + cx.expr_call_global(span, cmp_path.clone(), args) + }; + + new + } + None => cx.expr_path(equals_path.clone()), + }, Box::new(|cx, span, tag_tuple| { if tag_tuple.len() != 2 { cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`") 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 891485ab8ff..10b34985f47 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs @@ -2,7 +2,8 @@ use crate::deriving::generic::ty::*; use crate::deriving::generic::*; use crate::deriving::{path_std, pathvec_std}; -use rustc_ast::MetaItem; +use rustc_ast::ptr::P; +use rustc_ast::{Expr, MetaItem}; use rustc_expand::base::{Annotatable, ExtCtxt}; use rustc_span::symbol::{sym, Ident}; use rustc_span::Span; @@ -51,7 +52,6 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_ let test_id = Ident::new(sym::cmp, span); let ordering = cx.path_global(span, cx.std_path(&[sym::cmp, sym::Ordering, sym::Equal])); let ordering_expr = cx.expr_path(ordering.clone()); - let equals_expr = cx.expr_some(span, ordering_expr); let partial_cmp_path = cx.std_path(&[sym::cmp, sym::PartialOrd, sym::partial_cmp]); @@ -68,7 +68,7 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_ // cmp => cmp // } // - let expr = cs_fold( + let expr = cs_fold1( // foldr nests the if-elses correctly, leaving the first field // as the outermost one, and the last as the innermost. false, @@ -94,7 +94,21 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_ cx.expr_match(span, new, vec![eq_arm, neq_arm]) }, - equals_expr, + |cx: &mut ExtCtxt<'_>, args: Option<(Span, P, &[P])>| match args { + Some((span, self_f, other_fs)) => { + let new = { + let [other_f] = other_fs else { + cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`"); + }; + let args = + vec![cx.expr_addr_of(span, self_f), cx.expr_addr_of(span, other_f.clone())]; + cx.expr_call_global(span, partial_cmp_path.clone(), args) + }; + + new + } + None => cx.expr_some(span, ordering_expr.clone()), + }, Box::new(|cx, span, tag_tuple| { if tag_tuple.len() != 2 { cx.span_bug(span, "not exactly 2 arguments in `derive(PartialOrd)`") diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index f3e1176cab3..99793e675be 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -1694,31 +1694,6 @@ fn cs_fold_static(cx: &mut ExtCtxt<'_>, trait_span: Span) -> P { cx.span_bug(trait_span, "static function in `derive`") } -/// Fold the fields. `use_foldl` controls whether this is done -/// left-to-right (`true`) or right-to-left (`false`). -pub fn cs_fold( - use_foldl: bool, - f: F, - base: P, - enum_nonmatch_f: EnumNonMatchCollapsedFunc<'_>, - cx: &mut ExtCtxt<'_>, - trait_span: Span, - substructure: &Substructure<'_>, -) -> P -where - F: FnMut(&mut ExtCtxt<'_>, Span, P, P, &[P]) -> P, -{ - match *substructure.fields { - EnumMatching(.., ref all_fields) | Struct(_, ref all_fields) => { - cs_fold_fields(use_foldl, f, base, cx, all_fields) - } - EnumNonMatchingCollapsed(..) => { - cs_fold_enumnonmatch(enum_nonmatch_f, cx, trait_span, substructure) - } - StaticEnum(..) | StaticStruct(..) => cs_fold_static(cx, trait_span), - } -} - /// Function to fold over fields, with three cases, to generate more efficient and concise code. /// When the `substructure` has grouped fields, there are two cases: /// Zero fields: call the base case function with `None` (like the usual base case of `cs_fold`). diff --git a/src/test/ui/deriving/deriving-all-codegen.stdout b/src/test/ui/deriving/deriving-all-codegen.stdout index 763b39a212a..9b10192d75a 100644 --- a/src/test/ui/deriving/deriving-all-codegen.stdout +++ b/src/test/ui/deriving/deriving-all-codegen.stdout @@ -161,13 +161,7 @@ impl ::core::cmp::PartialOrd for Point { -> ::core::option::Option<::core::cmp::Ordering> { match ::core::cmp::PartialOrd::partial_cmp(&self.x, &other.x) { ::core::option::Option::Some(::core::cmp::Ordering::Equal) => - match ::core::cmp::PartialOrd::partial_cmp(&self.y, &other.y) - { - ::core::option::Option::Some(::core::cmp::Ordering::Equal) - => - ::core::option::Option::Some(::core::cmp::Ordering::Equal), - cmp => cmp, - }, + ::core::cmp::PartialOrd::partial_cmp(&self.y, &other.y), cmp => cmp, } } @@ -179,11 +173,7 @@ impl ::core::cmp::Ord for Point { fn cmp(&self, other: &Point) -> ::core::cmp::Ordering { match ::core::cmp::Ord::cmp(&self.x, &other.x) { ::core::cmp::Ordering::Equal => - match ::core::cmp::Ord::cmp(&self.y, &other.y) { - ::core::cmp::Ordering::Equal => - ::core::cmp::Ordering::Equal, - cmp => cmp, - }, + ::core::cmp::Ord::cmp(&self.y, &other.y), cmp => cmp, } } @@ -323,13 +313,7 @@ impl ::core::cmp::PartialOrd for Big { &other.b7) { ::core::option::Option::Some(::core::cmp::Ordering::Equal) => - match ::core::cmp::PartialOrd::partial_cmp(&self.b8, - &other.b8) { - ::core::option::Option::Some(::core::cmp::Ordering::Equal) - => - ::core::option::Option::Some(::core::cmp::Ordering::Equal), - cmp => cmp, - }, + ::core::cmp::PartialOrd::partial_cmp(&self.b8, &other.b8), cmp => cmp, }, cmp => cmp, @@ -365,11 +349,7 @@ impl ::core::cmp::Ord for Big { ::core::cmp::Ordering::Equal => match ::core::cmp::Ord::cmp(&self.b7, &other.b7) { ::core::cmp::Ordering::Equal => - match ::core::cmp::Ord::cmp(&self.b8, &other.b8) { - ::core::cmp::Ordering::Equal => - ::core::cmp::Ordering::Equal, - cmp => cmp, - }, + ::core::cmp::Ord::cmp(&self.b8, &other.b8), cmp => cmp, }, cmp => cmp, @@ -461,11 +441,7 @@ impl ::core::cmp::PartialOrd for Packed { -> ::core::option::Option<::core::cmp::Ordering> { let Self(__self_0_0) = *self; let Self(__self_1_0) = *other; - match ::core::cmp::PartialOrd::partial_cmp(&__self_0_0, &__self_1_0) { - ::core::option::Option::Some(::core::cmp::Ordering::Equal) => - ::core::option::Option::Some(::core::cmp::Ordering::Equal), - cmp => cmp, - } + ::core::cmp::PartialOrd::partial_cmp(&__self_0_0, &__self_1_0) } } #[automatically_derived] @@ -475,10 +451,7 @@ impl ::core::cmp::Ord for Packed { fn cmp(&self, other: &Packed) -> ::core::cmp::Ordering { let Self(__self_0_0) = *self; let Self(__self_1_0) = *other; - match ::core::cmp::Ord::cmp(&__self_0_0, &__self_1_0) { - ::core::cmp::Ordering::Equal => ::core::cmp::Ordering::Equal, - cmp => cmp, - } + ::core::cmp::Ord::cmp(&__self_0_0, &__self_1_0) } } @@ -621,13 +594,7 @@ impl ::core::cmp::PartialOrd for Enum1 { match (&*self, &*other) { (&Enum1::Single { x: ref __self_0 }, &Enum1::Single { x: ref __arg_1_0 }) => - match ::core::cmp::PartialOrd::partial_cmp(&*__self_0, - &*__arg_1_0) { - ::core::option::Option::Some(::core::cmp::Ordering::Equal) - => - ::core::option::Option::Some(::core::cmp::Ordering::Equal), - cmp => cmp, - }, + ::core::cmp::PartialOrd::partial_cmp(&*__self_0, &*__arg_1_0), } } } @@ -639,11 +606,7 @@ impl ::core::cmp::Ord for Enum1 { match (&*self, &*other) { (&Enum1::Single { x: ref __self_0 }, &Enum1::Single { x: ref __arg_1_0 }) => - match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) { - ::core::cmp::Ordering::Equal => - ::core::cmp::Ordering::Equal, - cmp => cmp, - }, + ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0), } } } @@ -873,26 +836,16 @@ impl ::core::cmp::PartialOrd for Mixed { if __self_vi == __arg_1_vi { match (&*self, &*other) { (&Mixed::R(ref __self_0), &Mixed::R(ref __arg_1_0)) => - match ::core::cmp::PartialOrd::partial_cmp(&*__self_0, - &*__arg_1_0) { - ::core::option::Option::Some(::core::cmp::Ordering::Equal) - => - ::core::option::Option::Some(::core::cmp::Ordering::Equal), - cmp => cmp, - }, + ::core::cmp::PartialOrd::partial_cmp(&*__self_0, + &*__arg_1_0), (&Mixed::S { d1: ref __self_0, d2: ref __self_1 }, &Mixed::S { d1: ref __arg_1_0, d2: ref __arg_1_1 }) => match ::core::cmp::PartialOrd::partial_cmp(&*__self_0, &*__arg_1_0) { ::core::option::Option::Some(::core::cmp::Ordering::Equal) => - match ::core::cmp::PartialOrd::partial_cmp(&*__self_1, - &*__arg_1_1) { - ::core::option::Option::Some(::core::cmp::Ordering::Equal) - => - ::core::option::Option::Some(::core::cmp::Ordering::Equal), - cmp => cmp, - }, + ::core::cmp::PartialOrd::partial_cmp(&*__self_1, + &*__arg_1_1), cmp => cmp, }, _ => @@ -913,20 +866,12 @@ impl ::core::cmp::Ord for Mixed { if __self_vi == __arg_1_vi { match (&*self, &*other) { (&Mixed::R(ref __self_0), &Mixed::R(ref __arg_1_0)) => - match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) { - ::core::cmp::Ordering::Equal => - ::core::cmp::Ordering::Equal, - cmp => cmp, - }, + ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0), (&Mixed::S { d1: ref __self_0, d2: ref __self_1 }, &Mixed::S { d1: ref __arg_1_0, d2: ref __arg_1_1 }) => match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) { ::core::cmp::Ordering::Equal => - match ::core::cmp::Ord::cmp(&*__self_1, &*__arg_1_1) { - ::core::cmp::Ordering::Equal => - ::core::cmp::Ordering::Equal, - cmp => cmp, - }, + ::core::cmp::Ord::cmp(&*__self_1, &*__arg_1_1), cmp => cmp, }, _ => ::core::cmp::Ordering::Equal, @@ -1054,29 +999,14 @@ impl ::core::cmp::PartialOrd for Fielded { if __self_vi == __arg_1_vi { match (&*self, &*other) { (&Fielded::X(ref __self_0), &Fielded::X(ref __arg_1_0)) => - match ::core::cmp::PartialOrd::partial_cmp(&*__self_0, - &*__arg_1_0) { - ::core::option::Option::Some(::core::cmp::Ordering::Equal) - => - ::core::option::Option::Some(::core::cmp::Ordering::Equal), - cmp => cmp, - }, + ::core::cmp::PartialOrd::partial_cmp(&*__self_0, + &*__arg_1_0), (&Fielded::Y(ref __self_0), &Fielded::Y(ref __arg_1_0)) => - match ::core::cmp::PartialOrd::partial_cmp(&*__self_0, - &*__arg_1_0) { - ::core::option::Option::Some(::core::cmp::Ordering::Equal) - => - ::core::option::Option::Some(::core::cmp::Ordering::Equal), - cmp => cmp, - }, + ::core::cmp::PartialOrd::partial_cmp(&*__self_0, + &*__arg_1_0), (&Fielded::Z(ref __self_0), &Fielded::Z(ref __arg_1_0)) => - match ::core::cmp::PartialOrd::partial_cmp(&*__self_0, - &*__arg_1_0) { - ::core::option::Option::Some(::core::cmp::Ordering::Equal) - => - ::core::option::Option::Some(::core::cmp::Ordering::Equal), - cmp => cmp, - }, + ::core::cmp::PartialOrd::partial_cmp(&*__self_0, + &*__arg_1_0), _ => unsafe { ::core::intrinsics::unreachable() } } } else { @@ -1094,23 +1024,11 @@ impl ::core::cmp::Ord for Fielded { if __self_vi == __arg_1_vi { match (&*self, &*other) { (&Fielded::X(ref __self_0), &Fielded::X(ref __arg_1_0)) => - match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) { - ::core::cmp::Ordering::Equal => - ::core::cmp::Ordering::Equal, - cmp => cmp, - }, + ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0), (&Fielded::Y(ref __self_0), &Fielded::Y(ref __arg_1_0)) => - match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) { - ::core::cmp::Ordering::Equal => - ::core::cmp::Ordering::Equal, - cmp => cmp, - }, + ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0), (&Fielded::Z(ref __self_0), &Fielded::Z(ref __arg_1_0)) => - match ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0) { - ::core::cmp::Ordering::Equal => - ::core::cmp::Ordering::Equal, - cmp => cmp, - }, + ::core::cmp::Ord::cmp(&*__self_0, &*__arg_1_0), _ => unsafe { ::core::intrinsics::unreachable() } } } else { ::core::cmp::Ord::cmp(&__self_vi, &__arg_1_vi) } -- cgit 1.4.1-3-g733a5 From 0da063c9916fc3ce1330ac14e28ea7bca002ac89 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 4 Jul 2022 17:10:36 +1000 Subject: Inline and remove the `cs_fold_*` functions. Because they now have a single call site each. Also rename `cs_fold1` as `cs_fold`, now that it's the only folding function left. --- .../rustc_builtin_macros/src/deriving/cmp/ord.rs | 2 +- .../src/deriving/cmp/partial_eq.rs | 2 +- .../src/deriving/cmp/partial_ord.rs | 2 +- .../src/deriving/generic/mod.rs | 62 +++++----------------- 4 files changed, 17 insertions(+), 51 deletions(-) (limited to 'compiler') diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs index cb19107ae38..bec59aac5ee 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs @@ -70,7 +70,7 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> Bl // cmp => cmp // } // - let expr = cs_fold1( + let expr = cs_fold( // foldr nests the if-elses correctly, leaving the first field // as the outermost one, and the last as the innermost. false, 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 f58e303b8f0..b44c290d12f 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs @@ -31,7 +31,7 @@ pub fn expand_deriving_partial_eq( cx.expr_binary(span, op, self_f, other_f.clone()) }; - let expr = cs_fold1( + let expr = cs_fold( true, // use foldl |cx, span, subexpr, self_f, other_fs| { let eq = op(cx, span, self_f, other_fs); 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 10b34985f47..5769f08f494 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs @@ -68,7 +68,7 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_ // cmp => cmp // } // - let expr = cs_fold1( + let expr = cs_fold( // foldr nests the if-elses correctly, leaving the first field // as the outermost one, and the last as the innermost. false, diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 99793e675be..e618255b0c6 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -1654,46 +1654,6 @@ impl<'a> TraitDef<'a> { } } -// helpful premade recipes - -fn cs_fold_fields<'a, F>( - use_foldl: bool, - mut f: F, - base: P, - cx: &mut ExtCtxt<'_>, - all_fields: &[FieldInfo<'a>], -) -> P -where - F: FnMut(&mut ExtCtxt<'_>, Span, P, P, &[P]) -> P, -{ - if use_foldl { - all_fields - .iter() - .fold(base, |old, field| f(cx, field.span, old, field.self_.clone(), &field.other)) - } else { - all_fields - .iter() - .rev() - .fold(base, |old, field| f(cx, field.span, old, field.self_.clone(), &field.other)) - } -} - -fn cs_fold_enumnonmatch( - mut enum_nonmatch_f: EnumNonMatchCollapsedFunc<'_>, - cx: &mut ExtCtxt<'_>, - trait_span: Span, - substructure: &Substructure<'_>, -) -> P { - match *substructure.fields { - EnumNonMatchingCollapsed(tuple) => enum_nonmatch_f(cx, trait_span, tuple), - _ => cx.span_bug(trait_span, "cs_fold_enumnonmatch expected an EnumNonMatchingCollapsed"), - } -} - -fn cs_fold_static(cx: &mut ExtCtxt<'_>, trait_span: Span) -> P { - cx.span_bug(trait_span, "static function in `derive`") -} - /// Function to fold over fields, with three cases, to generate more efficient and concise code. /// When the `substructure` has grouped fields, there are two cases: /// Zero fields: call the base case function with `None` (like the usual base case of `cs_fold`). @@ -1702,11 +1662,11 @@ fn cs_fold_static(cx: &mut ExtCtxt<'_>, trait_span: Span) -> P { /// fields. /// When the `substructure` is an `EnumNonMatchingCollapsed`, the result of `enum_nonmatch_f` /// is returned. Statics may not be folded over. -pub fn cs_fold1( +pub fn cs_fold( use_foldl: bool, - f: F, + mut f: F, mut b: B, - enum_nonmatch_f: EnumNonMatchCollapsedFunc<'_>, + mut enum_nonmatch_f: EnumNonMatchCollapsedFunc<'_>, cx: &mut ExtCtxt<'_>, trait_span: Span, substructure: &Substructure<'_>, @@ -1731,12 +1691,18 @@ where (true, _) => (b(cx, None), &all_fields[..]), }; - cs_fold_fields(use_foldl, f, base, cx, rest) - } - EnumNonMatchingCollapsed(..) => { - cs_fold_enumnonmatch(enum_nonmatch_f, cx, trait_span, substructure) + if use_foldl { + rest.iter().fold(base, |old, field| { + f(cx, field.span, old, field.self_.clone(), &field.other) + }) + } else { + rest.iter().rev().fold(base, |old, field| { + f(cx, field.span, old, field.self_.clone(), &field.other) + }) + } } - StaticEnum(..) | StaticStruct(..) => cs_fold_static(cx, trait_span), + EnumNonMatchingCollapsed(tuple) => enum_nonmatch_f(cx, trait_span, tuple), + StaticEnum(..) | StaticStruct(..) => cx.span_bug(trait_span, "static function in `derive`"), } } -- cgit 1.4.1-3-g733a5