OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #49986 - zofrex:better-derived-argument-names, r=Manishearth

Browse Source 
Provide better names for builtin deriving-generated attributes

First attempt at fixing #49967

Not in love with any choices here, don't be shy if you aren't happy with anything :)

I've tested that this produces nicer names in documentation, and that it no longer has issues conflicting with constants with the same name. (I guess we _could_ make a test for that... unsure if that would be valuable)

In all cases I took the names from the methods as declared in the relevant trait.

In some cases I had to prepend the names with _ otherwise there were errors about un-used variables. I'm uneasy with the inconsistency... do they all need to be like that? Is there a way to generate an alternate impl or use a different name (`_`?) in the cases where the arguments are not used?

Lastly the gensym addition to Ident I implemented largely as suggested, but I want to point out it's a little circuitous (at least, as far as I understand it). `cx.ident_of(name)` is just `Ident::from_str`, so we create an Ident then another Ident from it. `Ident::with_empty_ctxt(Symbol::gensym(string))` may or may not be equivalent, I don't know if it's important to intern it _then_ gensym it. It seems like either we could use that, or if we do want a new method to make this convenient, it could be on Ident instead (`from_str_gensymed`?)
This commit is contained in:
bors 2018-04-25 01:50:56 +00:00
commit 0c5740feb2
14 changed files with 97 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/librustc/middle/liveness.rs
View File

