Rollup merge of #100387 - cjgillot:hygiene-trait-impl, r=petrochenkov

Check uniqueness of impl items by trait item when applicable.

When checking uniqueness of item names in impl blocks, we currently use the same definition of hygiene as for toplevel items.  This means that a plain item and one generated by a macro 2.0 do not collide.

This hygiene rule does not match with how impl items resolve to associated trait items. As a consequence, we misdiagnose the trait impls.

This PR proposes to consider that trait impl items are uses of the corresponding trait items during resolution, instead of checking for duplicates later. An error is emitted when a trait impl item is used twice.

There should be no stable breakage, since macros 2.0 are still unstable.

r? ``@petrochenkov``
cc ``@RalfJung``

Fixes #71614.

compiler/rustc_hir_analysis/src/impl_wf_check.rs

@@ -197,6 +197,9 @@ fn report_unused_parameter(tcx: TyCtxt<'_>, span: Span, kind: &str, name: Symbol
 
 /// Enforce that we do not have two items in an impl with the same name.
 fn enforce_impl_items_are_distinct(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
+    if tcx.impl_trait_ref(impl_def_id).is_some() {
+        return;
+    }
     let mut seen_type_items = FxHashMap::default();
     let mut seen_value_items = FxHashMap::default();
     for &impl_item_ref in tcx.associated_item_def_ids(impl_def_id) {

compiler/rustc_resolve/src/diagnostics.rs

@@ -1050,6 +1050,19 @@ impl<'a> Resolver<'a> {
                 err.span_label(trait_item_span, "item in trait");
                 err
             }
+            ResolutionError::TraitImplDuplicate { name, trait_item_span, old_span } => {
+                let mut err = struct_span_err!(
+                    self.session,
+                    span,
+                    E0201,
+                    "duplicate definitions with name `{}`:",
+                    name,
+                );
+                err.span_label(old_span, "previous definition here");
+                err.span_label(trait_item_span, "item in trait");
+                err.span_label(span, "duplicate definition");
+                err
+            }
             ResolutionError::InvalidAsmSym => {
                 let mut err = self.session.struct_span_err(span, "invalid `sym` operand");
                 err.span_label(span, "is a local variable");

compiler/rustc_resolve/src/late.rs

@@ -2618,8 +2618,9 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                                         this.with_current_self_type(self_type, |this| {
                                             this.with_self_rib_ns(ValueNS, Res::SelfCtor(item_def_id), |this| {
                                                 debug!("resolve_implementation with_self_rib_ns(ValueNS, ...)");
+                                                let mut seen_trait_items = Default::default();
                                                 for item in impl_items {
-                                                    this.resolve_impl_item(&**item);
+                                                    this.resolve_impl_item(&**item, &mut seen_trait_items);
                                                 }
                                             });
                                         });
@@ -2633,7 +2634,11 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
         );
     }
 
-    fn resolve_impl_item(&mut self, item: &'ast AssocItem) {
+    fn resolve_impl_item(
+        &mut self,
+        item: &'ast AssocItem,
+        seen_trait_items: &mut FxHashMap<DefId, Span>,
+    ) {
         use crate::ResolutionError::*;
         match &item.kind {
             AssocItemKind::Const(_, ty, default) => {
@@ -2646,6 +2651,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                     &item.kind,
                     ValueNS,
                     item.span,
+                    seen_trait_items,
                     |i, s, c| ConstNotMemberOfTrait(i, s, c),
                 );
 
@@ -2686,6 +2692,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                             &item.kind,
                             ValueNS,
                             item.span,
+                            seen_trait_items,
                             |i, s, c| MethodNotMemberOfTrait(i, s, c),
                         );
 
@@ -2714,6 +2721,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                                 &item.kind,
                                 TypeNS,
                                 item.span,
+                                seen_trait_items,
                                 |i, s, c| TypeNotMemberOfTrait(i, s, c),
                             );
 
@@ -2735,6 +2743,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
         kind: &AssocItemKind,
         ns: Namespace,
         span: Span,
+        seen_trait_items: &mut FxHashMap<DefId, Span>,
         err: F,
     ) where
         F: FnOnce(Ident, String, Option<Symbol>) -> ResolutionError<'a>,
@@ -2767,7 +2776,25 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
         };
 
         let res = binding.res();
-        let Res::Def(def_kind, _) = res else { bug!() };
+        let Res::Def(def_kind, id_in_trait) = res else { bug!() };
+
+        match seen_trait_items.entry(id_in_trait) {
+            Entry::Occupied(entry) => {
+                self.report_error(
+                    span,
+                    ResolutionError::TraitImplDuplicate {
+                        name: ident.name,
+                        old_span: *entry.get(),
+                        trait_item_span: binding.span,
+                    },
+                );
+                return;
+            }
+            Entry::Vacant(entry) => {
+                entry.insert(span);
+            }
+        };
+
         match (def_kind, kind) {
             (DefKind::AssocTy, AssocItemKind::Type(..))
             | (DefKind::AssocFn, AssocItemKind::Fn(..))

compiler/rustc_resolve/src/lib.rs

@@ -235,6 +235,8 @@ enum ResolutionError<'a> {
         trait_item_span: Span,
         code: rustc_errors::DiagnosticId,
     },
+    /// Error E0201: multiple impl items for the same trait item.
+    TraitImplDuplicate { name: Symbol, trait_item_span: Span, old_span: Span },
     /// Inline asm `sym` operand must refer to a `fn` or `static`.
     InvalidAsmSym,
 }

