Auto merge of #81294 - pnkfelix:issue-81211-use-ufcs-in-derive-debug, r=oli-obk

Use ufcs in derive(Debug)

Cc #81211.

(Arguably this *is* the fix for it.)

compiler/rustc_builtin_macros/src/deriving/debug.rs

@@ -8,6 +8,10 @@ use rustc_expand::base::{Annotatable, ExtCtxt};
 use rustc_span::symbol::{sym, Ident};
 use rustc_span::{Span, DUMMY_SP};
 
+fn make_mut_borrow(cx: &mut ExtCtxt<'_>, sp: Span, expr: P<Expr>) -> P<Expr> {
+    cx.expr(sp, ast::ExprKind::AddrOf(ast::BorrowKind::Ref, ast::Mutability::Mut, expr))
+}
+
 pub fn expand_deriving_debug(
     cx: &mut ExtCtxt<'_>,
     span: Span,
@@ -67,11 +71,12 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
     let fmt = substr.nonself_args[0].clone();
 
     let mut stmts = Vec::with_capacity(fields.len() + 2);
+    let fn_path_finish;
     match vdata {
         ast::VariantData::Tuple(..) | ast::VariantData::Unit(..) => {
             // tuple struct/"normal" variant
-            let expr =
-                cx.expr_method_call(span, fmt, Ident::new(sym::debug_tuple, span), vec![name]);
+            let fn_path_debug_tuple = cx.std_path(&[sym::fmt, sym::Formatter, sym::debug_tuple]);
+            let expr = cx.expr_call_global(span, fn_path_debug_tuple, vec![fmt, name]);
             stmts.push(cx.stmt_let(span, true, builder, expr));
 
             for field in fields {
@@ -79,22 +84,21 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
                 let field = cx.expr_addr_of(field.span, field.self_.clone());
                 let field = cx.expr_addr_of(field.span, field);
 
-                let expr = cx.expr_method_call(
-                    span,
-                    builder_expr.clone(),
-                    Ident::new(sym::field, span),
-                    vec![field],
-                );
+                let fn_path_field = cx.std_path(&[sym::fmt, sym::DebugTuple, sym::field]);
+                let builder_recv = make_mut_borrow(cx, span, builder_expr.clone());
+                let expr = cx.expr_call_global(span, fn_path_field, vec![builder_recv, field]);
 
                 // Use `let _ = expr;` to avoid triggering the
                 // unused_results lint.
                 stmts.push(stmt_let_underscore(cx, span, expr));
             }
+
+            fn_path_finish = cx.std_path(&[sym::fmt, sym::DebugTuple, sym::finish]);
         }
         ast::VariantData::Struct(..) => {
             // normal struct/struct variant
-            let expr =
-                cx.expr_method_call(span, fmt, Ident::new(sym::debug_struct, span), vec![name]);
+            let fn_path_debug_struct = cx.std_path(&[sym::fmt, sym::Formatter, sym::debug_struct]);
+            let expr = cx.expr_call_global(span, fn_path_debug_struct, vec![fmt, name]);
             stmts.push(cx.stmt_let(DUMMY_SP, true, builder, expr));
 
             for field in fields {
@@ -104,20 +108,20 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>
                 );
 
                 // Use double indirection to make sure this works for unsized types
+                let fn_path_field = cx.std_path(&[sym::fmt, sym::DebugStruct, sym::field]);
                 let field = cx.expr_addr_of(field.span, field.self_.clone());
                 let field = cx.expr_addr_of(field.span, field);
-                let expr = cx.expr_method_call(
-                    span,
-                    builder_expr.clone(),
-                    Ident::new(sym::field, span),
-                    vec![name, field],
-                );
+                let builder_recv = make_mut_borrow(cx, span, builder_expr.clone());
+                let expr =
+                    cx.expr_call_global(span, fn_path_field, vec![builder_recv, name, field]);
                 stmts.push(stmt_let_underscore(cx, span, expr));
             }
+            fn_path_finish = cx.std_path(&[sym::fmt, sym::DebugStruct, sym::finish]);
         }
     }
 
-    let expr = cx.expr_method_call(span, builder_expr, Ident::new(sym::finish, span), vec![]);
+    let builder_recv = make_mut_borrow(cx, span, builder_expr);
+    let expr = cx.expr_call_global(span, fn_path_finish, vec![builder_recv]);
 
     stmts.push(cx.stmt_expr(expr));
     let block = cx.block(span, stmts);