@ -184,6 +184,7 @@ impl<'a, 'tcx> Visitor<'tcx> for IrMaps<'a, 'tcx> {
b: hir::BodyId, s: Span, id: NodeId) {
visit_fn(self, fk, fd, b, s, id);
}
fn visit_local(&mut self, l: &'tcx hir::Local) { visit_local(self, l); }
fn visit_expr(&mut self, ex: &'tcx Expr) { visit_expr(self, ex); }
fn visit_arm(&mut self, a: &'tcx hir::Arm) { visit_arm(self, a); }
@ -361,6 +362,16 @@ fn visit_fn<'a, 'tcx: 'a>(ir: &mut IrMaps<'a, 'tcx>,
// swap in a new set of IR maps for this function body:
let mut fn_maps = IrMaps::new(ir.tcx);
// Don't run unused pass for #[derive()]
if let FnKind::Method(..) = fk {
let parent = ir.tcx.hir.get_parent(id);
if let Some(hir::map::Node::NodeItem(i)) = ir.tcx.hir.find(parent) {
if i.attrs.iter().any(|a| a.check_name("automatically_derived")) {
return;
}
}
}
debug!("creating fn_maps: {:?}", &fn_maps as *const IrMaps);
let body = ir.tcx.hir.body(body_id);

10
src/libsyntax_ext/deriving/cmp/ord.rs
View File

@ -38,7 +38,7 @@ pub fn expand_deriving_ord(cx: &mut ExtCtxt,
name: "cmp",
generics: LifetimeBounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![borrowed_self()],
args: vec![(borrowed_self(), "other")],
ret_ty: Literal(path_std!(cx, cmp::Ordering)),
attributes: attrs,
is_unsafe: false,
@ -64,7 +64,7 @@ pub fn ordering_collapsed(cx: &mut ExtCtxt,
}
pub fn cs_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P<Expr> {
let test_id = cx.ident_of("__cmp");
let test_id = cx.ident_of("cmp").gensym();
let equals_path = cx.path_global(span, cx.std_path(&["cmp", "Ordering", "Equal"]));
let cmp_path = cx.std_path(&["cmp", "Ord", "cmp"]);
@ -77,9 +77,9 @@ pub fn cs_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P<Expr> {
// ::std::cmp::Ordering::Equal => {
// ...
// }
// __cmp => __cmp
// cmp => cmp
// },
// __cmp => __cmp
// cmp => cmp
// }
//
cs_fold(// foldr nests the if-elses correctly, leaving the first field
@ -88,7 +88,7 @@ pub fn cs_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P<Expr> {
|cx, span, old, self_f, other_fs| {
// match new {
// ::std::cmp::Ordering::Equal => old,
// __cmp => __cmp
// cmp => cmp
// }
let new = {

2
src/libsyntax_ext/deriving/cmp/partial_eq.rs
View File

@ -78,7 +78,7 @@ pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt,
name: $name,
generics: LifetimeBounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![borrowed_self()],
args: vec![(borrowed_self(), "other")],
ret_ty: Literal(path_local!(bool)),
attributes: attrs,
is_unsafe: false,

12
src/libsyntax_ext/deriving/cmp/partial_ord.rs
View File

@ -34,7 +34,7 @@ pub fn expand_deriving_partial_ord(cx: &mut ExtCtxt,
name: $name,
generics: LifetimeBounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![borrowed_self()],
args: vec![(borrowed_self(), "other")],
ret_ty: Literal(path_local!(bool)),
attributes: attrs,
is_unsafe: false,
@ -59,7 +59,7 @@ pub fn expand_deriving_partial_ord(cx: &mut ExtCtxt,
name: "partial_cmp",
generics: LifetimeBounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![borrowed_self()],
args: vec![(borrowed_self(), "other")],
ret_ty,
attributes: attrs,
is_unsafe: false,
@ -123,7 +123,7 @@ pub fn some_ordering_collapsed(cx: &mut ExtCtxt,
}
pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P<Expr> {
let test_id = cx.ident_of("__cmp");
let test_id = cx.ident_of("cmp").gensym();
let ordering = cx.path_global(span, cx.std_path(&["cmp", "Ordering", "Equal"]));
let ordering_expr = cx.expr_path(ordering.clone());
let equals_expr = cx.expr_some(span, ordering_expr);
@ -138,9 +138,9 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P<
// ::std::option::Option::Some(::std::cmp::Ordering::Equal) => {
// ...
// }
// __cmp => __cmp
// cmp => cmp
// },
// __cmp => __cmp
// cmp => cmp
// }
//
cs_fold(// foldr nests the if-elses correctly, leaving the first field
@ -149,7 +149,7 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P<
|cx, span, old, self_f, other_fs| {
// match new {
// Some(::std::cmp::Ordering::Equal) => old,
// __cmp => __cmp
// cmp => cmp
// }
let new = {

4
src/libsyntax_ext/deriving/debug.rs
View File

@ -40,7 +40,7 @@ pub fn expand_deriving_debug(cx: &mut ExtCtxt,
name: "fmt",
generics: LifetimeBounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![fmtr],
args: vec![(fmtr, "f")],
ret_ty: Literal(path_std!(cx, fmt::Result)),
attributes: Vec::new(),
is_unsafe: false,
@ -70,7 +70,7 @@ fn show_substructure(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P<E
// We want to make sure we have the ctxt set so that we can use unstable methods
let span = span.with_ctxt(cx.backtrace());
let name = cx.expr_lit(span, ast::LitKind::Str(ident.name, ast::StrStyle::Cooked));
let builder = Ident::from_str("__debug_trait_builder");
let builder = Ident::from_str("debug_trait_builder").gensym();
let builder_expr = cx.expr_ident(span, builder.clone());
let fmt = substr.nonself_args[0].clone();

4
src/libsyntax_ext/deriving/decodable.rs
View File

@ -67,8 +67,8 @@ fn expand_deriving_decodable_imp(cx: &mut ExtCtxt,
PathKind::Global)])],
},
explicit_self: None,
args: vec![Ptr(Box::new(Literal(Path::new_local(typaram))),
Borrowed(None, Mutability::Mutable))],
args: vec![(Ptr(Box::new(Literal(Path::new_local(typaram))),
Borrowed(None, Mutability::Mutable)), "d")],
ret_ty:
Literal(Path::new_(pathvec_std!(cx, result::Result),
None,

4
src/libsyntax_ext/deriving/encodable.rs
View File

@ -148,8 +148,8 @@ fn expand_deriving_encodable_imp(cx: &mut ExtCtxt,
],
},
explicit_self: borrowed_explicit_self(),
args: vec![Ptr(Box::new(Literal(Path::new_local(typaram))),
Borrowed(None, Mutability::Mutable))],
args: vec![(Ptr(Box::new(Literal(Path::new_local(typaram))),
Borrowed(None, Mutability::Mutable)), "s")],
ret_ty: Literal(Path::new_(
pathvec_std!(cx, result::Result),
None,

24
src/libsyntax_ext/deriving/generic/mod.rs
View File

@ -252,7 +252,7 @@ pub struct MethodDef<'a> {
pub explicit_self: Option<Option<PtrTy<'a>>>,
/// Arguments other than the self argument
pub args: Vec<Ty<'a>>,
pub args: Vec<(Ty<'a>, &'a str)>,
/// Return type
pub ret_ty: Ty<'a>,
@ -915,9 +915,9 @@ impl<'a> MethodDef<'a> {
explicit_self
});
for (i, ty) in self.args.iter().enumerate() {
for (ty, name) in self.args.iter() {
let ast_ty = ty.to_ty(cx, trait_.span, type_ident, generics);
let ident = cx.ident_of(&format!("__arg_{}", i));
let ident = cx.ident_of(name).gensym();
arg_tys.push((ident, ast_ty));
let arg_expr = cx.expr_ident(trait_.span, ident);
@ -1004,10 +1004,10 @@ impl<'a> MethodDef<'a> {
///
/// // equivalent to:
/// impl PartialEq for A {
/// fn eq(&self, __arg_1: &A) -> bool {
/// fn eq(&self, other: &A) -> bool {
/// match *self {
/// A {x: ref __self_0_0, y: ref __self_0_1} => {
/// match *__arg_1 {
/// match *other {
/// A {x: ref __self_1_0, y: ref __self_1_1} => {
/// __self_0_0.eq(__self_1_0) && __self_0_1.eq(__self_1_1)
/// }
@ -1020,10 +1020,10 @@ impl<'a> MethodDef<'a> {
/// // or if A is repr(packed) - note fields are matched by-value
/// // instead of by-reference.
/// impl PartialEq for A {
/// fn eq(&self, __arg_1: &A) -> bool {
/// fn eq(&self, other: &A) -> bool {
/// match *self {
/// A {x: __self_0_0, y: __self_0_1} => {
/// match __arg_1 {
/// match other {
/// A {x: __self_1_0, y: __self_1_1} => {
/// __self_0_0.eq(&__self_1_0) && __self_0_1.eq(&__self_1_1)
/// }
@ -1134,14 +1134,14 @@ impl<'a> MethodDef<'a> {
/// // is equivalent to
///
/// impl PartialEq for A {
/// fn eq(&self, __arg_1: &A) -> ::bool {
/// match (&*self, &*__arg_1) {
/// fn eq(&self, other: &A) -> ::bool {
/// match (&*self, &*other) {
/// (&A1, &A1) => true,
/// (&A2(ref self_0),
/// &A2(ref __arg_1_0)) => (*self_0).eq(&(*__arg_1_0)),
/// _ => {
/// let __self_vi = match *self { A1(..) => 0, A2(..) => 1 };
/// let __arg_1_vi = match *__arg_1 { A1(..) => 0, A2(..) => 1 };
/// let __arg_1_vi = match *other { A1(..) => 0, A2(..) => 1 };
/// false
/// }
/// }
@ -1240,7 +1240,7 @@ impl<'a> MethodDef<'a> {
let vi_idents: Vec<ast::Ident> = self_arg_names.iter()
.map(|name| {
let vi_suffix = format!("{}_vi", &name[..]);
cx.ident_of(&vi_suffix[..])
cx.ident_of(&vi_suffix[..]).gensym()
})
.collect::<Vec<ast::Ident>>();
@ -1616,7 +1616,7 @@ impl<'a> TraitDef<'a> {
let mut ident_exprs = Vec::new();
for (i, struct_field) in struct_def.fields().iter().enumerate() {
let sp = struct_field.span.with_ctxt(self.span.ctxt());
let ident = cx.ident_of(&format!("{}_{}", prefix, i));
let ident = cx.ident_of(&format!("{}_{}", prefix, i)).gensym();
paths.push(ident.with_span_pos(sp));
let val = cx.expr_path(cx.path_ident(sp, ident));
let val = if use_temporaries {

4
src/libsyntax_ext/deriving/hash.rs
View File

@ -44,8 +44,8 @@ pub fn expand_deriving_hash(cx: &mut ExtCtxt,
bounds: vec![(typaram, vec![path_std!(cx, hash::Hasher)])],
},
explicit_self: borrowed_explicit_self(),
args: vec![Ptr(Box::new(Literal(arg)),
Borrowed(None, Mutability::Mutable))],
args: vec![(Ptr(Box::new(Literal(arg)),
Borrowed(None, Mutability::Mutable)), "state")],
ret_ty: nil_ty(),
attributes: vec![],
is_unsafe: false,

16
src/libsyntax_ext/format.rs
View File

@ -543,6 +543,10 @@ impl<'a, 'b> Context<'a, 'b> {
let mut pats = Vec::new();
let mut heads = Vec::new();
let names_pos: Vec<_> = (0..self.args.len()).map(|i| {
self.ecx.ident_of(&format!("arg{}", i)).gensym()
}).collect();
// 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);
@ -560,7 +564,7 @@ impl<'a, 'b> Context<'a, 'b> {
// of each variable because we don't want to move out of the arguments
// passed to this function.
for (i, e) in self.args.into_iter().enumerate() {
let name = self.ecx.ident_of(&format!("__arg{}", i));
let name = names_pos[i];
let span =
DUMMY_SP.with_ctxt(e.span.ctxt().apply_mark(self.ecx.current_expansion.mark));
pats.push(self.ecx.pat_ident(span, name));
@ -570,14 +574,12 @@ impl<'a, 'b> Context<'a, 'b> {
heads.push(self.ecx.expr_addr_of(e.span, e));
}
for pos in self.count_args {
let name = self.ecx.ident_of(&match pos {
Exact(i) => format!("__arg{}", i),
_ => panic!("should never happen"),
});
let span = match pos {
Exact(i) => spans_pos[i],
let index = match pos {
Exact(i) => i,
_ => panic!("should never happen"),
};
let name = names_pos[index];
let span = spans_pos[index];
counts.push(Context::format_arg(self.ecx, self.macsp, span, &Count, name));
}

4
src/libsyntax_pos/symbol.rs
View File

@ -53,6 +53,10 @@ impl Ident {
pub fn modern(self) -> Ident {
Ident::new(self.name, self.span.modern())
}
pub fn gensym(self) -> Ident {
Ident::new(self.name.gensymed(), self.span)
}
}
impl PartialEq for Ident {

2
src/test/run-pass-fulldeps/auxiliary/custom_derive_partial_eq.rs
View File

@ -58,7 +58,7 @@ fn expand_deriving_partial_eq(cx: &mut ExtCtxt, span: Span, mitem: &MetaItem, it
name: "eq",
generics: LifetimeBounds::empty(),
explicit_self: borrowed_explicit_self(),
args: vec![borrowed_self()],
args: vec![(borrowed_self(), "other")],
ret_ty: Literal(deriving::generic::ty::Path::new_local("bool")),
attributes: attrs,
is_unsafe: false,

25
src/test/run-pass-fulldeps/deriving-hygiene.rs Normal file
View File

@ -0,0 +1,25 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(rustc_private)]
extern crate serialize;
pub const other: u8 = 1;
pub const f: u8 = 1;
pub const d: u8 = 1;
pub const s: u8 = 1;
pub const state: u8 = 1;
pub const cmp: u8 = 1;
#[derive(Ord,Eq,PartialOrd,PartialEq,Debug,Decodable,Encodable,Hash)]
struct Foo {}
fn main() {
}

15
src/test/run-pass/format-hygiene.rs Normal file
View File

@ -0,0 +1,15 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
pub const arg0: u8 = 1;
pub fn main() {
format!("{}", 1);
}