Auto merge of rust-lang#128912 - compiler-errors:do-not-recommend-impl, r=<try>

Store `do_not_recommend`-ness in impl header

Alternative to rust-lang#128674

It's less flexible, but also less invasive. Hopefully it's also performant. I'd recommend we think separately about the design for how to gate arbitrary diagnostic attributes moving forward.

Author: bors

compiler/rustc_hir_analysis/src/coherence/builtin.rs

@@ -15,6 +15,7 @@ use rustc_infer::traits::Obligation;
 use rustc_middle::ty::adjustment::CoerceUnsizedInfo;
 use rustc_middle::ty::print::PrintTraitRefExt as _;
 use rustc_middle::ty::{self, suggest_constraining_type_params, Ty, TyCtxt, TypeVisitableExt};
+use rustc_span::symbol::{kw, Ident};
 use rustc_span::{Span, DUMMY_SP};
 use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
 use rustc_trait_selection::traits::misc::{
@@ -103,26 +104,50 @@ fn visit_implementation_of_copy(checker: &Checker<'_>) -> Result<(), ErrorGuaran
 
     let cause = traits::ObligationCause::misc(DUMMY_SP, impl_did);
     match type_allowed_to_implement_copy(tcx, param_env, self_type, cause) {
-        Ok(()) => Ok(()),
+        Ok(()) => {}
         Err(CopyImplementationError::InfringingFields(fields)) => {
             let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span;
-            Err(infringing_fields_error(
+            return Err(infringing_fields_error(
                 tcx,
                 fields.into_iter().map(|(field, ty, reason)| (tcx.def_span(field.did), ty, reason)),
                 LangItem::Copy,
                 impl_did,
                 span,
-            ))
+            ));
         }
         Err(CopyImplementationError::NotAnAdt) => {
             let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span;
-            Err(tcx.dcx().emit_err(errors::CopyImplOnNonAdt { span }))
+            return Err(tcx.dcx().emit_err(errors::CopyImplOnNonAdt { span }));
         }
         Err(CopyImplementationError::HasDestructor) => {
             let span = tcx.hir().expect_item(impl_did).expect_impl().self_ty.span;
-            Err(tcx.dcx().emit_err(errors::CopyImplOnTypeWithDtor { span }))
+            return Err(tcx.dcx().emit_err(errors::CopyImplOnTypeWithDtor { span }));
+        }
+    }
+
+    // Let's additionally lint the fields of the copy impl.
+    if let ty::Adt(def, _) = *self_type.kind() {
+        let impl_span = tcx.def_span(impl_did);
+        let (_, scope) = tcx.adjust_ident_and_get_scope(
+            Ident::new(kw::Empty, impl_span),
+            def.did(),
+            tcx.local_def_id_to_hir_id(impl_did),
+        );
+        if tcx.crate_name(scope.krate).as_str() == "core" {
+            return Ok(());
+        }
+        println!("{scope:?}");
+        let infringing_fields: Vec<_> =
+            def.all_fields().filter(|field| !field.vis.is_accessible_from(scope, tcx)).collect();
+        if !infringing_fields.is_empty() {
+            rustc_middle::span_bug!(
+                impl_span,
+                "{infringing_fields:#?} in {scope:#?} for {impl_did:#?}"
+            );
         }
     }
+
+    Ok(())
 }
 
 fn visit_implementation_of_const_param_ty(

compiler/rustc_hir_analysis/src/collect.rs

@@ -1699,6 +1699,8 @@ fn impl_trait_header(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<ty::ImplTrai
             trait_ref: ty::EarlyBinder::bind(trait_ref),
             safety: impl_.safety,
             polarity: polarity_of_impl(tcx, def_id, impl_, item.span),
+            do_not_recommend: tcx.features().do_not_recommend
+                && tcx.has_attrs_with_path(def_id, &[sym::diagnostic, sym::do_not_recommend]),
         }
     })
 }

compiler/rustc_middle/src/ty/context.rs

@@ -3176,6 +3176,12 @@ impl<'tcx> TyCtxt<'tcx> {
     pub fn impl_polarity(self, def_id: impl IntoQueryParam<DefId>) -> ty::ImplPolarity {
         self.impl_trait_header(def_id).map_or(ty::ImplPolarity::Positive, |h| h.polarity)
     }
+
+    /// Whether this is a trait implementation that has `#[diagnostic::do_not_recommend]`
+    pub fn do_not_recommend_impl(self, def_id: DefId) -> bool {
+        matches!(self.def_kind(def_id), DefKind::Impl { of_trait: true })
+            && self.impl_trait_header(def_id).is_some_and(|header| header.do_not_recommend)
+    }
 }
 
 /// Parameter attributes that can only be determined by examining the body of a function instead

