From 9f1490d83f1cde688c14ab6afe8ef0439d9652da Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Wed, 2 Mar 2016 00:58:04 -0500 Subject: [PATCH 1/9] refactor derive-no-std test, add empty struct/enum --- .../derive-no-std.rs | 29 ++++++++++--------- src/test/run-pass/derive-no-std.rs | 22 ++++++++++++++ 2 files changed, 37 insertions(+), 14 deletions(-) rename src/test/{run-pass-fulldeps => auxiliary}/derive-no-std.rs (64%) create mode 100644 src/test/run-pass/derive-no-std.rs diff --git a/src/test/run-pass-fulldeps/derive-no-std.rs b/src/test/auxiliary/derive-no-std.rs similarity index 64% rename from src/test/run-pass-fulldeps/derive-no-std.rs rename to src/test/auxiliary/derive-no-std.rs index 78e9da001f799..f083e10bfdb32 100644 --- a/src/test/run-pass-fulldeps/derive-no-std.rs +++ b/src/test/auxiliary/derive-no-std.rs @@ -8,32 +8,33 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(rand, collections, rustc_private)] -#![no_std] +// no-prefer-dynamic -extern crate rand; -extern crate serialize as rustc_serialize; -extern crate collections; +#![crate_type = "rlib"] +#![no_std] // Issue #16803 #[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Debug, Default, Copy)] -struct Foo { - x: u32, +pub struct Foo { + pub x: u32, } #[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Debug, Copy)] -enum Bar { +pub enum Bar { Qux, Quux(u32), } -enum Baz { A=0, B=5, } +#[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord, + Debug, Copy)] +pub enum Void {} +#[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord, + Debug, Copy)] +pub struct Empty; +#[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord, + Debug, Copy)] +pub struct AlsoEmpty {} -fn main() { - Foo { x: 0 }; - Bar::Quux(3); - Baz::A; -} diff --git a/src/test/run-pass/derive-no-std.rs b/src/test/run-pass/derive-no-std.rs new file mode 100644 index 0000000000000..0cbe4f4ebd0ed --- /dev/null +++ b/src/test/run-pass/derive-no-std.rs @@ -0,0 +1,22 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// aux-build:derive-no-std.rs + +extern crate derive_no_std; +use derive_no_std::*; + +fn main() { + let f = Foo { x: 0 }; + assert_eq!(f.clone(), Foo::default()); + + assert!(Bar::Qux < Bar::Quux(42)); +} + From ebc60b8ce02da2aebd8eb864f3ee691b13a8d8d3 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Thu, 3 Mar 2016 11:45:58 -0500 Subject: [PATCH 2/9] derive: emit intrinsics::unreachable for impls on empty enums fixes #31574 --- src/libsyntax_ext/deriving/generic/mod.rs | 29 ++++++++++++++--------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index c0237a5d29a41..3896a7ce71462 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -381,6 +381,22 @@ fn find_type_parameters(ty: &ast::Ty, ty_param_names: &[ast::Name]) -> Vec P { + let path = cx.std_path(&["intrinsics", "unreachable"]); + let call = cx.expr_call_global( + sp, path, vec![]); + let unreachable = cx.expr_block(P(ast::Block { + stmts: vec![], + expr: Some(call), + id: ast::DUMMY_NODE_ID, + rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated), + span: sp })); + + unreachable +} + impl<'a> TraitDef<'a> { pub fn expand(&self, cx: &mut ExtCtxt, @@ -1297,16 +1313,7 @@ impl<'a> MethodDef<'a> { //Since we know that all the arguments will match if we reach the match expression we //add the unreachable intrinsics as the result of the catch all which should help llvm //in optimizing it - let path = cx.std_path(&["intrinsics", "unreachable"]); - let call = cx.expr_call_global( - sp, path, vec![]); - let unreachable = cx.expr_block(P(ast::Block { - stmts: vec![], - expr: Some(call), - id: ast::DUMMY_NODE_ID, - rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated), - span: sp })); - match_arms.push(cx.arm(sp, vec![cx.pat_wild(sp)], unreachable)); + match_arms.push(cx.arm(sp, vec![cx.pat_wild(sp)], expr_unreachable_intrinsic(cx, sp))); // Final wrinkle: the self_args are expressions that deref // down to desired l-values, but we cannot actually deref @@ -1382,7 +1389,7 @@ impl<'a> MethodDef<'a> { // derive Debug on such a type could here generate code // that needs the feature gate enabled.) - cx.expr_unreachable(sp) + expr_unreachable_intrinsic(cx, sp) } else { From b8dce9bdb76ea3ad914665790b69435fe1f561aa Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Thu, 3 Mar 2016 14:55:24 -0500 Subject: [PATCH 3/9] derive: assume enum repr defaults to i64 It was originally intended to be i32, but it isn't. Fixes #31886. --- src/libsyntax_ext/deriving/generic/mod.rs | 2 +- src/test/run-pass/enum-discrim-autosizing.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 3896a7ce71462..119b88befd1d8 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -757,7 +757,7 @@ impl<'a> TraitDef<'a> { fn find_repr_type_name(diagnostic: &Handler, type_attrs: &[ast::Attribute]) -> &'static str { - let mut repr_type_name = "i32"; + let mut repr_type_name = "i64"; for a in type_attrs { for r in &attr::find_repr_attrs(diagnostic, a) { repr_type_name = match *r { diff --git a/src/test/run-pass/enum-discrim-autosizing.rs b/src/test/run-pass/enum-discrim-autosizing.rs index 99e44735d0f03..f027689a64a54 100644 --- a/src/test/run-pass/enum-discrim-autosizing.rs +++ b/src/test/run-pass/enum-discrim-autosizing.rs @@ -46,6 +46,7 @@ enum Ei64 { Bi64 = 0x8000_0000 } +#[derive(PartialEq)] enum Eu64 { Au64 = 0, Bu64 = 0x8000_0000_0000_0000 @@ -60,4 +61,7 @@ pub fn main() { assert_eq!(size_of::(), 4); assert_eq!(size_of::(), 8); assert_eq!(size_of::(), 8); + + // ensure no i32 collisions + assert!(Eu64::Au64 != Eu64::Bu64); } From 03e00dfebf21812798913f7c4444085c90bf78ff Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Sat, 5 Mar 2016 14:51:24 -0500 Subject: [PATCH 4/9] add #[derive(Hash)] test for #21714 --- src/test/run-pass/deriving-hash.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/test/run-pass/deriving-hash.rs b/src/test/run-pass/deriving-hash.rs index 69e9816ab9486..ea19510680f63 100644 --- a/src/test/run-pass/deriving-hash.rs +++ b/src/test/run-pass/deriving-hash.rs @@ -12,6 +12,7 @@ #![feature(hash_default)] use std::hash::{Hash, SipHasher, Hasher}; +use std::mem::size_of; #[derive(Hash)] struct Person { @@ -20,12 +21,30 @@ struct Person { phone: usize, } +#[derive(Hash)] +enum E { A=1, B } + fn hash(t: &T) -> u64 { let mut s = SipHasher::new_with_keys(0, 0); t.hash(&mut s); s.finish() } +struct FakeHasher<'a>(&'a mut Vec); +impl<'a> Hasher for FakeHasher<'a> { + fn finish(&self) -> u64 { + unimplemented!() + } + + fn write(&mut self, bytes: &[u8]) { + self.0.extend(bytes); + } +} + +fn fake_hash(v: &mut Vec, e: E) { + e.hash(&mut FakeHasher(v)); +} + fn main() { let person1 = Person { id: 5, @@ -39,4 +58,11 @@ fn main() { }; assert_eq!(hash(&person1), hash(&person1)); assert!(hash(&person1) != hash(&person2)); + + // test #21714 + let mut va = vec![]; + let mut vb = vec![]; + fake_hash(&mut va, E::A); + fake_hash(&mut vb, E::B); + assert!(va != vb); } From 0cdfa7ecb5efcd68ab210807586dacedaaaca2b4 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 7 Mar 2016 13:05:12 -0500 Subject: [PATCH 5/9] fix #21714 by using discriminant_value in #[derive(Hash)] This is the same approach taken in #24270, except that this should not be a breaking change because it only changes the output of hash functions, which nobody should be relying on. --- src/libsyntax_ext/deriving/hash.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/libsyntax_ext/deriving/hash.rs b/src/libsyntax_ext/deriving/hash.rs index bf8aa8fb23deb..96c4e6536e783 100644 --- a/src/libsyntax_ext/deriving/hash.rs +++ b/src/libsyntax_ext/deriving/hash.rs @@ -11,7 +11,7 @@ use deriving::generic::*; use deriving::generic::ty::*; -use syntax::ast::{MetaItem, Expr, Mutability}; +use syntax::ast::{self, MetaItem, Expr, Mutability}; use syntax::codemap::Span; use syntax::ext::base::{ExtCtxt, Annotatable}; use syntax::ext::build::AstBuilder; @@ -77,15 +77,18 @@ fn hash_substructure(cx: &mut ExtCtxt, trait_span: Span, substr: &Substructure) let fields = match *substr.fields { Struct(_, ref fs) => fs, - EnumMatching(index, variant, ref fs) => { - // Determine the discriminant. We will feed this value to the byte - // iteration function. - let discriminant = match variant.node.disr_expr { - Some(ref d) => d.clone(), - None => cx.expr_usize(trait_span, index) - }; + EnumMatching(_, _, ref fs) => { + let path = cx.std_path(&["intrinsics", "discriminant_value"]); + let call = cx.expr_call_global( + trait_span, path, vec![cx.expr_self(trait_span)]); + let variant_value = cx.expr_block(P(ast::Block { + stmts: vec![], + expr: Some(call), + id: ast::DUMMY_NODE_ID, + rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated), + span: trait_span })); - stmts.push(call_hash(trait_span, discriminant)); + stmts.push(call_hash(trait_span, variant_value)); fs } From 21f40042b8b528693df6cba571b47e1e1597dabd Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 7 Mar 2016 18:35:01 -0500 Subject: [PATCH 6/9] 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 | 53 +++++++++---------- 2 files changed, 48 insertions(+), 55 deletions(-) diff --git a/src/libsyntax_ext/deriving/cmp/ord.rs b/src/libsyntax_ext/deriving/cmp/ord.rs index 2fa847ee430d8..f4082e0b123bf 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 e857f7d52f912..3353addebc971 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 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_))) + 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)); + + 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 b5a0e71f9ca152f9174a8557ef46034bf0c81d57 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Mon, 7 Mar 2016 19:07:06 -0500 Subject: [PATCH 7/9] 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 f4082e0b123bf..a69d57423a22c 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 3353addebc971..b3864a6c2e79e 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 119b88befd1d8..2f72db48b9a33 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 @@ -842,7 +842,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); @@ -927,12 +927,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) /// } /// } /// } @@ -958,7 +958,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); @@ -1036,14 +1036,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 /// } /// } @@ -1051,10 +1051,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>, @@ -1085,14 +1085,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, @@ -1120,9 +1120,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::>(); @@ -1259,17 +1259,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 = @@ -1319,7 +1319,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)); @@ -1333,7 +1333,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)); @@ -1357,8 +1357,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 @@ -1397,7 +1397,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) @@ -1611,8 +1611,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 e084a8477e9c1b74e18ed20a3d8945180c69e760 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Tue, 8 Mar 2016 13:24:28 -0500 Subject: [PATCH 8/9] 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 a69d57423a22c..a7e156c5f6820 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 092f8548966da..49f14c937e953 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 8262a04e9ce17..a05bd7869b2a9 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 96c4e6536e783..9368658648910 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 4e2142f1fb482..75de5c56ea139 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 ea19510680f63..c40d0683657a8 100644 --- a/src/test/run-pass/deriving-hash.rs +++ b/src/test/run-pass/deriving-hash.rs @@ -24,6 +24,10 @@ struct Person { #[derive(Hash)] enum E { A=1, B } +// 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); From a03707363660eaee0ea594d61468ebd2d08041f4 Mon Sep 17 00:00:00 2001 From: Alex Burka Date: Thu, 10 Mar 2016 00:31:19 -0500 Subject: [PATCH 9/9] deriving: factor out discriminant_value construction --- src/libsyntax_ext/deriving/generic/mod.rs | 38 +++++++---------------- src/libsyntax_ext/deriving/hash.rs | 15 +++------ src/libsyntax_ext/deriving/mod.rs | 17 ++++++++++ 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 2f72db48b9a33..93fcfa4cc6372 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -209,6 +209,8 @@ use syntax::ptr::P; use self::ty::{LifetimeBounds, Path, Ptr, PtrTy, Self_, Ty}; +use deriving; + pub mod ty; pub struct TraitDef<'a> { @@ -381,22 +383,6 @@ fn find_type_parameters(ty: &ast::Ty, ty_param_names: &[ast::Name]) -> Vec P { - let path = cx.std_path(&["intrinsics", "unreachable"]); - let call = cx.expr_call_global( - sp, path, vec![]); - let unreachable = cx.expr_block(P(ast::Block { - stmts: vec![], - expr: Some(call), - id: ast::DUMMY_NODE_ID, - rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated), - span: sp })); - - unreachable -} - impl<'a> TraitDef<'a> { pub fn expand(&self, cx: &mut ExtCtxt, @@ -1277,15 +1263,11 @@ impl<'a> MethodDef<'a> { let mut first_ident = None; for (&ident, self_arg) in vi_idents.iter().zip(&self_args) { - let path = cx.std_path(&["intrinsics", "discriminant_value"]); - let call = cx.expr_call_global( - sp, path, vec![cx.expr_addr_of(sp, self_arg.clone())]); - let variant_value = cx.expr_block(P(ast::Block { - stmts: vec![], - expr: Some(call), - id: ast::DUMMY_NODE_ID, - rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated), - span: sp })); + let self_addr = cx.expr_addr_of(sp, self_arg.clone()); + let variant_value = deriving::call_intrinsic(cx, + sp, + "discriminant_value", + vec![self_addr]); let target_ty = cx.ty_ident(sp, cx.ident_of(target_type_name)); let variant_disr = cx.expr_cast(sp, variant_value, target_ty); @@ -1313,7 +1295,9 @@ impl<'a> MethodDef<'a> { //Since we know that all the arguments will match if we reach the match expression we //add the unreachable intrinsics as the result of the catch all which should help llvm //in optimizing it - match_arms.push(cx.arm(sp, vec![cx.pat_wild(sp)], expr_unreachable_intrinsic(cx, sp))); + match_arms.push(cx.arm(sp, + vec![cx.pat_wild(sp)], + deriving::call_intrinsic(cx, sp, "unreachable", vec![]))); // Final wrinkle: the self_args are expressions that deref // down to desired l-values, but we cannot actually deref @@ -1389,7 +1373,7 @@ impl<'a> MethodDef<'a> { // derive Debug on such a type could here generate code // that needs the feature gate enabled.) - expr_unreachable_intrinsic(cx, sp) + deriving::call_intrinsic(cx, sp, "unreachable", vec![]) } else { diff --git a/src/libsyntax_ext/deriving/hash.rs b/src/libsyntax_ext/deriving/hash.rs index 9368658648910..c37ae116d379b 100644 --- a/src/libsyntax_ext/deriving/hash.rs +++ b/src/libsyntax_ext/deriving/hash.rs @@ -12,7 +12,7 @@ use deriving; use deriving::generic::*; use deriving::generic::ty::*; -use syntax::ast::{self, MetaItem, Expr, Mutability}; +use syntax::ast::{MetaItem, Expr, Mutability}; use syntax::codemap::Span; use syntax::ext::base::{ExtCtxt, Annotatable}; use syntax::ext::build::AstBuilder; @@ -82,15 +82,10 @@ fn hash_substructure(cx: &mut ExtCtxt, trait_span: Span, substr: &Substructure) let fields = match *substr.fields { Struct(_, ref fs) => fs, EnumMatching(_, _, ref fs) => { - let path = cx.std_path(&["intrinsics", "discriminant_value"]); - let call = cx.expr_call_global( - trait_span, path, vec![cx.expr_self(trait_span)]); - let variant_value = cx.expr_block(P(ast::Block { - stmts: vec![], - expr: Some(call), - id: ast::DUMMY_NODE_ID, - rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated), - span: trait_span })); + let variant_value = deriving::call_intrinsic(cx, + trait_span, + "discriminant_value", + vec![cx.expr_self(trait_span)]); stmts.push(call_hash(trait_span, variant_value)); diff --git a/src/libsyntax_ext/deriving/mod.rs b/src/libsyntax_ext/deriving/mod.rs index 75de5c56ea139..615cb1de0f475 100644 --- a/src/libsyntax_ext/deriving/mod.rs +++ b/src/libsyntax_ext/deriving/mod.rs @@ -18,6 +18,7 @@ use syntax::ext::build::AstBuilder; use syntax::feature_gate; use syntax::codemap::Span; use syntax::parse::token::{intern, intern_and_get_ident}; +use syntax::ptr::P; macro_rules! pathvec { ($($x:ident)::+) => ( @@ -218,3 +219,19 @@ fn hygienic_type_parameter(item: &Annotatable, base: &str) -> String { typaram } +/// Constructs an expression that calls an intrinsic +fn call_intrinsic(cx: &ExtCtxt, + span: Span, + intrinsic: &str, + args: Vec>) -> P { + let path = cx.std_path(&["intrinsics", intrinsic]); + let call = cx.expr_call_global(span, path, args); + + cx.expr_block(P(ast::Block { + stmts: vec![], + expr: Some(call), + id: ast::DUMMY_NODE_ID, + rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated), + span: span })) +} +