Skip to content

More derive output improvements #98758

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 7 commits into from
Jul 8, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
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.
  • Loading branch information
nnethercote committed Jul 4, 2022
commit 5762d2385ebdd735b110ab82e80eed6efa7d7c55
15 changes: 7 additions & 8 deletions compiler/rustc_builtin_macros/src/deriving/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -98,7 +97,7 @@ fn cs_clone_simple(
trait_span: Span,
substr: &Substructure<'_>,
is_union: bool,
) -> P<Expr> {
) -> BlockOrExpr {
let mut stmts = Vec::new();
let mut process_variant = |variant: &VariantData| {
for field in variant.fields() {
Expand Down Expand Up @@ -139,16 +138,15 @@ 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(
name: &str,
cx: &mut ExtCtxt<'_>,
trait_span: Span,
substr: &Substructure<'_>,
) -> P<Expr> {
) -> BlockOrExpr {
let ctor_path;
let all_fields;
let fn_path = cx.std_path(&[sym::clone, sym::Clone, sym::clone]);
Expand Down Expand Up @@ -177,7 +175,7 @@ fn cs_clone(
}
}

match *vdata {
let expr = match *vdata {
VariantData::Struct(..) => {
let fields = all_fields
.iter()
Expand All @@ -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)
}
7 changes: 3 additions & 4 deletions compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,7 +51,7 @@ fn cs_total_eq_assert(
cx: &mut ExtCtxt<'_>,
trait_span: Span,
substr: &Substructure<'_>,
) -> P<Expr> {
) -> BlockOrExpr {
let mut stmts = Vec::new();
let mut process_variant = |variant: &ast::VariantData| {
for field in variant.fields() {
Expand All @@ -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)
}
9 changes: 5 additions & 4 deletions compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Expr> {
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]));

Expand All @@ -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,
Expand Down Expand Up @@ -107,5 +107,6 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<
cx,
span,
substr,
)
);
BlockOrExpr::new_expr(expr)
}
13 changes: 6 additions & 7 deletions compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,14 @@ 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,
substr: &Substructure<'_>,
op: BinOpKind,
combiner: BinOpKind,
base: bool,
) -> P<Expr> {
) -> BlockOrExpr {
let op = |cx: &mut ExtCtxt<'_>, span: Span, self_f: P<Expr>, other_fs: &[P<Expr>]| {
let [other_f] = other_fs else {
cx.span_bug(span, "not exactly 2 arguments in `derive(PartialEq)`");
Expand All @@ -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);
Expand All @@ -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<Expr> {
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<Expr> {
fn cs_ne(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
cs_op(cx, span, substr, BinOpKind::Ne, BinOpKind::Or, false)
}

Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Expr> {
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());
Expand All @@ -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,
Expand Down Expand Up @@ -110,5 +109,6 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
cx,
span,
substr,
)
);
BlockOrExpr::new_expr(expr)
}
15 changes: 7 additions & 8 deletions compiler/rustc_builtin_macros/src/deriving/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Expr> {
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),
Expand Down Expand Up @@ -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 {
Expand All @@ -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());
Expand Down Expand Up @@ -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)
}
}
7 changes: 4 additions & 3 deletions compiler/rustc_builtin_macros/src/deriving/decodable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ fn decodable_substructure(
trait_span: Span,
substr: &Substructure<'_>,
krate: Symbol,
) -> P<Expr> {
) -> BlockOrExpr {
let decoder = substr.nonself_args[0].clone();
let recurse = vec![
Ident::new(krate, trait_span),
Expand All @@ -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(),
Expand Down Expand Up @@ -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:
Expand Down
41 changes: 18 additions & 23 deletions compiler/rustc_builtin_macros/src/deriving/default.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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),
) {
Expand Down Expand Up @@ -59,12 +58,12 @@ fn default_struct_substructure(
trait_span: Span,
substr: &Substructure<'_>,
summary: &StaticFields,
) -> P<Expr> {
) -> 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)
Expand All @@ -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<Expr> {
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>(
Expand Down
Loading