From 4982f913461a2a6dfbac8bace53961233322646a Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 7 Mar 2016 18:35:01 -0500 Subject: [PATCH 1/3] fix FIXME(#6449) in #[derive(PartialOrd, Ord)] This replaces some `if`s with `match`es. This was originally not possible because using a global path in a match statement caused a "non-constant path in constant expr" ICE. The issue is long since closed, though you still hit it (as an error now, not an ICE) if you try to generate match patterns using pat_lit(expr_path). But it works when constructing the patterns more carefully. --- src/libsyntax_ext/deriving/cmp/ord.rs | 50 +++++++++--------- src/libsyntax_ext/deriving/cmp/partial_ord.rs | 51 +++++++++---------- 2 files changed, 47 insertions(+), 54 deletions(-) diff --git a/src/libsyntax_ext/deriving/cmp/ord.rs b/src/libsyntax_ext/deriving/cmp/ord.rs index 2fa847ee430..f4082e0b123 100644 --- a/src/libsyntax_ext/deriving/cmp/ord.rs +++ b/src/libsyntax_ext/deriving/cmp/ord.rs @@ -73,36 +73,31 @@ pub fn cs_cmp(cx: &mut ExtCtxt, span: Span, /* Builds: - let __test = ::std::cmp::Ord::cmp(&self_field1, &other_field1); - if other == ::std::cmp::Ordering::Equal { - let __test = ::std::cmp::Ord::cmp(&self_field2, &other_field2); - if __test == ::std::cmp::Ordering::Equal { - ... - } else { - __test - } - } else { - __test + match ::std::cmp::Ord::cmp(&self_field1, &other_field1) { + ::std::cmp::Ordering::Equal => + match ::std::cmp::Ord::cmp(&self_field2, &other_field2) { + ::std::cmp::Ordering::Equal => { + ... + } + __test => __test + }, + __test => __test } - - FIXME #6449: These `if`s could/should be `match`es. */ cs_fold( // foldr nests the if-elses correctly, leaving the first field // as the outermost one, and the last as the innermost. false, |cx, span, old, self_f, other_fs| { - // let __test = new; - // if __test == ::std::cmp::Ordering::Equal { - // old - // } else { - // __test + // match new { + // ::std::cmp::Ordering::Equal => old, + // __test => __test // } let new = { let other_f = match (other_fs.len(), other_fs.get(0)) { (1, Some(o_f)) => o_f, - _ => cx.span_bug(span, "not exactly 2 arguments in `derive(PartialOrd)`"), + _ => cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`"), }; let args = vec![ @@ -113,20 +108,21 @@ pub fn cs_cmp(cx: &mut ExtCtxt, span: Span, cx.expr_call_global(span, cmp_path.clone(), args) }; - let assign = cx.stmt_let(span, false, test_id, new); + let eq_arm = cx.arm(span, + vec![cx.pat_enum(span, + equals_path.clone(), + vec![])], + old); + let neq_arm = cx.arm(span, + vec![cx.pat_ident(span, test_id)], + cx.expr_ident(span, test_id)); - let cond = cx.expr_binary(span, BinOpKind::Eq, - cx.expr_ident(span, test_id), - cx.expr_path(equals_path.clone())); - let if_ = cx.expr_if(span, - cond, - old, Some(cx.expr_ident(span, test_id))); - cx.expr_block(cx.block(span, vec!(assign), Some(if_))) + cx.expr_match(span, new, vec![eq_arm, neq_arm]) }, cx.expr_path(equals_path.clone()), Box::new(|cx, span, (self_args, tag_tuple), _non_self_args| { if self_args.len() != 2 { - cx.span_bug(span, "not exactly 2 arguments in `derives(Ord)`") + cx.span_bug(span, "not exactly 2 arguments in `derive(Ord)`") } else { ordering_collapsed(cx, span, tag_tuple) } diff --git a/src/libsyntax_ext/deriving/cmp/partial_ord.rs b/src/libsyntax_ext/deriving/cmp/partial_ord.rs index e857f7d52f9..3353addebc9 100644 --- a/src/libsyntax_ext/deriving/cmp/partial_ord.rs +++ b/src/libsyntax_ext/deriving/cmp/partial_ord.rs @@ -110,38 +110,33 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, let test_id = cx.ident_of("__test"); let ordering = cx.path_global(span, cx.std_path(&["cmp", "Ordering", "Equal"])); - let ordering = cx.expr_path(ordering); - let equals_expr = cx.expr_some(span, ordering); + let ordering_expr = cx.expr_path(ordering.clone()); + let equals_expr = cx.expr_some(span, ordering_expr); let partial_cmp_path = cx.std_path(&["cmp", "PartialOrd", "partial_cmp"]); /* Builds: - let __test = ::std::cmp::PartialOrd::partial_cmp(&self_field1, &other_field1); - if __test == ::std::option::Option::Some(::std::cmp::Ordering::Equal) { - let __test = ::std::cmp::PartialOrd::partial_cmp(&self_field2, &other_field2); - if __test == ::std::option::Option::Some(::std::cmp::Ordering::Equal) { - ... - } else { - __test - } - } else { - __test + match ::std::cmp::PartialOrd::partial_cmp(&self_field1, &other_field1) { + ::std::option::Option::Some(::std::cmp::Ordering::Equal) => + match ::std::cmp::PartialOrd::partial_cmp(&self_field2, &other_field2) { + ::std::option::Option::Some(::std::cmp::Ordering::Equal) => { + ... + } + __test => __test + }, + __test => __test } - - FIXME #6449: These `if`s could/should be `match`es. */ cs_fold( // foldr nests the if-elses correctly, leaving the first field // as the outermost one, and the last as the innermost. false, |cx, span, old, self_f, other_fs| { - // let __test = new; - // if __test == Some(::std::cmp::Ordering::Equal) { - // old - // } else { - // __test + // match new { + // Some(::std::cmp::Ordering::Equal) => old, + // __test => __test // } let new = { @@ -158,15 +153,17 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, cx.expr_call_global(span, partial_cmp_path.clone(), args) }; - let assign = cx.stmt_let(span, false, test_id, new); + let eq_arm = cx.arm(span, + vec![cx.pat_some(span, + cx.pat_enum(span, + ordering.clone(), + vec![]))], + old); + let neq_arm = cx.arm(span, + vec![cx.pat_ident(span, test_id)], + cx.expr_ident(span, test_id)); - let cond = cx.expr_binary(span, BinOpKind::Eq, - cx.expr_ident(span, test_id), - equals_expr.clone()); - let if_ = cx.expr_if(span, - cond, - old, Some(cx.expr_ident(span, test_id))); - cx.expr_block(cx.block(span, vec!(assign), Some(if_))) + cx.expr_match(span, new, vec![eq_arm, neq_arm]) }, equals_expr.clone(), Box::new(|cx, span, (self_args, tag_tuple), _non_self_args| { From fd4fa62885d7b7319a7cf88e834fa1016ac9ae5c Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 7 Mar 2016 19:07:06 -0500 Subject: [PATCH 2/3] derive: remove most __ strings FIXME(#2810) This changes local variable names in all derives to remove leading double-underscores. As far as I can tell, this doesn't break anything because there is no user code in these generated functions except for struct, field and type parameter names, and this doesn't cause shadowing of those. But I am still a bit nervous. --- src/libsyntax_ext/deriving/cmp/ord.rs | 8 +- src/libsyntax_ext/deriving/cmp/partial_ord.rs | 8 +- src/libsyntax_ext/deriving/generic/mod.rs | 80 +++++++++---------- 3 files changed, 48 insertions(+), 48 deletions(-) diff --git a/src/libsyntax_ext/deriving/cmp/ord.rs b/src/libsyntax_ext/deriving/cmp/ord.rs index f4082e0b123..a69d57423a2 100644 --- a/src/libsyntax_ext/deriving/cmp/ord.rs +++ b/src/libsyntax_ext/deriving/cmp/ord.rs @@ -64,7 +64,7 @@ pub fn ordering_collapsed(cx: &mut ExtCtxt, pub fn cs_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P { - let test_id = cx.ident_of("__test"); + let test_id = cx.ident_of("cmp"); let equals_path = cx.path_global(span, cx.std_path(&["cmp", "Ordering", "Equal"])); @@ -79,9 +79,9 @@ pub fn cs_cmp(cx: &mut ExtCtxt, span: Span, ::std::cmp::Ordering::Equal => { ... } - __test => __test + cmp => cmp }, - __test => __test + cmp => cmp } */ cs_fold( @@ -91,7 +91,7 @@ pub fn cs_cmp(cx: &mut ExtCtxt, span: Span, |cx, span, old, self_f, other_fs| { // match new { // ::std::cmp::Ordering::Equal => old, - // __test => __test + // cmp => cmp // } let new = { diff --git a/src/libsyntax_ext/deriving/cmp/partial_ord.rs b/src/libsyntax_ext/deriving/cmp/partial_ord.rs index 3353addebc9..b3864a6c2e7 100644 --- a/src/libsyntax_ext/deriving/cmp/partial_ord.rs +++ b/src/libsyntax_ext/deriving/cmp/partial_ord.rs @@ -107,7 +107,7 @@ pub fn some_ordering_collapsed(cx: &mut ExtCtxt, pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, substr: &Substructure) -> P { - let test_id = cx.ident_of("__test"); + let test_id = cx.ident_of("cmp"); let ordering = cx.path_global(span, cx.std_path(&["cmp", "Ordering", "Equal"])); let ordering_expr = cx.expr_path(ordering.clone()); @@ -124,9 +124,9 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, ::std::option::Option::Some(::std::cmp::Ordering::Equal) => { ... } - __test => __test + cmp => cmp }, - __test => __test + cmp => cmp } */ cs_fold( @@ -136,7 +136,7 @@ pub fn cs_partial_cmp(cx: &mut ExtCtxt, span: Span, |cx, span, old, self_f, other_fs| { // match new { // Some(::std::cmp::Ordering::Equal) => old, - // __test => __test + // cmp => cmp // } let new = { diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index c0237a5d29a..f72fa98b482 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -156,14 +156,14 @@ //! //! ```{.text} //! EnumNonMatchingCollapsed( -//! vec![, ], +//! vec![, ], //! &[, ], -//! &[, ]) +//! &[, ]) //! ``` //! //! It is the same for when the arguments are flipped to `C1 {x}` and //! `C0(a)`; the only difference is what the values of the identifiers -//! and will +//! and will //! be in the generated code. //! //! `EnumNonMatchingCollapsed` deliberately provides far less information @@ -826,7 +826,7 @@ impl<'a> MethodDef<'a> { for (i, ty) in self.args.iter().enumerate() { 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(&format!("arg_{}", i)); arg_tys.push((ident, ast_ty)); let arg_expr = cx.expr_ident(trait_.span, ident); @@ -911,12 +911,12 @@ impl<'a> MethodDef<'a> { /// /// // equivalent to: /// impl PartialEq for A { - /// fn eq(&self, __arg_1: &A) -> bool { + /// fn eq(&self, arg_1: &A) -> bool { /// match *self { - /// A {x: ref __self_0_0, y: ref __self_0_1} => { - /// match *__arg_1 { - /// 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) + /// A {x: ref self_0_0, y: ref self_0_1} => { + /// match *arg_1 { + /// 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) /// } /// } /// } @@ -942,7 +942,7 @@ impl<'a> MethodDef<'a> { trait_.create_struct_pattern(cx, struct_path, struct_def, - &format!("__self_{}", + &format!("self_{}", i), ast::Mutability::Immutable); patterns.push(pat); @@ -1020,14 +1020,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, arg_1: &A) -> ::bool { + /// match (&*self, &*arg_1) { /// (&A1, &A1) => true, - /// (&A2(ref __self_0), - /// &A2(ref __arg_1_0)) => (*__self_0).eq(&(*__arg_1_0)), + /// (&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 self_vi = match *self { A1(..) => 0, A2(..) => 1 }; + /// let arg_1_vi = match *arg_1 { A1(..) => 0, A2(..) => 1 }; /// false /// } /// } @@ -1035,10 +1035,10 @@ impl<'a> MethodDef<'a> { /// } /// ``` /// - /// (Of course `__self_vi` and `__arg_1_vi` are unused for + /// (Of course `self_vi` and `arg_1_vi` are unused for /// `PartialEq`, and those subcomputations will hopefully be removed - /// as their results are unused. The point of `__self_vi` and - /// `__arg_1_vi` is for `PartialOrd`; see #15503.) + /// as their results are unused. The point of `self_vi` and + /// `arg_1_vi` is for `PartialOrd`; see #15503.) fn expand_enum_method_body<'b>(&self, cx: &mut ExtCtxt, trait_: &TraitDef<'b>, @@ -1069,14 +1069,14 @@ impl<'a> MethodDef<'a> { /// for each of the self-args, carried in precomputed variables. /// ```{.text} - /// let __self0_vi = unsafe { + /// let self0_vi = unsafe { /// std::intrinsics::discriminant_value(&self) } as i32; - /// let __self1_vi = unsafe { - /// std::intrinsics::discriminant_value(&__arg1) } as i32; - /// let __self2_vi = unsafe { - /// std::intrinsics::discriminant_value(&__arg2) } as i32; + /// let self1_vi = unsafe { + /// std::intrinsics::discriminant_value(&arg1) } as i32; + /// let self2_vi = unsafe { + /// std::intrinsics::discriminant_value(&arg2) } as i32; /// - /// if __self0_vi == __self1_vi && __self0_vi == __self2_vi && ... { + /// if self0_vi == self1_vi && self0_vi == self2_vi && ... { /// match (...) { /// (Variant1, Variant1, ...) => Body1 /// (Variant2, Variant2, ...) => Body2, @@ -1104,9 +1104,9 @@ impl<'a> MethodDef<'a> { let self_arg_names = self_args.iter().enumerate() .map(|(arg_count, _self_arg)| { if arg_count == 0 { - "__self".to_string() + "self".to_string() } else { - format!("__arg_{}", arg_count) + format!("arg_{}", arg_count) } }) .collect::>(); @@ -1243,17 +1243,17 @@ impl<'a> MethodDef<'a> { // with three Self args, builds three statements: // // ``` - // let __self0_vi = unsafe { + // let self0_vi = unsafe { // std::intrinsics::discriminant_value(&self) } as i32; - // let __self1_vi = unsafe { - // std::intrinsics::discriminant_value(&__arg1) } as i32; - // let __self2_vi = unsafe { - // std::intrinsics::discriminant_value(&__arg2) } as i32; + // let self1_vi = unsafe { + // std::intrinsics::discriminant_value(&arg1) } as i32; + // let self2_vi = unsafe { + // std::intrinsics::discriminant_value(&arg2) } as i32; // ``` let mut index_let_stmts: Vec = Vec::new(); //We also build an expression which checks whether all discriminants are equal - // discriminant_test = __self0_vi == __self1_vi && __self0_vi == __self2_vi && ... + // discriminant_test = self0_vi == self1_vi && self0_vi == self2_vi && ... let mut discriminant_test = cx.expr_bool(sp, true); let target_type_name = @@ -1312,7 +1312,7 @@ impl<'a> MethodDef<'a> { // down to desired l-values, but we cannot actually deref // them when they are fed as r-values into a tuple // expression; here add a layer of borrowing, turning - // `(*self, *__arg_0, ...)` into `(&*self, &*__arg_0, ...)`. + // `(*self, *arg_0, ...)` into `(&*self, &*arg_0, ...)`. let borrowed_self_args = self_args.move_map(|self_arg| cx.expr_addr_of(sp, self_arg)); let match_arg = cx.expr(sp, ast::ExprKind::Tup(borrowed_self_args)); @@ -1326,7 +1326,7 @@ impl<'a> MethodDef<'a> { // } // } // else { - // + // // } let all_match = cx.expr_match(sp, match_arg, match_arms); let arm_expr = cx.expr_if(sp, discriminant_test, all_match, Some(arm_expr)); @@ -1350,8 +1350,8 @@ impl<'a> MethodDef<'a> { // error-prone, since the catch-all as defined above would // generate code like this: // - // _ => { let __self0 = match *self { }; - // let __self1 = match *__arg_0 { }; + // _ => { let self0 = match *self { }; + // let self1 = match *arg_0 { }; // } // // Which is yields bindings for variables which type @@ -1390,7 +1390,7 @@ impl<'a> MethodDef<'a> { // down to desired l-values, but we cannot actually deref // them when they are fed as r-values into a tuple // expression; here add a layer of borrowing, turning - // `(*self, *__arg_0, ...)` into `(&*self, &*__arg_0, ...)`. + // `(*self, *arg_0, ...)` into `(&*self, &*arg_0, ...)`. let borrowed_self_args = self_args.move_map(|self_arg| cx.expr_addr_of(sp, self_arg)); let match_arg = cx.expr(sp, ast::ExprKind::Tup(borrowed_self_args)); cx.expr_match(sp, match_arg, match_arms) @@ -1604,8 +1604,8 @@ pub fn cs_fold(use_foldl: bool, /// process the collected results. i.e. /// /// ```ignore -/// f(cx, span, vec![self_1.method(__arg_1_1, __arg_2_1), -/// self_2.method(__arg_1_2, __arg_2_2)]) +/// f(cx, span, vec![self_1.method(arg_1_1, arg_2_1), +/// self_2.method(arg_1_2, arg_2_2)]) /// ``` #[inline] pub fn cs_same_method(f: F, From 8355389e3e9d299d90ea78197c0e5b6c2162a957 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Tue, 8 Mar 2016 13:24:28 -0500 Subject: [PATCH 3/3] derive: improve hygiene for type parameters (see #2810) When deriving Hash, RustcEncodable and RustcDecodable, the syntax extension needs a type parameter to use in the inner method. They used to use __H, __S and __D respectively. If this conflicts with a type parameter already declared for the item, bad times result (see the test). There is no hygiene for type parameters, but this commit introduces a better heuristic by concatenating the names of all extant type parameters (and prepending __H). --- src/libsyntax_ext/deriving/cmp/ord.rs | 2 +- src/libsyntax_ext/deriving/decodable.rs | 12 +++++----- src/libsyntax_ext/deriving/encodable.rs | 12 +++++----- src/libsyntax_ext/deriving/hash.rs | 8 +++++-- src/libsyntax_ext/deriving/mod.rs | 29 +++++++++++++++++++++---- src/test/run-pass/deriving-hash.rs | 4 ++++ 6 files changed, 50 insertions(+), 17 deletions(-) diff --git a/src/libsyntax_ext/deriving/cmp/ord.rs b/src/libsyntax_ext/deriving/cmp/ord.rs index a69d57423a2..a7e156c5f68 100644 --- a/src/libsyntax_ext/deriving/cmp/ord.rs +++ b/src/libsyntax_ext/deriving/cmp/ord.rs @@ -11,7 +11,7 @@ use deriving::generic::*; use deriving::generic::ty::*; -use syntax::ast::{MetaItem, Expr, BinOpKind, self}; +use syntax::ast::{MetaItem, Expr, self}; use syntax::codemap::Span; use syntax::ext::base::{ExtCtxt, Annotatable}; use syntax::ext::build::AstBuilder; diff --git a/src/libsyntax_ext/deriving/decodable.rs b/src/libsyntax_ext/deriving/decodable.rs index 092f8548966..49f14c937e9 100644 --- a/src/libsyntax_ext/deriving/decodable.rs +++ b/src/libsyntax_ext/deriving/decodable.rs @@ -10,6 +10,7 @@ //! The compiler code necessary for `#[derive(Decodable)]`. See encodable.rs for more. +use deriving; use deriving::generic::*; use deriving::generic::ty::*; @@ -54,6 +55,8 @@ fn expand_deriving_decodable_imp(cx: &mut ExtCtxt, return } + let typaram = &*deriving::hygienic_type_parameter(item, "__D"); + let trait_def = TraitDef { span: span, attributes: Vec::new(), @@ -66,18 +69,17 @@ fn expand_deriving_decodable_imp(cx: &mut ExtCtxt, name: "decode", generics: LifetimeBounds { lifetimes: Vec::new(), - bounds: vec!(("__D", vec!(Path::new_( - vec!(krate, "Decoder"), None, - vec!(), true)))) + bounds: vec![(typaram, + vec![Path::new_(vec!(krate, "Decoder"), None, vec!(), true)])] }, explicit_self: None, - args: vec!(Ptr(Box::new(Literal(Path::new_local("__D"))), + args: vec!(Ptr(Box::new(Literal(Path::new_local(typaram))), Borrowed(None, Mutability::Mutable))), ret_ty: Literal(Path::new_( pathvec_std!(cx, core::result::Result), None, vec!(Box::new(Self_), Box::new(Literal(Path::new_( - vec!["__D", "Error"], None, vec![], false + vec![typaram, "Error"], None, vec![], false )))), true )), diff --git a/src/libsyntax_ext/deriving/encodable.rs b/src/libsyntax_ext/deriving/encodable.rs index 8262a04e9ce..a05bd7869b2 100644 --- a/src/libsyntax_ext/deriving/encodable.rs +++ b/src/libsyntax_ext/deriving/encodable.rs @@ -88,6 +88,7 @@ //! } //! ``` +use deriving; use deriving::generic::*; use deriving::generic::ty::*; @@ -130,6 +131,8 @@ fn expand_deriving_encodable_imp(cx: &mut ExtCtxt, return; } + let typaram = &*deriving::hygienic_type_parameter(item, "__S"); + let trait_def = TraitDef { span: span, attributes: Vec::new(), @@ -142,18 +145,17 @@ fn expand_deriving_encodable_imp(cx: &mut ExtCtxt, name: "encode", generics: LifetimeBounds { lifetimes: Vec::new(), - bounds: vec!(("__S", vec!(Path::new_( - vec!(krate, "Encoder"), None, - vec!(), true)))) + bounds: vec![(typaram, + vec![Path::new_(vec![krate, "Encoder"], None, vec!(), true)])] }, explicit_self: borrowed_explicit_self(), - args: vec!(Ptr(Box::new(Literal(Path::new_local("__S"))), + args: vec!(Ptr(Box::new(Literal(Path::new_local(typaram))), Borrowed(None, Mutability::Mutable))), ret_ty: Literal(Path::new_( pathvec_std!(cx, core::result::Result), None, vec!(Box::new(Tuple(Vec::new())), Box::new(Literal(Path::new_( - vec!["__S", "Error"], None, vec![], false + vec![typaram, "Error"], None, vec![], false )))), true )), diff --git a/src/libsyntax_ext/deriving/hash.rs b/src/libsyntax_ext/deriving/hash.rs index bf8aa8fb23d..ba38ebc8607 100644 --- a/src/libsyntax_ext/deriving/hash.rs +++ b/src/libsyntax_ext/deriving/hash.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use deriving; use deriving::generic::*; use deriving::generic::ty::*; @@ -26,7 +27,10 @@ pub fn expand_deriving_hash(cx: &mut ExtCtxt, let path = Path::new_(pathvec_std!(cx, core::hash::Hash), None, vec!(), true); - let arg = Path::new_local("__H"); + + let typaram = &*deriving::hygienic_type_parameter(item, "__H"); + + let arg = Path::new_local(typaram); let hash_trait_def = TraitDef { span: span, attributes: Vec::new(), @@ -39,7 +43,7 @@ pub fn expand_deriving_hash(cx: &mut ExtCtxt, name: "hash", generics: LifetimeBounds { lifetimes: Vec::new(), - bounds: vec![("__H", + bounds: vec![(typaram, vec![path_std!(cx, core::hash::Hasher)])], }, explicit_self: borrowed_explicit_self(), diff --git a/src/libsyntax_ext/deriving/mod.rs b/src/libsyntax_ext/deriving/mod.rs index 4e2142f1fb4..75de5c56ea1 100644 --- a/src/libsyntax_ext/deriving/mod.rs +++ b/src/libsyntax_ext/deriving/mod.rs @@ -9,11 +9,8 @@ // except according to those terms. //! The compiler code necessary to implement the `#[derive]` extensions. -//! -//! FIXME (#2810): hygiene. Search for "__" strings (in other files too). We also assume "extra" is -//! the standard library, and "std" is the core library. -use syntax::ast::{MetaItem, MetaItemKind}; +use syntax::ast::{MetaItem, MetaItemKind, self}; use syntax::attr::AttrMetaMethods; use syntax::ext::base::{ExtCtxt, SyntaxEnv, Annotatable}; use syntax::ext::base::{MultiDecorator, MultiItemDecorator, MultiModifier}; @@ -197,3 +194,27 @@ fn warn_if_deprecated(ecx: &mut ExtCtxt, sp: Span, name: &str) { name, replacement)); } } + +/// Construct a name for the inner type parameter that can't collide with any type parameters of +/// the item. This is achieved by starting with a base and then concatenating the names of all +/// other type parameters. +// FIXME(aburka): use real hygiene when that becomes possible +fn hygienic_type_parameter(item: &Annotatable, base: &str) -> String { + let mut typaram = String::from(base); + if let Annotatable::Item(ref item) = *item { + match item.node { + ast::ItemKind::Struct(_, ast::Generics { ref ty_params, .. }) | + ast::ItemKind::Enum(_, ast::Generics { ref ty_params, .. }) => { + + for ty in ty_params.iter() { + typaram.push_str(&ty.ident.name.as_str()); + } + } + + _ => {} + } + } + + typaram +} + diff --git a/src/test/run-pass/deriving-hash.rs b/src/test/run-pass/deriving-hash.rs index 69e9816ab94..a98cfa2393f 100644 --- a/src/test/run-pass/deriving-hash.rs +++ b/src/test/run-pass/deriving-hash.rs @@ -20,6 +20,10 @@ struct Person { phone: usize, } +// test for hygiene name collisions +#[derive(Hash)] struct __H__H; +#[derive(Hash)] enum Collision<__H> { __H { __H__H: __H } } + fn hash(t: &T) -> u64 { let mut s = SipHasher::new_with_keys(0, 0); t.hash(&mut s);