Skip to content

Commit 96f09d7

Browse files
committed
Remove unnecessary &* sigil pairs in derived code.
By producing `&T` expressions for fields instead of `T`. This matches what the existing comments (e.g. on `FieldInfo`) claim is happening, and it's also what most of the trait-specific code needs. The exception is `PartialEq`, which needs `T` expressions for lots of special case error messaging to work. So we now convert the `&T` back to a `T` for `PartialEq`.
1 parent 277bc96 commit 96f09d7

File tree

8 files changed

+102
-81
lines changed

8 files changed

+102
-81
lines changed

compiler/rustc_builtin_macros/src/deriving/clone.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ fn cs_clone(
161161
let all_fields;
162162
let fn_path = cx.std_path(&[sym::clone, sym::Clone, sym::clone]);
163163
let subcall = |cx: &mut ExtCtxt<'_>, field: &FieldInfo| {
164-
let args = vec![cx.expr_addr_of(field.span, field.self_expr.clone())];
164+
let args = vec![field.self_expr.clone()];
165165
cx.expr_call_global(field.span, fn_path.clone(), args)
166166
};
167167

compiler/rustc_builtin_macros/src/deriving/cmp/ord.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,7 @@ pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> Bl
6363
let [other_expr] = &field.other_selflike_exprs[..] else {
6464
cx.span_bug(field.span, "not exactly 2 arguments in `derive(Ord)`");
6565
};
66-
let args = vec![
67-
cx.expr_addr_of(field.span, field.self_expr.clone()),
68-
cx.expr_addr_of(field.span, other_expr.clone()),
69-
];
66+
let args = vec![field.self_expr.clone(), other_expr.clone()];
7067
cx.expr_call_global(field.span, cmp_path.clone(), args)
7168
}
7269
CsFold::Combine(span, expr1, expr2) => {

compiler/rustc_builtin_macros/src/deriving/cmp/partial_eq.rs

+17-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use crate::deriving::generic::ty::*;
22
use crate::deriving::generic::*;
33
use crate::deriving::{path_local, path_std};
44

5-
use rustc_ast::{BinOpKind, MetaItem};
5+
use rustc_ast::ptr::P;
6+
use rustc_ast::{BinOpKind, BorrowKind, Expr, ExprKind, MetaItem, Mutability};
67
use rustc_expand::base::{Annotatable, ExtCtxt};
78
use rustc_span::symbol::sym;
89
use rustc_span::Span;
@@ -32,7 +33,21 @@ pub fn expand_deriving_partial_eq(
3233
let [other_expr] = &field.other_selflike_exprs[..] else {
3334
cx.span_bug(field.span, "not exactly 2 arguments in `derive(PartialEq)`");
3435
};
35-
cx.expr_binary(field.span, op, field.self_expr.clone(), other_expr.clone())
36+
37+
// We received `&T` arguments. Convert them to `T` by
38+
// stripping `&` or adding `*`. This isn't necessary for
39+
// type checking, but it results in much better error
40+
// messages if something goes wrong.
41+
let convert = |expr: &P<Expr>| {
42+
if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, inner) =
43+
&expr.kind
44+
{
45+
inner.clone()
46+
} else {
47+
cx.expr_deref(field.span, expr.clone())
48+
}
49+
};
50+
cx.expr_binary(field.span, op, convert(&field.self_expr), convert(other_expr))
3651
}
3752
CsFold::Combine(span, expr1, expr2) => cx.expr_binary(span, combiner, expr1, expr2),
3853
CsFold::Fieldless => cx.expr_bool(span, base),

compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,7 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_
7171
let [other_expr] = &field.other_selflike_exprs[..] else {
7272
cx.span_bug(field.span, "not exactly 2 arguments in `derive(Ord)`");
7373
};
74-
let args = vec![
75-
cx.expr_addr_of(field.span, field.self_expr.clone()),
76-
cx.expr_addr_of(field.span, other_expr.clone()),
77-
];
74+
let args = vec![field.self_expr.clone(), other_expr.clone()];
7875
cx.expr_call_global(field.span, partial_cmp_path.clone(), args)
7976
}
8077
CsFold::Combine(span, expr1, expr2) => {

compiler/rustc_builtin_macros/src/deriving/debug.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,8 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
9595
);
9696
args.push(name);
9797
}
98-
// Use double indirection to make sure this works for unsized types
98+
// Use an extra indirection to make sure this works for unsized types.
9999
let field = cx.expr_addr_of(field.span, field.self_expr.clone());
100-
let field = cx.expr_addr_of(field.span, field);
101100
args.push(field);
102101
}
103102
let expr = cx.expr_call_global(span, fn_path_debug, args);
@@ -115,9 +114,9 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
115114
));
116115
}
117116