compiler/rustc_span/src/symbol.rs

@@ -133,6 +133,8 @@ symbols! {
         Copy,
         Count,
         Debug,
+        DebugStruct,
+        DebugTuple,
         Decodable,
         Decoder,
         Default,

src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.partial_eq/partial_eq.{impl#7}-fmt.-------.InstrumentCoverage.0.html

@@ -78,32 +78,36 @@ For revisions in Pull Requests (PR):
 4:17-4:22: @0[13]: _8 = &(*_9)
 4:17-4:22: @0.Call: _6 = Formatter::debug_struct(move _7, move _8) -> [return: bb1, unwind: bb6]
 4:17-4:22: @1[2]: FakeRead(ForLet, _6)
-4:17-4:22: @1[6]: _11 = &mut _6
-4:17-4:22: @1[9]: _13 = const "major"
-4:17-4:22: @1[10]: _12 = &(*_13)
-4:17-4:22: @1[15]: _17 = &(*_3)
-4:17-4:22: @1[16]: _16 = &_17
-4:17-4:22: @1[17]: _15 = &(*_16)
-4:17-4:22: @1[18]: _14 = move _15 as &dyn std::fmt::Debug (Pointer(Unsize))
-4:17-4:22: @1.Call: _10 = DebugStruct::field(move _11, move _12, move _14) -> [return: bb2, unwind: bb6]
-4:17-4:22: @2[9]: _19 = &mut _6
-4:17-4:22: @2[12]: _21 = const "minor"
-4:17-4:22: @2[13]: _20 = &(*_21)
-4:17-4:22: @2[18]: _25 = &(*_4)
-4:17-4:22: @2[19]: _24 = &_25
-4:17-4:22: @2[20]: _23 = &(*_24)
-4:17-4:22: @2[21]: _22 = move _23 as &dyn std::fmt::Debug (Pointer(Unsize))
-4:17-4:22: @2.Call: _18 = DebugStruct::field(move _19, move _20, move _22) -> [return: bb3, unwind: bb6]
-4:17-4:22: @3[9]: _27 = &mut _6
-4:17-4:22: @3[12]: _29 = const "patch"
-4:17-4:22: @3[13]: _28 = &(*_29)
-4:17-4:22: @3[18]: _33 = &(*_5)
-4:17-4:22: @3[19]: _32 = &_33
-4:17-4:22: @3[20]: _31 = &(*_32)
-4:17-4:22: @3[21]: _30 = move _31 as &dyn std::fmt::Debug (Pointer(Unsize))
-4:17-4:22: @3.Call: _26 = DebugStruct::field(move _27, move _28, move _30) -> [return: bb4, unwind: bb6]
-4:17-4:22: @4[8]: _34 = &mut _6
-4:17-4:22: @4.Call: _0 = DebugStruct::finish(move _34) -> [return: bb5, unwind: bb6]
+4:17-4:22: @1[7]: _12 = &mut _6
+4:17-4:22: @1[8]: _11 = &mut (*_12)
+4:17-4:22: @1[11]: _14 = const "major"
+4:17-4:22: @1[12]: _13 = &(*_14)
+4:17-4:22: @1[17]: _18 = &(*_3)
+4:17-4:22: @1[18]: _17 = &_18
+4:17-4:22: @1[19]: _16 = &(*_17)
+4:17-4:22: @1[20]: _15 = move _16 as &dyn std::fmt::Debug (Pointer(Unsize))
+4:17-4:22: @1.Call: _10 = DebugStruct::field(move _11, move _13, move _15) -> [return: bb2, unwind: bb6]
+4:17-4:22: @2[11]: _21 = &mut _6
+4:17-4:22: @2[12]: _20 = &mut (*_21)
+4:17-4:22: @2[15]: _23 = const "minor"
+4:17-4:22: @2[16]: _22 = &(*_23)
+4:17-4:22: @2[21]: _27 = &(*_4)
+4:17-4:22: @2[22]: _26 = &_27
+4:17-4:22: @2[23]: _25 = &(*_26)
+4:17-4:22: @2[24]: _24 = move _25 as &dyn std::fmt::Debug (Pointer(Unsize))
+4:17-4:22: @2.Call: _19 = DebugStruct::field(move _20, move _22, move _24) -> [return: bb3, unwind: bb6]
+4:17-4:22: @3[11]: _30 = &mut _6
+4:17-4:22: @3[12]: _29 = &mut (*_30)
+4:17-4:22: @3[15]: _32 = const "patch"
+4:17-4:22: @3[16]: _31 = &(*_32)
+4:17-4:22: @3[21]: _36 = &(*_5)
+4:17-4:22: @3[22]: _35 = &_36
+4:17-4:22: @3[23]: _34 = &(*_35)
+4:17-4:22: @3[24]: _33 = move _34 as &dyn std::fmt::Debug (Pointer(Unsize))
+4:17-4:22: @3.Call: _28 = DebugStruct::field(move _29, move _31, move _33) -> [return: bb4, unwind: bb6]
+4:17-4:22: @4[10]: _38 = &mut _6
+4:17-4:22: @4[11]: _37 = &mut (*_38)
+4:17-4:22: @4.Call: _0 = DebugStruct::finish(move _37) -> [return: bb5, unwind: bb6]
 4:22-4:22: @5.Return: return"><span class="annotation">@0,1,2,3,4,5⦊</span>Debug<span class="annotation">⦉@0,1,2,3,4,5</span></span></span></span></div>
 </body>
 </html>

src/test/ui/derives/derive-Debug-use-ufcs-struct.rs

@@ -0,0 +1,40 @@
+// run-pass
+#![allow(warnings)]
+
+#[derive(Debug)]
+pub struct Bar { pub t: () }
+
+impl<T> Access for T {}
+pub trait Access {
+    fn field(&self, _: impl Sized, _: impl Sized) {
+        panic!("got into Access::field");
+    }
+
+    fn finish(&self) -> Result<(), std::fmt::Error> {
+        panic!("got into Access::finish");
+    }
+
+    fn debug_struct(&self, _: impl Sized, _: impl Sized) {
+        panic!("got into Access::debug_struct");
+    }
+}
+
+impl<T> MutAccess for T {}
+pub trait MutAccess {
+    fn field(&mut self, _: impl Sized, _: impl Sized) {
+        panic!("got into MutAccess::field");
+    }
+
+    fn finish(&mut self) -> Result<(), std::fmt::Error> {
+        panic!("got into MutAccess::finish");
+    }
+
+    fn debug_struct(&mut self, _: impl Sized, _: impl Sized) {
+        panic!("got into MutAccess::debug_struct");
+    }
+}
+
+fn main() {
+    let bar = Bar { t: () };
+    assert_eq!("Bar { t: () }", format!("{:?}", bar));
+}

src/test/ui/derives/derive-Debug-use-ufcs-tuple.rs

@@ -0,0 +1,32 @@
+// run-pass
+#![allow(warnings)]
+
+#[derive(Debug)]
+pub struct Foo<T>(pub T);
+
+use std::fmt;
+
+impl<T> Field for T {}
+impl<T> Finish for T {}
+impl Dt for &mut fmt::Formatter<'_> {}
+
+pub trait Field {
+    fn field(&self, _: impl Sized) {
+        panic!("got into field");
+    }
+}
+pub trait Finish {
+    fn finish(&self) -> Result<(), std::fmt::Error> {
+        panic!("got into finish");
+    }
+}
+pub trait Dt {
+    fn debug_tuple(&self, _: &str) {
+        panic!("got into debug_tuple");
+    }
+}
+
+fn main() {
+    let foo = Foo(());
+    assert_eq!("Foo(())", format!("{:?}", foo));
+}

src/test/ui/methods/method-lookup-order.rs

@@ -0,0 +1,190 @@
+// ignore-tidy-linelength
+
+// run-pass
+
+// There are five cfg's below. I explored the set of all non-empty combinations
+// of the below five cfg's, which is 2^5 - 1 = 31 combinations.
+//
+// Of the 31, 11 resulted in ambiguous method resolutions; while it may be good
+// to have a test for all of the eleven variations of that error, I am not sure
+// this particular test is the best way to encode it. So they are skipped in
+// this revisions list (but not in the expansion mapping the binary encoding to
+// the corresponding cfg flags).
+//
+// Notable, here are the cases that will be incompatible if something does not override them first:
+// {bar_for_foo, valbar_for_et_foo}: these are higher precedent than the `&mut self` method on `Foo`, and so no case matching bx1x1x is included.
+// {mutbar_for_foo, valbar_for_etmut_foo} (which are lower precedent than the inherent `&mut self` method on `Foo`; e.g. b10101 *is* included.
+
+// revisions: b00001 b00010 b00011 b00100 b00101 b00110 b00111 b01000 b01001 b01100 b01101 b10000 b10001 b10010 b10011 b10101 b10111 b11000 b11001 b11101
+
+//[b00001]compile-flags:  --cfg inherent_mut
+//[b00010]compile-flags:                     --cfg bar_for_foo
+//[b00011]compile-flags:  --cfg inherent_mut --cfg bar_for_foo
+//[b00100]compile-flags:                                       --cfg mutbar_for_foo
+//[b00101]compile-flags:  --cfg inherent_mut                   --cfg mutbar_for_foo
+//[b00110]compile-flags:                     --cfg bar_for_foo --cfg mutbar_for_foo
+//[b00111]compile-flags:  --cfg inherent_mut --cfg bar_for_foo --cfg mutbar_for_foo
+//[b01000]compile-flags:                                                            --cfg valbar_for_et_foo
+//[b01001]compile-flags:  --cfg inherent_mut                                        --cfg valbar_for_et_foo
+//[b01010]compile-flags:                     --cfg bar_for_foo                      --cfg valbar_for_et_foo
+//[b01011]compile-flags:  --cfg inherent_mut --cfg bar_for_foo                      --cfg valbar_for_et_foo
+//[b01100]compile-flags:                                       --cfg mutbar_for_foo --cfg valbar_for_et_foo
+//[b01101]compile-flags:  --cfg inherent_mut                   --cfg mutbar_for_foo --cfg valbar_for_et_foo
+//[b01110]compile-flags:                     --cfg bar_for_foo --cfg mutbar_for_foo --cfg valbar_for_et_foo
+//[b01111]compile-flags:  --cfg inherent_mut --cfg bar_for_foo --cfg mutbar_for_foo --cfg valbar_for_et_foo
+//[b10000]compile-flags:                                                                                    --cfg valbar_for_etmut_foo
+//[b10001]compile-flags:  --cfg inherent_mut                                                                --cfg valbar_for_etmut_foo
+//[b10010]compile-flags:                     --cfg bar_for_foo                                              --cfg valbar_for_etmut_foo
+//[b10011]compile-flags:  --cfg inherent_mut --cfg bar_for_foo                                              --cfg valbar_for_etmut_foo
+//[b10100]compile-flags:                                       --cfg mutbar_for_foo                         --cfg valbar_for_etmut_foo
+//[b10101]compile-flags:  --cfg inherent_mut                   --cfg mutbar_for_foo                         --cfg valbar_for_etmut_foo
+//[b10110]compile-flags:                     --cfg bar_for_foo --cfg mutbar_for_foo                         --cfg valbar_for_etmut_foo
+//[b10111]compile-flags:  --cfg inherent_mut --cfg bar_for_foo --cfg mutbar_for_foo                         --cfg valbar_for_etmut_foo
+//[b11000]compile-flags:                                                            --cfg valbar_for_et_foo --cfg valbar_for_etmut_foo
+//[b11001]compile-flags:  --cfg inherent_mut                                        --cfg valbar_for_et_foo --cfg valbar_for_etmut_foo
+//[b11010]compile-flags:                     --cfg bar_for_foo                      --cfg valbar_for_et_foo --cfg valbar_for_etmut_foo
+//[b11011]compile-flags:  --cfg inherent_mut --cfg bar_for_foo                      --cfg valbar_for_et_foo --cfg valbar_for_etmut_foo
+//[b11100]compile-flags:                                       --cfg mutbar_for_foo --cfg valbar_for_et_foo --cfg valbar_for_etmut_foo
+//[b11101]compile-flags:  --cfg inherent_mut                   --cfg mutbar_for_foo --cfg valbar_for_et_foo --cfg valbar_for_etmut_foo
+//[b11110]compile-flags:                     --cfg bar_for_foo --cfg mutbar_for_foo --cfg valbar_for_et_foo --cfg valbar_for_etmut_foo
+//[b11111]compile-flags:  --cfg inherent_mut --cfg bar_for_foo --cfg mutbar_for_foo --cfg valbar_for_et_foo --cfg valbar_for_etmut_foo
+
+struct Foo {}
+
+type S = &'static str;
+
+trait Bar {
+    fn bar(&self, _: &str) -> S;
+}
+
+trait MutBar {
+    fn bar(&mut self, _: &str) -> S;
+}
+
+trait ValBar {
+    fn bar(self, _: &str) -> S;
+}
+
+#[cfg(inherent_mut)]
+impl Foo {
+    fn bar(&mut self, _: &str) -> S {
+        "In struct impl!"
+    }
+}
+
+#[cfg(bar_for_foo)]
+impl Bar for Foo {
+    fn bar(&self, _: &str) -> S {
+        "In trait &self impl!"
+    }
+}
+
+#[cfg(mutbar_for_foo)]
+impl MutBar for Foo {
+    fn bar(&mut self, _: &str) -> S {
+        "In trait &mut self impl!"
+    }
+}
+
+#[cfg(valbar_for_et_foo)]
+impl ValBar for &Foo {
+    fn bar(self, _: &str) -> S {
+        "In trait self impl for &Foo!"
+    }
+}
+
+#[cfg(valbar_for_etmut_foo)]
+impl ValBar for &mut Foo {
+    fn bar(self, _: &str) -> S {
+        "In trait self impl for &mut Foo!"
+    }
+}
+
+fn main() {
+    #![allow(unused_mut)] // some of the impls above will want it.
+
+    #![allow(unreachable_patterns)] // the cfg-coding pattern below generates unreachable patterns.
+
+    {
+        macro_rules! all_variants_on_value {
+            ($e:expr) => {
+                match $e {
+                    #[cfg(bar_for_foo)]
+                    x => assert_eq!(x, "In trait &self impl!"),
+
+                    #[cfg(valbar_for_et_foo)]
+                    x => assert_eq!(x, "In trait self impl for &Foo!"),
+
+                    #[cfg(inherent_mut)]
+                    x => assert_eq!(x, "In struct impl!"),
+
+                    #[cfg(mutbar_for_foo)]
+                    x => assert_eq!(x, "In trait &mut self impl!"),
+
+                    #[cfg(valbar_for_etmut_foo)]
+                    x => assert_eq!(x, "In trait self impl for &mut Foo!"),
+                }
+            }
+        }
+
+        let mut f = Foo {};
+        all_variants_on_value!(f.bar("f.bar"));
+
+        let f_mr = &mut Foo {};
+        all_variants_on_value!((*f_mr).bar("(*f_mr).bar"));
+    }
+
+    // This is sort of interesting: `&mut Foo` ends up with a significantly
+    // different resolution order than what was devised above. Presumably this
+    // is because we can get to a `&self` method by first a deref of the given
+    // `&mut Foo` and then an autoref, and that is a longer path than a mere
+    // auto-ref of a `Foo`.
+
+    {
+        let f_mr = &mut Foo {};
+
+        match f_mr.bar("f_mr.bar") {
+            #[cfg(inherent_mut)]
+            x => assert_eq!(x, "In struct impl!"),
+
+            #[cfg(valbar_for_etmut_foo)]
+            x => assert_eq!(x, "In trait self impl for &mut Foo!"),
+
+            #[cfg(mutbar_for_foo)]
+            x => assert_eq!(x, "In trait &mut self impl!"),
+
+            #[cfg(valbar_for_et_foo)]
+            x => assert_eq!(x, "In trait self impl for &Foo!"),
+
+            #[cfg(bar_for_foo)]
+            x => assert_eq!(x, "In trait &self impl!"),
+        }
+    }
+
+
+    // Note that this isn't actually testing a resolution order; if both of these are
+    // enabled, it yields an ambiguous method resolution error. The test tries to embed
+    // that fact by testing *both* orders (and so the only way that can be right is if
+    // they are not actually compatible).
+    #[cfg(any(bar_for_foo, valbar_for_et_foo))]
+    {
+        let f_r = &Foo {};
+
+        match f_r.bar("f_r.bar") {
+            #[cfg(bar_for_foo)]
+            x => assert_eq!(x, "In trait &self impl!"),
+
+            #[cfg(valbar_for_et_foo)]
+            x => assert_eq!(x, "In trait self impl for &Foo!"),
+        }
+
+        match f_r.bar("f_r.bar") {
+            #[cfg(valbar_for_et_foo)]
+            x => assert_eq!(x, "In trait self impl for &Foo!"),
+
+            #[cfg(bar_for_foo)]
+            x => assert_eq!(x, "In trait &self impl!"),
+        }
+    }
+
+}