src/test/ui/associated-item/associated-item-duplicate-names-3.rs

@@ -16,4 +16,5 @@ impl Foo for Baz {
 
 fn main() {
     let x: Baz::Bar = 5;
+    //~^ ERROR ambiguous associated type
 }

src/test/ui/associated-item/associated-item-duplicate-names-3.stderr

@@ -1,11 +1,21 @@
 error[E0201]: duplicate definitions with name `Bar`:
   --> $DIR/associated-item-duplicate-names-3.rs:14:5
    |
+LL |     type Bar;
+   |     --------- item in trait
+...
 LL |     type Bar = i16;
-   |     -------- previous definition of `Bar` here
+   |     --------------- previous definition here
 LL |     type Bar = u16;
-   |     ^^^^^^^^ duplicate definition
+   |     ^^^^^^^^^^^^^^^ duplicate definition
 
-error: aborting due to previous error
+error[E0223]: ambiguous associated type
+  --> $DIR/associated-item-duplicate-names-3.rs:18:12
+   |
+LL |     let x: Baz::Bar = 5;
+   |            ^^^^^^^^ help: use fully-qualified syntax: `<Baz as Trait>::Bar`
+
+error: aborting due to 2 previous errors
 
-For more information about this error, try `rustc --explain E0201`.
+Some errors have detailed explanations: E0201, E0223.
+For more information about an error, try `rustc --explain E0201`.

src/test/ui/associated-item/associated-item-duplicate-names.stderr

@@ -1,18 +1,24 @@
 error[E0201]: duplicate definitions with name `Ty`:
   --> $DIR/associated-item-duplicate-names.rs:11:5
    |
+LL |     type Ty;
+   |     -------- item in trait
+...
 LL |     type Ty = ();
-   |     ------- previous definition of `Ty` here
+   |     ------------- previous definition here
 LL |     type Ty = usize;
-   |     ^^^^^^^ duplicate definition
+   |     ^^^^^^^^^^^^^^^^ duplicate definition
 
 error[E0201]: duplicate definitions with name `BAR`:
   --> $DIR/associated-item-duplicate-names.rs:13:5
    |
+LL |     const BAR: u32;
+   |     --------------- item in trait
+...
 LL |     const BAR: u32 = 7;
-   |     -------------- previous definition of `BAR` here
+   |     ------------------- previous definition here
 LL |     const BAR: u32 = 8;
-   |     ^^^^^^^^^^^^^^ duplicate definition
+   |     ^^^^^^^^^^^^^^^^^^^ duplicate definition
 
 error: aborting due to 2 previous errors
 

src/test/ui/error-codes/E0201.stderr

@@ -1,27 +1,33 @@
-error[E0201]: duplicate definitions with name `bar`:
-  --> $DIR/E0201.rs:5:5
-   |
-LL |     fn bar(&self) -> bool { self.0 > 5 }
-   |     --------------------- previous definition of `bar` here
-LL |     fn bar() {}
-   |     ^^^^^^^^ duplicate definition
-
 error[E0201]: duplicate definitions with name `baz`:
   --> $DIR/E0201.rs:17:5
    |
+LL |     fn baz(&self) -> bool;
+   |     ---------------------- item in trait
+...
 LL |     fn baz(&self) -> bool { true }
-   |     --------------------- previous definition of `baz` here
+   |     ------------------------------ previous definition here
 LL |     fn baz(&self) -> bool { self.0 > 5 }
-   |     ^^^^^^^^^^^^^^^^^^^^^ duplicate definition
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ duplicate definition
 
 error[E0201]: duplicate definitions with name `Quux`:
   --> $DIR/E0201.rs:18:5
    |
+LL |     type Quux;
+   |     ---------- item in trait
+...
 LL |     type Quux = u32;
-   |     --------- previous definition of `Quux` here
+   |     ---------------- previous definition here
 ...
 LL |     type Quux = u32;
-   |     ^^^^^^^^^ duplicate definition
+   |     ^^^^^^^^^^^^^^^^ duplicate definition
+
+error[E0201]: duplicate definitions with name `bar`:
+  --> $DIR/E0201.rs:5:5
+   |
+LL |     fn bar(&self) -> bool { self.0 > 5 }
+   |     --------------------- previous definition of `bar` here
+LL |     fn bar() {}
+   |     ^^^^^^^^ duplicate definition
 
 error: aborting due to 3 previous errors
 

src/test/ui/hygiene/impl_items-2.rs

@@ -0,0 +1,26 @@
+#![feature(decl_macro)]
+
+trait Trait {
+    fn foo() {}
+}
+
+macro trait_impl() {
+    fn foo() {}
+}
+
+// Check that we error on multiple impl items that resolve to the same trait item.
+impl Trait for i32 {
+    trait_impl!();
+    fn foo() {}
+    //~^ ERROR duplicate definitions with name `foo`: [E0201]
+}
+
+struct Type;
+
+// Check that we do not error with inherent impls.
+impl Type {
+    trait_impl!();
+    fn foo() {}
+}
+
+fn main() {}

src/test/ui/hygiene/impl_items-2.stderr

@@ -0,0 +1,15 @@
+error[E0201]: duplicate definitions with name `foo`:
+  --> $DIR/impl_items-2.rs:14:5
+   |
+LL |     fn foo() {}
+   |     ----------- item in trait
+...
+LL |     fn foo() {}
+   |     ----------- previous definition here
+...
+LL |     fn foo() {}
+   |     ^^^^^^^^^^^ duplicate definition
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0201`.

src/test/ui/traits/issue-8153.stderr

@@ -1,10 +1,13 @@
 error[E0201]: duplicate definitions with name `bar`:
   --> $DIR/issue-8153.rs:11:5
    |
+LL |     fn bar(&self) -> isize;
+   |     ----------------------- item in trait
+...
 LL |     fn bar(&self) -> isize {1}
-   |     ---------------------- previous definition of `bar` here
+   |     -------------------------- previous definition here
 LL |     fn bar(&self) -> isize {2}
-   |     ^^^^^^^^^^^^^^^^^^^^^^ duplicate definition
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ duplicate definition
 
 error: aborting due to previous error