compiler/rustc_middle/src/ty/mod.rs

@@ -262,6 +262,7 @@ pub struct ImplTraitHeader<'tcx> {
     pub trait_ref: ty::EarlyBinder<'tcx, ty::TraitRef<'tcx>>,
     pub polarity: ImplPolarity,
     pub safety: hir::Safety,
+    pub do_not_recommend: bool,
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug, TypeFoldable, TypeVisitable)]

compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs

@@ -687,10 +687,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
         let mut applied_do_not_recommend = false;
         loop {
             if let ObligationCauseCode::ImplDerived(ref c) = base_cause {
-                if self.tcx.has_attrs_with_path(
-                    c.impl_or_alias_def_id,
-                    &[sym::diagnostic, sym::do_not_recommend],
-                ) {
+                if self.tcx.do_not_recommend_impl(c.impl_or_alias_def_id) {
                     let code = (*c.derived.parent_code).clone();
                     obligation.cause.map_code(|_| code);
                     obligation.predicate = c.derived.parent_trait_pred.upcast(self.tcx);
@@ -1630,11 +1627,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                 .tcx
                 .all_impls(def_id)
                 // ignore `do_not_recommend` items
-                .filter(|def_id| {
-                    !self
-                        .tcx
-                        .has_attrs_with_path(*def_id, &[sym::diagnostic, sym::do_not_recommend])
-                })
+                .filter(|def_id| !self.tcx.do_not_recommend_impl(*def_id))
                 // Ignore automatically derived impls and `!Trait` impls.
                 .filter_map(|def_id| self.tcx.impl_trait_header(def_id))
                 .filter_map(|header| {
@@ -1904,12 +1897,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
         let impl_candidates = impl_candidates
             .into_iter()
             .cloned()
-            .filter(|cand| {
-                !self.tcx.has_attrs_with_path(
-                    cand.impl_def_id,
-                    &[sym::diagnostic, sym::do_not_recommend],
-                )
-            })
+            .filter(|cand| !self.tcx.do_not_recommend_impl(cand.impl_def_id))
             .collect::<Vec<_>>();
 
         let def_id = trait_ref.def_id();

compiler/rustc_trait_selection/src/solve/fulfill.rs

@@ -13,7 +13,6 @@ use rustc_middle::bug;
 use rustc_middle::ty::error::{ExpectedFound, TypeError};
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_next_trait_solver::solve::{GenerateProofTree, SolverDelegateEvalExt as _};
-use rustc_span::symbol::sym;
 
 use super::delegate::SolverDelegate;
 use super::inspect::{self, ProofTreeInferCtxtExt, ProofTreeVisitor};
@@ -440,10 +439,7 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
             source: CandidateSource::Impl(impl_def_id),
             result: _,
         } = candidate.kind()
-            && goal
-                .infcx()
-                .tcx
-                .has_attrs_with_path(impl_def_id, &[sym::diagnostic, sym::do_not_recommend])
+            && goal.infcx().tcx.do_not_recommend_impl(impl_def_id)
         {
             return ControlFlow::Break(self.obligation.clone());
         }

tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.rs

@@ -0,0 +1,21 @@
+#![allow(unknown_or_malformed_diagnostic_attributes)]
+
+trait Foo {}
+
+#[diagnostic::do_not_recommend]
+impl<A> Foo for (A,) {}
+
+#[diagnostic::do_not_recommend]
+impl<A, B> Foo for (A, B) {}
+
+#[diagnostic::do_not_recommend]
+impl<A, B, C> Foo for (A, B, C) {}
+
+impl Foo for i32 {}
+
+fn check(a: impl Foo) {}
+
+fn main() {
+    check(());
+    //~^ ERROR the trait bound `(): Foo` is not satisfied
+}

tests/ui/diagnostic_namespace/do_not_recommend/do_not_apply_attribute_without_feature_flag.stderr

@@ -0,0 +1,21 @@
+error[E0277]: the trait bound `(): Foo` is not satisfied
+  --> $DIR/do_not_apply_attribute_without_feature_flag.rs:19:11
+   |
+LL |     check(());
+   |     ----- ^^ the trait `Foo` is not implemented for `()`
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the following other types implement trait `Foo`:
+             (A, B)
+             (A, B, C)
+             (A,)
+note: required by a bound in `check`
+  --> $DIR/do_not_apply_attribute_without_feature_flag.rs:16:18
+   |
+LL | fn check(a: impl Foo) {}
+   |                  ^^^ required by this bound in `check`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.