Skip to content
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

Improve derive(Debug) #98190

Merged
merged 3 commits into from
Jun 26, 2022
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
205 changes: 117 additions & 88 deletions compiler/rustc_builtin_macros/src/deriving/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@ use crate::deriving::generic::*;
use crate::deriving::path_std;

use rustc_ast::ptr::P;
use rustc_ast::{self as ast, Expr, LocalKind, MetaItem};
use rustc_ast::{self as ast, Expr, MetaItem};
use rustc_expand::base::{Annotatable, ExtCtxt};
use rustc_span::symbol::{sym, Ident};
use rustc_span::{Span, DUMMY_SP};

fn make_mut_borrow(cx: &mut ExtCtxt<'_>, sp: Span, expr: P<Expr>) -> P<Expr> {
cx.expr(sp, ast::ExprKind::AddrOf(ast::BorrowKind::Ref, ast::Mutability::Mut, expr))
}
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::Span;

pub fn expand_deriving_debug(
cx: &mut ExtCtxt<'_>,
Expand Down Expand Up @@ -49,11 +45,7 @@ pub fn expand_deriving_debug(
trait_def.expand(cx, mitem, item, push)
}

/// We use the debug builders to do the heavy lifting here
fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> P<Expr> {
// build fmt.debug_struct(<name>).field(<fieldname>, &<fieldval>)....build()
// or fmt.debug_tuple(<name>).field(&<fieldval>)....build()
// based on the "shape".
let (ident, vdata, fields) = match substr.fields {
Struct(vdata, fields) => (substr.type_ident, *vdata, fields),
EnumMatching(_, _, v, fields) => (v.ident, &v.data, fields),
Expand All @@ -67,93 +59,130 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
let name = cx.expr_lit(span, ast::LitKind::Str(ident.name, ast::StrStyle::Cooked));
let fmt = substr.nonself_args[0].clone();

// Special fast path for unit variants. In the common case of an enum that is entirely unit
// variants (i.e. a C-like enum), this fast path allows LLVM to eliminate the entire switch in
// favor of a lookup table.
if let ast::VariantData::Unit(..) = vdata {
let fn_path_write_str = cx.std_path(&[sym::fmt, sym::Formatter, sym::write_str]);
let expr = cx.expr_call_global(span, fn_path_write_str, vec![fmt, name]);
let stmts = vec![cx.stmt_expr(expr)];
let block = cx.block(span, stmts);
return cx.expr_block(block);
}

let builder = Ident::new(sym::debug_trait_builder, span);
let builder_expr = cx.expr_ident(span, builder);

let mut stmts = Vec::with_capacity(fields.len() + 2);
let fn_path_finish;
match vdata {
// Struct and tuples are similar enough that we use the same code for both,
// with some extra pieces for structs due to the field names.
let (is_struct, args_per_field) = match vdata {
ast::VariantData::Unit(..) => {
cx.span_bug(span, "unit variants should have been handled above");
// Special fast path for unit variants.
//let fn_path_write_str = cx.std_path(&[sym::fmt, sym::Formatter, sym::write_str]);
//return cx.expr_call_global(span, fn_path_write_str, vec![fmt, name]);
assert!(fields.is_empty());
(false, 0)
}
ast::VariantData::Tuple(..) => {
// tuple struct/"normal" variant
let fn_path_debug_tuple = cx.std_path(&[sym::fmt, sym::Formatter, sym::debug_tuple]);
let expr = cx.expr_call_global(span, fn_path_debug_tuple, vec![fmt, name]);
let expr = make_mut_borrow(cx, span, expr);
stmts.push(cx.stmt_let(span, false, builder, expr));

for field in fields {
// Use double indirection to make sure this works for unsized types
let field = cx.expr_addr_of(field.span, field.self_.clone());
let field = cx.expr_addr_of(field.span, field);

let fn_path_field = cx.std_path(&[sym::fmt, sym::DebugTuple, sym::field]);
let expr =
cx.expr_call_global(span, fn_path_field, vec![builder_expr.clone(), field]);

// Use `let _ = expr;` to avoid triggering the
// unused_results lint.
stmts.push(stmt_let_underscore(cx, span, expr));
}
ast::VariantData::Tuple(..) => (false, 1),
ast::VariantData::Struct(..) => (true, 2),
};

fn_path_finish = cx.std_path(&[sym::fmt, sym::DebugTuple, sym::finish]);
}
ast::VariantData::Struct(..) => {
// normal struct/struct variant
let fn_path_debug_struct = cx.std_path(&[sym::fmt, sym::Formatter, sym::debug_struct]);
let expr = cx.expr_call_global(span, fn_path_debug_struct, vec![fmt, name]);
let expr = make_mut_borrow(cx, span, expr);
stmts.push(cx.stmt_let(DUMMY_SP, false, builder, expr));

for field in fields {
// The number of fields that can be handled without an array.
const CUTOFF: usize = 5;

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])
} else if fields.len() <= CUTOFF {
// Few enough fields that we can use a specific-length method.
let debug = if is_struct {
format!("debug_struct_field{}_finish", fields.len())
} else {
format!("debug_tuple_field{}_finish", fields.len())
};
let fn_path_debug = cx.std_path(&[sym::fmt, sym::Formatter, Symbol::intern(&debug)]);
Comment on lines +85 to +90
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hacky:

  1. String can be preinterned
  2. Given that preinterned string sorted, we can get symbol as offset from other known symbol, for example for structs for fields.len() == 4 it should be debug_struct_field1_finish+3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can get symbol as offset from other known symbol

Symbol doesn't have a public constructor that accepts an integer. It is an optimization LLVM should be able to make itself though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I'd want to write this code assuming a particular order of representation of tokens.

It could probably use a [sym::debug_struct_field1_finish, sym::debug_struct_field2_finish, sym::debug_struct_field3_finish].get(fields.len() - 1) kind of thing, though? I wouldn't block on that, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code isn't hot enough for symbol interning tricks to be worthwhile.


let mut args = Vec::with_capacity(2 + fields.len() * args_per_field);
args.extend([fmt, name]);
for i in 0..fields.len() {
let field = &fields[i];
if is_struct {
let name = cx.expr_lit(
field.span,
ast::LitKind::Str(field.name.unwrap().name, ast::StrStyle::Cooked),
);

// Use double indirection to make sure this works for unsized types
let fn_path_field = cx.std_path(&[sym::fmt, sym::DebugStruct, sym::field]);
let field = cx.expr_addr_of(field.span, field.self_.clone());
let field = cx.expr_addr_of(field.span, field);
let expr = cx.expr_call_global(
span,
fn_path_field,
vec![builder_expr.clone(), name, field],
);
stmts.push(stmt_let_underscore(cx, span, expr));
args.push(name);
}
fn_path_finish = cx.std_path(&[sym::fmt, sym::DebugStruct, sym::finish]);
// Use double indirection to make sure this works for unsized types
let field = cx.expr_addr_of(field.span, field.self_.clone());
let field = cx.expr_addr_of(field.span, field);
args.push(field);
}
}
cx.expr_call_global(span, fn_path_debug, args)
} else {
// Enough fields that we must use the any-length method.
let mut name_exprs = Vec::with_capacity(fields.len());
let mut value_exprs = Vec::with_capacity(fields.len());

for field in fields {
if is_struct {
name_exprs.push(cx.expr_lit(
field.span,
ast::LitKind::Str(field.name.unwrap().name, ast::StrStyle::Cooked),
));
}

let expr = cx.expr_call_global(span, fn_path_finish, vec![builder_expr]);
// Use double indirection to make sure this works for unsized types
let value_ref = cx.expr_addr_of(field.span, field.self_.clone());
value_exprs.push(cx.expr_addr_of(field.span, value_ref));
}

stmts.push(cx.stmt_expr(expr));
let block = cx.block(span, stmts);
cx.expr_block(block)
}
// `let names: &'static _ = &["field1", "field2"];`
let names_let = if is_struct {
let lt_static = Some(cx.lifetime_static(span));
let ty_static_ref =
cx.ty_rptr(span, cx.ty_infer(span), lt_static, ast::Mutability::Not);
Some(cx.stmt_let_ty(
span,
false,
Ident::new(sym::names, span),
Some(ty_static_ref),
cx.expr_array_ref(span, name_exprs),
))
} else {
None
};

// `let values: &[&dyn Debug] = &[&&self.field1, &&self.field2];`
let path_debug = cx.path_global(span, cx.std_path(&[sym::fmt, sym::Debug]));
let ty_dyn_debug = cx.ty(
span,
ast::TyKind::TraitObject(vec![cx.trait_bound(path_debug)], ast::TraitObjectSyntax::Dyn),
);
let ty_slice = cx.ty(
span,
ast::TyKind::Slice(cx.ty_rptr(span, ty_dyn_debug, None, ast::Mutability::Not)),
);
let values_let = cx.stmt_let_ty(
span,
false,
Ident::new(sym::values, span),
Some(cx.ty_rptr(span, ty_slice, None, ast::Mutability::Not)),
cx.expr_array_ref(span, value_exprs),
);

// `fmt::Formatter::debug_struct_fields_finish(fmt, name, names, values)` or
// `fmt::Formatter::debug_tuple_fields_finish(fmt, name, values)`
let sym_debug = if is_struct {
sym::debug_struct_fields_finish
} else {
sym::debug_tuple_fields_finish
};
let fn_path_debug_internal = cx.std_path(&[sym::fmt, sym::Formatter, sym_debug]);

let mut args = Vec::with_capacity(4);
args.push(fmt);
args.push(name);
if is_struct {
args.push(cx.expr_ident(span, Ident::new(sym::names, span)));
}
args.push(cx.expr_ident(span, Ident::new(sym::values, span)));
let expr = cx.expr_call_global(span, fn_path_debug_internal, args);

fn stmt_let_underscore(cx: &mut ExtCtxt<'_>, sp: Span, expr: P<ast::Expr>) -> ast::Stmt {
let local = P(ast::Local {
pat: cx.pat_wild(sp),
ty: None,
id: ast::DUMMY_NODE_ID,
kind: LocalKind::Init(expr),
span: sp,
attrs: ast::AttrVec::new(),
tokens: None,
});
ast::Stmt { id: ast::DUMMY_NODE_ID, kind: ast::StmtKind::Local(local), span: sp }
let mut stmts = Vec::with_capacity(3);
if is_struct {
stmts.push(names_let.unwrap());
}
stmts.push(values_let);
stmts.push(cx.stmt_expr(expr));

cx.expr_block(cx.block(span, stmts))
}
}
5 changes: 2 additions & 3 deletions compiler/rustc_builtin_macros/src/deriving/decodable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,13 @@ fn decodable_substructure(
cx.expr_match(trait_span, cx.expr_ident(trait_span, variant), arms),
);
let lambda = cx.lambda(trait_span, vec![blkarg, variant], result);
let variant_vec = cx.expr_vec(trait_span, variants);
let variant_vec = cx.expr_addr_of(trait_span, variant_vec);
let variant_array_ref = cx.expr_array_ref(trait_span, variants);
let fn_read_enum_variant_path: Vec<_> =
cx.def_site_path(&[sym::rustc_serialize, sym::Decoder, sym::read_enum_variant]);
let result = cx.expr_call_global(
trait_span,
fn_read_enum_variant_path,
vec![blkdecoder, variant_vec, lambda],
vec![blkdecoder, variant_array_ref, lambda],
);
let fn_read_enum_path: Vec<_> =
cx.def_site_path(&[sym::rustc_serialize, sym::Decoder, sym::read_enum]);
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ impl<'a, 'b> Context<'a, 'b> {

// First, build up the static array which will become our precompiled
// format "string"
let pieces = self.ecx.expr_vec_slice(self.fmtsp, self.str_pieces);
let pieces = self.ecx.expr_array_ref(self.fmtsp, self.str_pieces);

// We need to construct a &[ArgumentV1] to pass into the fmt::Arguments
// constructor. In general the expressions in this slice might be
Expand Down Expand Up @@ -849,7 +849,7 @@ impl<'a, 'b> Context<'a, 'b> {
fmt_args.push(Context::format_arg(self.ecx, self.macsp, span, arg_ty, arg));
}

let args_array = self.ecx.expr_vec(self.macsp, fmt_args);
let args_array = self.ecx.expr_array(self.macsp, fmt_args);
let args_slice = self.ecx.expr_addr_of(
self.macsp,
if no_need_for_match {
Expand Down Expand Up @@ -879,7 +879,7 @@ impl<'a, 'b> Context<'a, 'b> {
} else {
// Build up the static array which will store our precompiled
// nonstandard placeholders, if there are any.
let fmt = self.ecx.expr_vec_slice(self.macsp, self.pieces);
let fmt = self.ecx.expr_array_ref(self.macsp, self.pieces);

let path = self.ecx.std_path(&[sym::fmt, sym::UnsafeArg, sym::new]);
let unsafe_arg = self.ecx.expr_call_global(self.macsp, path, Vec::new());
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_builtin_macros/src/proc_macro_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ fn mk_decls(cx: &mut ExtCtxt<'_>, macros: &[ProcMacro]) -> P<ast::Item> {
proc_macro_ty_method_path(cx, custom_derive),
vec![
cx.expr_str(span, cd.trait_name),
cx.expr_vec_slice(
cx.expr_array_ref(
span,
cd.attrs.iter().map(|&s| cx.expr_str(span, s)).collect::<Vec<_>>(),
),
Expand Down Expand Up @@ -362,7 +362,7 @@ fn mk_decls(cx: &mut ExtCtxt<'_>, macros: &[ProcMacro]) -> P<ast::Item> {
ast::Mutability::Not,
),
ast::Mutability::Not,
cx.expr_vec_slice(span, decls),
cx.expr_array_ref(span, decls),
)
.map(|mut i| {
let attr = cx.meta_word(span, sym::rustc_proc_macro_decls);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/test_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ fn mk_tests_slice(cx: &TestCtxt<'_>, sp: Span) -> P<ast::Expr> {
debug!("building test vector from {} tests", cx.test_cases.len());
let ecx = &cx.ext_cx;

ecx.expr_vec_slice(
ecx.expr_array_ref(
sp,
cx.test_cases
.iter()
Expand Down
31 changes: 27 additions & 4 deletions compiler/rustc_expand/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ impl<'a> ExtCtxt<'a> {
P(ast::Ty { id: ast::DUMMY_NODE_ID, span, kind, tokens: None })
}

pub fn ty_infer(&self, span: Span) -> P<ast::Ty> {
self.ty(span, ast::TyKind::Infer)
}

pub fn ty_path(&self, path: ast::Path) -> P<ast::Ty> {
self.ty(path.span, ast::TyKind::Path(None, path))
}
Expand Down Expand Up @@ -140,11 +144,26 @@ impl<'a> ExtCtxt<'a> {
ast::Lifetime { id: ast::DUMMY_NODE_ID, ident: ident.with_span_pos(span) }
}

pub fn lifetime_static(&self, span: Span) -> ast::Lifetime {
self.lifetime(span, Ident::new(kw::StaticLifetime, span))
}

pub fn stmt_expr(&self, expr: P<ast::Expr>) -> ast::Stmt {
ast::Stmt { id: ast::DUMMY_NODE_ID, span: expr.span, kind: ast::StmtKind::Expr(expr) }
}

pub fn stmt_let(&self, sp: Span, mutbl: bool, ident: Ident, ex: P<ast::Expr>) -> ast::Stmt {
self.stmt_let_ty(sp, mutbl, ident, None, ex)
}

pub fn stmt_let_ty(
&self,
sp: Span,
mutbl: bool,
ident: Ident,
ty: Option<P<ast::Ty>>,
ex: P<ast::Expr>,
) -> ast::Stmt {
let pat = if mutbl {
let binding_mode = ast::BindingMode::ByValue(ast::Mutability::Mut);
self.pat_ident_binding_mode(sp, ident, binding_mode)
Expand All @@ -153,7 +172,7 @@ impl<'a> ExtCtxt<'a> {
};
let local = P(ast::Local {
pat,
ty: None,
ty,
id: ast::DUMMY_NODE_ID,
kind: LocalKind::Init(ex),
span: sp,
Expand Down Expand Up @@ -315,12 +334,16 @@ impl<'a> ExtCtxt<'a> {
self.expr_lit(sp, ast::LitKind::Bool(value))
}

pub fn expr_vec(&self, sp: Span, exprs: Vec<P<ast::Expr>>) -> P<ast::Expr> {
/// `[expr1, expr2, ...]`
pub fn expr_array(&self, sp: Span, exprs: Vec<P<ast::Expr>>) -> P<ast::Expr> {
self.expr(sp, ast::ExprKind::Array(exprs))
}
pub fn expr_vec_slice(&self, sp: Span, exprs: Vec<P<ast::Expr>>) -> P<ast::Expr> {
self.expr_addr_of(sp, self.expr_vec(sp, exprs))

/// `&[expr1, expr2, ...]`
pub fn expr_array_ref(&self, sp: Span, exprs: Vec<P<ast::Expr>>) -> P<ast::Expr> {
self.expr_addr_of(sp, self.expr_array(sp, exprs))
}

pub fn expr_str(&self, sp: Span, s: Symbol) -> P<ast::Expr> {
self.expr_lit(sp, ast::LitKind::Str(s, ast::StrStyle::Cooked))
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,10 @@ symbols! {
debug_assert_ne_macro,
debug_assertions,
debug_struct,
debug_struct_fields_finish,
debug_trait_builder,
debug_tuple,
debug_tuple_fields_finish,
debugger_visualizer,
decl_macro,
declare_lint_pass,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_type_ir/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![feature(fmt_helpers_for_derive)]
#![feature(min_specialization)]
#![feature(rustc_attrs)]

Expand Down
Loading