Rollup merge of #112318 - oli-obk:assoc_ty_sized_bound_for_object_safety, r=compiler-errors

Merge method, type and const object safety checks

cc `@spastorino` and `@compiler-errors` on the first commit. I believe it to be correct, as the field is only `Some` for assoc types, so just checking the field without checking the assoc kind to be `Type` is fine.

The second commit avoids going through all associated items thrice and just goes over all of them once, running the object safety checks per assoc item kind.

Author: matthiaskrgr

compiler/rustc_hir_analysis/src/astconv/mod.rs

@@ -1601,7 +1601,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                             tcx.associated_items(pred.def_id())
                                 .in_definition_order()
                                 .filter(|item| item.kind == ty::AssocKind::Type)
-                                .filter(|item| tcx.opt_rpitit_info(item.def_id).is_none())
+                                .filter(|item| item.opt_rpitit_info.is_none())
                                 .map(|item| item.def_id),
                         );
                     }
@@ -1643,6 +1643,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
             }
         }
 
+        // `dyn Trait<Assoc = Foo>` desugars to (not Rust syntax) `dyn Trait where <Self as Trait>::Assoc = Foo`.
+        // So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated
+        // types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a
+        // corresponding `Projection` clause
         for (projection_bound, _) in &projection_bounds {
             for def_ids in associated_types.values_mut() {
                 def_ids.remove(&projection_bound.projection_def_id());

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -1216,7 +1216,7 @@ fn compare_number_of_generics<'tcx>(
     // has mismatched type or const generic arguments, then the method that it's
     // inheriting the generics from will also have mismatched arguments, and
     // we'll report an error for that instead. Delay a bug for safety, though.
-    if tcx.opt_rpitit_info(trait_.def_id).is_some() {
+    if trait_.opt_rpitit_info.is_some() {
         return Err(tcx.sess.delay_span_bug(
             rustc_span::DUMMY_SP,
             "errors comparing numbers of generics of trait/impl functions were not emitted",
@@ -2006,7 +2006,7 @@ pub(super) fn check_type_bounds<'tcx>(
     // A synthetic impl Trait for RPITIT desugaring has no HIR, which we currently use to get the
     // span for an impl's associated type. Instead, for these, use the def_span for the synthesized
     // associated type.
-    let impl_ty_span = if tcx.opt_rpitit_info(impl_ty.def_id).is_some() {
+    let impl_ty_span = if impl_ty.opt_rpitit_info.is_some() {
         tcx.def_span(impl_ty_def_id)
     } else {
         match tcx.hir().get_by_def_id(impl_ty_def_id) {

compiler/rustc_hir_analysis/src/check/mod.rs

@@ -188,7 +188,7 @@ fn missing_items_err(
     full_impl_span: Span,
 ) {
     let missing_items =
-        missing_items.iter().filter(|trait_item| tcx.opt_rpitit_info(trait_item.def_id).is_none());
+        missing_items.iter().filter(|trait_item| trait_item.opt_rpitit_info.is_none());
 
     let missing_items_msg = missing_items
         .clone()

compiler/rustc_trait_selection/src/traits/object_safety.rs

@@ -115,15 +115,11 @@ fn object_safety_violations_for_trait(
     tcx: TyCtxt<'_>,
     trait_def_id: DefId,
 ) -> Vec<ObjectSafetyViolation> {
-    // Check methods for violations.
+    // Check assoc items for violations.
     let mut violations: Vec<_> = tcx
         .associated_items(trait_def_id)
         .in_definition_order()
-        .filter(|item| item.kind == ty::AssocKind::Fn)
-        .filter_map(|&item| {
-            object_safety_violation_for_method(tcx, trait_def_id, item)
-                .map(|(code, span)| ObjectSafetyViolation::Method(item.name, code, span))
-        })
+        .filter_map(|&item| object_safety_violation_for_assoc_item(tcx, trait_def_id, item))
         .collect();
 
     // Check the trait itself.
@@ -145,30 +141,6 @@ fn object_safety_violations_for_trait(
         violations.push(ObjectSafetyViolation::SupertraitNonLifetimeBinder(spans));
     }
 
-    violations.extend(
-        tcx.associated_items(trait_def_id)
-            .in_definition_order()
-            .filter(|item| item.kind == ty::AssocKind::Const)
-            .map(|item| {
-                let ident = item.ident(tcx);
-                ObjectSafetyViolation::AssocConst(ident.name, ident.span)
-            }),
-    );
-
-    if !tcx.features().generic_associated_types_extended {
-        violations.extend(
-            tcx.associated_items(trait_def_id)
-                .in_definition_order()
-                .filter(|item| item.kind == ty::AssocKind::Type)
-                .filter(|item| !tcx.generics_of(item.def_id).params.is_empty())
-                .filter(|item| tcx.opt_rpitit_info(item.def_id).is_none())
-                .map(|item| {
-                    let ident = item.ident(tcx);
-                    ObjectSafetyViolation::GAT(ident.name, ident.span)
-                }),
-        );
-    }
-
     debug!(
         "object_safety_violations_for_trait(trait_def_id={:?}) = {:?}",
         trait_def_id, violations
@@ -401,34 +373,54 @@ fn generics_require_sized_self(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
     })
 }
 
-/// Returns `Some(_)` if this method makes the containing trait not object safe.
-fn object_safety_violation_for_method(
+/// Returns `Some(_)` if this item makes the containing trait not object safe.
+#[instrument(level = "debug", skip(tcx), ret)]
+fn object_safety_violation_for_assoc_item(
     tcx: TyCtxt<'_>,
     trait_def_id: DefId,
-    method: ty::AssocItem,
-) -> Option<(MethodViolationCode, Span)> {
-    debug!("object_safety_violation_for_method({:?}, {:?})", trait_def_id, method);
-    // Any method that has a `Self : Sized` requisite is otherwise
+    item: ty::AssocItem,
+) -> Option<ObjectSafetyViolation> {
+    // Any item that has a `Self : Sized` requisite is otherwise
     // exempt from the regulations.
-    if generics_require_sized_self(tcx, method.def_id) {
+    if generics_require_sized_self(tcx, item.def_id) {
         return None;
     }
 
-    let violation = virtual_call_violation_for_method(tcx, trait_def_id, method);
-    // Get an accurate span depending on the violation.
-    violation.map(|v| {
-        let node = tcx.hir().get_if_local(method.def_id);
-        let span = match (&v, node) {
-            (MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span,
-            (MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span,
-            (MethodViolationCode::ReferencesImplTraitInTrait(span), _) => *span,
-            (MethodViolationCode::ReferencesSelfOutput, Some(node)) => {
-                node.fn_decl().map_or(method.ident(tcx).span, |decl| decl.output.span())
+    match item.kind {
+        // Associated consts are never object safe, as they can't have `where` bounds yet at all,
+        // and associated const bounds in trait objects aren't a thing yet either.
+        ty::AssocKind::Const => {
+            Some(ObjectSafetyViolation::AssocConst(item.name, item.ident(tcx).span))
+        }
+        ty::AssocKind::Fn => virtual_call_violation_for_method(tcx, trait_def_id, item).map(|v| {
+            let node = tcx.hir().get_if_local(item.def_id);
+            // Get an accurate span depending on the violation.
+            let span = match (&v, node) {
+                (MethodViolationCode::ReferencesSelfInput(Some(span)), _) => *span,
+                (MethodViolationCode::UndispatchableReceiver(Some(span)), _) => *span,
+                (MethodViolationCode::ReferencesImplTraitInTrait(span), _) => *span,
+                (MethodViolationCode::ReferencesSelfOutput, Some(node)) => {
+                    node.fn_decl().map_or(item.ident(tcx).span, |decl| decl.output.span())
+                }
+                _ => item.ident(tcx).span,
+            };
+
+            ObjectSafetyViolation::Method(item.name, v, span)
+        }),
+        // Associated types can only be object safe if they have `Self: Sized` bounds.
+        ty::AssocKind::Type => {
+            if !tcx.features().generic_associated_types_extended
+                && !tcx.generics_of(item.def_id).params.is_empty()
+                && item.opt_rpitit_info.is_none()
+            {
+                Some(ObjectSafetyViolation::GAT(item.name, item.ident(tcx).span))
+            } else {
+                // We will permit associated types if they are explicitly mentioned in the trait object.
+                // We can't check this here, as here we only check if it is guaranteed to not be possible.
+                None
             }
-            _ => method.ident(tcx).span,
-        };
-        (v, span)
-    })
+        }
+    }
 }
 
 /// Returns `Some(_)` if this method cannot be called on a trait