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

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`?)

3 files changed, 12 insertions, 12 deletions

diff --git a/src/libsyntax_ext/deriving/cmp/ord.rs b/src/libsyntax_ext/deriving/cmp/ord.rs
index 1a392ac3765..99b6f752e94 100644
--- a/src/libsyntax_ext/deriving/cmp/ord.rs
+++ b/src/libsyntax_ext/deriving/cmp/ord.rs
@@ -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 = {
diff --git a/src/libsyntax_ext/deriving/cmp/partial_eq.rs b/src/libsyntax_ext/deriving/cmp/partial_eq.rs
index 81ca7e73228..c259733d81a 100644
--- a/src/libsyntax_ext/deriving/cmp/partial_eq.rs
+++ b/src/libsyntax_ext/deriving/cmp/partial_eq.rs
@@ -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,
diff --git a/src/libsyntax_ext/deriving/cmp/partial_ord.rs b/src/libsyntax_ext/deriving/cmp/partial_ord.rs
index 9560fd0570a..d71527fd0ed 100644
--- a/src/libsyntax_ext/deriving/cmp/partial_ord.rs
+++ b/src/libsyntax_ext/deriving/cmp/partial_ord.rs
@@ -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 = {