118-
// Use double indirection to make sure this works for unsized types
119-
let value_ref = cx.expr_addr_of(field.span, field.self_expr.clone());
120-
value_exprs.push(cx.expr_addr_of(field.span, value_ref));
117+
// Use an extra indirection to make sure this works for unsized types.
118+
let field = cx.expr_addr_of(field.span, field.self_expr.clone());
119+
value_exprs.push(field);
121120
}
122121

123122
// `let names: &'static _ = &["field1", "field2"];`

compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

+32-16
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,7 @@ impl<'a> MethodDef<'a> {
10041004
/// ```
10051005
/// #[derive(PartialEq)]
10061006
/// # struct Dummy;
1007-
/// struct A { x: i32, y: i32 }
1007+
/// struct A { x: u8, y: u8 }
10081008
///
10091009
/// // equivalent to:
10101010
/// impl PartialEq for A {
@@ -1016,9 +1016,9 @@ impl<'a> MethodDef<'a> {
10161016
/// But if the struct is `repr(packed)`, we can't use something like
10171017
/// `&self.x` on a packed type (as required for e.g. `Debug` and `Hash`)
10181018
/// because that might cause an unaligned ref. So we use let-destructuring
1019-
/// instead.
1019+
/// instead. If the struct impls `Copy`:
10201020
/// ```
1021-
/// # struct A { x: i32, y: i32 }
1021+
/// # struct A { x: u8, y: u8 }
10221022
/// impl PartialEq for A {
10231023
/// fn eq(&self, other: &A) -> bool {
10241024
/// let Self { x: __self_0_0, y: __self_0_1 } = *self;
@@ -1027,6 +1027,19 @@ impl<'a> MethodDef<'a> {
10271027
/// }
10281028
/// }
10291029
/// ```
1030+
/// If it doesn't impl `Copy`:
1031+
/// ```
1032+
/// # struct A { x: u8, y: u8 }
1033+
/// impl PartialEq for A {
1034+
/// fn eq(&self, other: &A) -> bool {
1035+
/// let Self { x: ref __self_0_0, y: ref __self_0_1 } = *self;
1036+
/// let Self { x: ref __self_1_0, y: ref __self_1_1 } = *other;
1037+
/// *__self_0_0 == *__self_1_0 && *__self_0_1 == *__self_1_1
1038+
/// }
1039+
/// }
1040+
/// ```
1041+
/// This latter case only works if the fields match the alignment required
1042+
/// by the `packed(N)` attribute.
10301043
fn expand_struct_method_body<'b>(
10311044
&self,
10321045
cx: &mut ExtCtxt<'_>,
@@ -1058,9 +1071,9 @@ impl<'a> MethodDef<'a> {
10581071
} else {
10591072
let prefixes: Vec<_> =
10601073
(0..selflike_args.len()).map(|i| format!("__self_{}", i)).collect();
1061-
let no_deref = always_copy;
1074+
let addr_of = always_copy;
10621075
let selflike_fields =
1063-
trait_.create_struct_pattern_fields(cx, struct_def, &prefixes, no_deref);
1076+
trait_.create_struct_pattern_fields(cx, struct_def, &prefixes, addr_of);
10641077
let mut body = mk_body(cx, selflike_fields);
10651078

10661079
let struct_path = cx.path(span, vec![Ident::new(kw::SelfUpper, type_ident.span)]);
@@ -1194,9 +1207,9 @@ impl<'a> MethodDef<'a> {
11941207
// A single arm has form (&VariantK, &VariantK, ...) => BodyK
11951208
// (see "Final wrinkle" note below for why.)
11961209

1197-
let no_deref = false; // because enums can't be repr(packed)
1210+
let addr_of = false; // because enums can't be repr(packed)
11981211
let fields =
1199-
trait_.create_struct_pattern_fields(cx, &variant.data, &prefixes, no_deref);
1212+
trait_.create_struct_pattern_fields(cx, &variant.data, &prefixes, addr_of);
12001213

12011214
let sp = variant.span.with_ctxt(trait_.span.ctxt());
12021215
let variant_path = cx.path(sp, vec![type_ident, variant.ident]);
@@ -1512,15 +1525,15 @@ impl<'a> TraitDef<'a> {
15121525
cx: &mut ExtCtxt<'_>,
15131526
struct_def: &'a VariantData,
15141527
prefixes: &[String],
1515-
no_deref: bool,
1528+
addr_of: bool,
15161529
) -> Vec<FieldInfo> {
15171530
self.create_fields(struct_def, |i, _struct_field, sp| {
15181531
prefixes
15191532
.iter()
15201533
.map(|prefix| {
15211534
let ident = self.mk_pattern_ident(prefix, i);
15221535
let expr = cx.expr_path(cx.path_ident(sp, ident));
1523-
if no_deref { expr } else { cx.expr_deref(sp, expr) }
1536+
if addr_of { cx.expr_addr_of(sp, expr) } else { expr }
15241537
})
15251538
.collect()
15261539
})
@@ -1536,17 +1549,20 @@ impl<'a> TraitDef<'a> {
15361549
selflike_args
15371550
.iter()
15381551
.map(|selflike_arg| {
1539-
// Note: we must use `struct_field.span` rather than `span` in the
1552+
// Note: we must use `struct_field.span` rather than `sp` in the
15401553
// `unwrap_or_else` case otherwise the hygiene is wrong and we get
15411554
// "field `0` of struct `Point` is private" errors on tuple
15421555
// structs.
1543-
cx.expr(
1556+
cx.expr_addr_of(
15441557
sp,
1545-
ast::ExprKind::Field(
1546-
selflike_arg.clone(),
1547-
struct_field.ident.unwrap_or_else(|| {
1548-
Ident::from_str_and_span(&i.to_string(), struct_field.span)
1549-
}),
1558+
cx.expr(
1559+
sp,
1560+
ast::ExprKind::Field(
1561+
selflike_arg.clone(),
1562+
struct_field.ident.unwrap_or_else(|| {
1563+
Ident::from_str_and_span(&i.to_string(), struct_field.span)
1564+
}),
1565+
),
15501566
),
15511567
)
15521568
})

compiler/rustc_builtin_macros/src/deriving/hash.rs

+9-7
Original file line numberDiff line numberDiff line change
@@ -52,26 +52,28 @@ fn hash_substructure(
5252
let [state_expr] = substr.nonselflike_args else {
5353
cx.span_bug(trait_span, "incorrect number of arguments in `derive(Hash)`");
5454
};
55-
let call_hash = |span, thing_expr| {
55+
let call_hash = |span, expr| {
5656
let hash_path = {
5757
let strs = cx.std_path(&[sym::hash, sym::Hash, sym::hash]);
5858

5959
cx.expr_path(cx.path_global(span, strs))
6060
};
61-
let ref_thing = cx.expr_addr_of(span, thing_expr);
62-
let expr = cx.expr_call(span, hash_path, vec![ref_thing, state_expr.clone()]);
61+
let expr = cx.expr_call(span, hash_path, vec![expr, state_expr.clone()]);
6362
cx.stmt_expr(expr)
6463
};
6564
let mut stmts = Vec::new();
6665

6766
let fields = match substr.fields {
6867
Struct(_, fs) | EnumMatching(_, 1, .., fs) => fs,
6968
EnumMatching(.., fs) => {
70-
let variant_value = deriving::call_intrinsic(
71-
cx,
69+
let variant_value = cx.expr_addr_of(
7270
trait_span,
73-
sym::discriminant_value,
74-
vec![cx.expr_self(trait_span)],
71+
deriving::call_intrinsic(
72+
cx,
73+
trait_span,
74+
sym::discriminant_value,
75+
vec![cx.expr_self(trait_span)],
76+
),
7577
);
7678

7779
stmts.push(call_hash(trait_span, variant_value));

0 commit comments

Comments
 (0)