Auto merge of rust-lang#124187 - compiler-errors:self-ctor, r=petrochenkov

Warn (or error) when `Self` ctor from outer item is referenced in inner nested item

This implements a warning `SELF_CONSTRUCTOR_FROM_OUTER_ITEM` when a self constructor from an outer impl is referenced in an inner nested item. This is a proper fix mentioned rust-lang#117246 (comment).

This warning is additionally bumped to a hard error when the self type references generic parameters, since it's almost always going to ICE, and is basically *never* correct to do.

This also reverts part of rust-lang#117246, since I believe this is the proper fix and we shouldn't need the helper functions (`opt_param_at`/`opt_type_param`) any longer, since they shouldn't really ever be used in cases where we don't have this problem.

Author: bors

compiler/rustc_hir_analysis/src/check/intrinsic.rs

@@ -164,9 +164,8 @@ pub fn check_intrinsic_type(
 ) {
     let generics = tcx.generics_of(intrinsic_id);
     let param = |n| {
-        if let Some(&ty::GenericParamDef {
-            name, kind: ty::GenericParamDefKind::Type { .. }, ..
-        }) = generics.opt_param_at(n as usize, tcx)
+        if let &ty::GenericParamDef { name, kind: ty::GenericParamDefKind::Type { .. }, .. } =
+            generics.param_at(n as usize, tcx)
         {
             Ty::new_param(tcx, n, name)
         } else {

compiler/rustc_hir_typeck/messages.ftl

@@ -137,6 +137,10 @@ hir_typeck_rpit_change_return_type = you could change the return type to be a bo
 hir_typeck_rustcall_incorrect_args =
     functions with the "rust-call" ABI must take a single non-self tuple argument
 
+hir_typeck_self_ctor_from_outer_item = can't reference `Self` constructor from outer item
+    .label = the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference
+    .suggestion = replace `Self` with the actual type
+
 hir_typeck_struct_expr_non_exhaustive =
     cannot create non-exhaustive {$what} using struct expression
 

compiler/rustc_hir_typeck/src/errors.rs

@@ -651,3 +651,31 @@ pub enum SuggestBoxingForReturnImplTrait {
         ends: Vec<Span>,
     },
 }
+
+#[derive(Diagnostic)]
+#[diag(hir_typeck_self_ctor_from_outer_item, code = E0401)]
+pub struct SelfCtorFromOuterItem {
+    #[primary_span]
+    pub span: Span,
+    #[label]
+    pub impl_span: Span,
+    #[subdiagnostic]
+    pub sugg: Option<ReplaceWithName>,
+}
+
+#[derive(LintDiagnostic)]
+#[diag(hir_typeck_self_ctor_from_outer_item)]
+pub struct SelfCtorFromOuterItemLint {
+    #[label]
+    pub impl_span: Span,
+    #[subdiagnostic]
+    pub sugg: Option<ReplaceWithName>,
+}
+
+#[derive(Subdiagnostic)]
+#[suggestion(hir_typeck_suggestion, code = "{name}", applicability = "machine-applicable")]
+pub struct ReplaceWithName {
+    #[primary_span]
+    pub span: Span,
+    pub name: String,
+}

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

@@ -1,5 +1,5 @@
 use crate::callee::{self, DeferredCallResolution};
-use crate::errors::CtorIsPrivate;
+use crate::errors::{self, CtorIsPrivate};
 use crate::method::{self, MethodCallee, SelfSource};
 use crate::rvalue_scopes;
 use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy};
@@ -21,6 +21,7 @@ use rustc_hir_analysis::hir_ty_lowering::{
 use rustc_infer::infer::canonical::{Canonical, OriginalQueryValues, QueryResponse};
 use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282;
 use rustc_infer::infer::{DefineOpaqueTypes, InferResult};
+use rustc_lint::builtin::SELF_CONSTRUCTOR_FROM_OUTER_ITEM;
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
 use rustc_middle::ty::error::TypeError;
 use rustc_middle::ty::fold::TypeFoldable;
@@ -1156,6 +1157,43 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 span,
                 tcx.at(span).type_of(impl_def_id).instantiate_identity(),
             );
+
+            // Firstly, check that this SelfCtor even comes from the item we're currently
+            // typechecking. This can happen because we never validated the resolution of
+            // SelfCtors, and when we started doing so, we noticed regressions. After
+            // sufficiently long time, we can remove this check and turn it into a hard
+            // error in `validate_res_from_ribs` -- it's just difficult to tell whether the
+            // self type has any generic types during rustc_resolve, which is what we use
+            // to determine if this is a hard error or warning.
+            if std::iter::successors(Some(self.body_id.to_def_id()), |def_id| {
+                self.tcx.generics_of(def_id).parent
+            })
+            .all(|def_id| def_id != impl_def_id)
+            {
+                let sugg = ty.normalized.ty_adt_def().map(|def| errors::ReplaceWithName {
+                    span: path_span,
+                    name: self.tcx.item_name(def.did()).to_ident_string(),
+                });
+                if ty.raw.has_param() {
+                    let guar = self.tcx.dcx().emit_err(errors::SelfCtorFromOuterItem {
+                        span: path_span,
+                        impl_span: tcx.def_span(impl_def_id),
+                        sugg,
+                    });
+                    return (Ty::new_error(self.tcx, guar), res);
+                } else {
+                    self.tcx.emit_node_span_lint(
+                        SELF_CONSTRUCTOR_FROM_OUTER_ITEM,
+                        hir_id,
+                        path_span,
+                        errors::SelfCtorFromOuterItemLint {
+                            impl_span: tcx.def_span(impl_def_id),
+                            sugg,
+                        },
+                    );
+                }
+            }
+
             match ty.normalized.ty_adt_def() {
                 Some(adt_def) if adt_def.has_ctor() => {
                     let (ctor_kind, ctor_def_id) = adt_def.non_enum_variant().ctor.unwrap();

compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs

@@ -54,17 +54,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     }
                     (ty::Param(expected), ty::Param(found)) => {
                         let generics = tcx.generics_of(body_owner_def_id);
-                        if let Some(param) = generics.opt_type_param(expected, tcx) {
-                            let e_span = tcx.def_span(param.def_id);
-                            if !sp.contains(e_span) {
-                                diag.span_label(e_span, "expected type parameter");
-                            }
+                        let e_span = tcx.def_span(generics.type_param(expected, tcx).def_id);
+                        if !sp.contains(e_span) {
+                            diag.span_label(e_span, "expected type parameter");
                         }
-                        if let Some(param) = generics.opt_type_param(found, tcx) {
-                            let f_span = tcx.def_span(param.def_id);
-                            if !sp.contains(f_span) {
-                                diag.span_label(f_span, "found type parameter");
-                            }
+                        let f_span = tcx.def_span(generics.type_param(found, tcx).def_id);
+                        if !sp.contains(f_span) {
+                            diag.span_label(f_span, "found type parameter");
                         }
                         diag.note(
                             "a type parameter was expected, but a different one was found; \
@@ -87,29 +83,22 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     | (ty::Alias(ty::Projection, proj), ty::Param(p))
                         if !tcx.is_impl_trait_in_trait(proj.def_id) =>
                     {
-                        let parent = tcx
-                            .generics_of(body_owner_def_id)
-                            .opt_type_param(p, tcx)
-                            .and_then(|param| {
-                                let p_def_id = param.def_id;
-                                let p_span = tcx.def_span(p_def_id);
-                                let expected = match (values.expected.kind(), values.found.kind()) {
-                                    (ty::Param(_), _) => "expected ",
-                                    (_, ty::Param(_)) => "found ",
-                                    _ => "",
-                                };
-                                if !sp.contains(p_span) {
-                                    diag.span_label(
-                                        p_span,
-                                        format!("{expected}this type parameter"),
-                                    );
-                                }
-                                p_def_id.as_local().and_then(|id| {
-                                    let local_id = tcx.local_def_id_to_hir_id(id);
-                                    let generics = tcx.parent_hir_node(local_id).generics()?;
-                                    Some((id, generics))
-                                })
-                            });
+                        let param = tcx.generics_of(body_owner_def_id).type_param(p, tcx);
+                        let p_def_id = param.def_id;
+                        let p_span = tcx.def_span(p_def_id);
+                        let expected = match (values.expected.kind(), values.found.kind()) {
+                            (ty::Param(_), _) => "expected ",
+                            (_, ty::Param(_)) => "found ",
+                            _ => "",
+                        };
+                        if !sp.contains(p_span) {
+                            diag.span_label(p_span, format!("{expected}this type parameter"));
+                        }
+                        let parent = p_def_id.as_local().and_then(|id| {
+                            let local_id = tcx.local_def_id_to_hir_id(id);
+                            let generics = tcx.parent_hir_node(local_id).generics()?;
+                            Some((id, generics))
+                        });
                         let mut note = true;
                         if let Some((local_id, generics)) = parent {
                             // Synthesize the associated type restriction `Add<Output = Expected>`.
@@ -183,16 +172,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     (ty::Param(p), ty::Dynamic(..) | ty::Alias(ty::Opaque, ..))
                     | (ty::Dynamic(..) | ty::Alias(ty::Opaque, ..), ty::Param(p)) => {
                         let generics = tcx.generics_of(body_owner_def_id);
-                        if let Some(param) = generics.opt_type_param(p, tcx) {
-                            let p_span = tcx.def_span(param.def_id);
-                            let expected = match (values.expected.kind(), values.found.kind()) {
-                                (ty::Param(_), _) => "expected ",
-                                (_, ty::Param(_)) => "found ",
-                                _ => "",
-                            };
-                            if !sp.contains(p_span) {
-                                diag.span_label(p_span, format!("{expected}this type parameter"));
-                            }
+                        let p_span = tcx.def_span(generics.type_param(p, tcx).def_id);
+                        let expected = match (values.expected.kind(), values.found.kind()) {
+                            (ty::Param(_), _) => "expected ",
+                            (_, ty::Param(_)) => "found ",
+                            _ => "",
+                        };
+                        if !sp.contains(p_span) {
+                            diag.span_label(p_span, format!("{expected}this type parameter"));
                         }
                         diag.help("type parameters must be constrained to match other types");
                         if tcx.sess.teach(diag.code.unwrap()) {
@@ -233,11 +220,9 @@ impl<T> Trait<T> for X {
                         ty::Closure(..) | ty::CoroutineClosure(..) | ty::Coroutine(..),
                     ) => {
                         let generics = tcx.generics_of(body_owner_def_id);
-                        if let Some(param) = generics.opt_type_param(p, tcx) {
-                            let p_span = tcx.def_span(param.def_id);
-                            if !sp.contains(p_span) {
-                                diag.span_label(p_span, "expected this type parameter");
-                            }
+                        let p_span = tcx.def_span(generics.type_param(p, tcx).def_id);
+                        if !sp.contains(p_span) {
+                            diag.span_label(p_span, "expected this type parameter");
                         }
                         diag.help(format!(
                             "every closure has a distinct type and so could not always match the \
@@ -246,16 +231,14 @@ impl<T> Trait<T> for X {
                     }
                     (ty::Param(p), _) | (_, ty::Param(p)) => {
                         let generics = tcx.generics_of(body_owner_def_id);
-                        if let Some(param) = generics.opt_type_param(p, tcx) {
-                            let p_span = tcx.def_span(param.def_id);
-                            let expected = match (values.expected.kind(), values.found.kind()) {
-                                (ty::Param(_), _) => "expected ",
-                                (_, ty::Param(_)) => "found ",
-                                _ => "",
-                            };
-                            if !sp.contains(p_span) {
-                                diag.span_label(p_span, format!("{expected}this type parameter"));
-                            }
+                        let p_span = tcx.def_span(generics.type_param(p, tcx).def_id);
+                        let expected = match (values.expected.kind(), values.found.kind()) {
+                            (ty::Param(_), _) => "expected ",
+                            (_, ty::Param(_)) => "found ",
+                            _ => "",
+                        };
+                        if !sp.contains(p_span) {
+                            diag.span_label(p_span, format!("{expected}this type parameter"));
                         }
                     }
                     (ty::Alias(ty::Projection | ty::Inherent, proj_ty), _)
@@ -545,10 +528,8 @@ impl<T> Trait<T> for X {
             return false;
         };
         let generics = tcx.generics_of(body_owner_def_id);
-        let Some(param) = generics.opt_type_param(param_ty, tcx) else {
-            return false;
-        };
-        let Some(def_id) = param.def_id.as_local() else {
+        let def_id = generics.type_param(param_ty, tcx).def_id;
+        let Some(def_id) = def_id.as_local() else {
             return false;
         };
 

compiler/rustc_lint_defs/src/builtin.rs

@@ -90,6 +90,7 @@ declare_lint_pass! {
         RUST_2021_PREFIXES_INCOMPATIBLE_SYNTAX,
         RUST_2021_PRELUDE_COLLISIONS,
         RUST_2024_INCOMPATIBLE_PAT,
+        SELF_CONSTRUCTOR_FROM_OUTER_ITEM,
         SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
         SINGLE_USE_LIFETIMES,
         SOFT_UNSTABLE,
@@ -3149,6 +3150,47 @@ declare_lint! {
     "detects `#[unstable]` on stable trait implementations for stable types"
 }
 
+declare_lint! {
+    /// The `self_constructor_from_outer_item` lint detects cases where the `Self` constructor
+    /// was silently allowed due to a bug in the resolver, and which may produce surprising
+    /// and unintended behavior.
+    ///
+    /// Using a `Self` type alias from an outer item was never intended, but was silently allowed.
+    /// This is deprecated -- and is a hard error when the `Self` type alias references generics
+    /// that are not in scope.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(self_constructor_from_outer_item)]
+    ///
+    /// struct S0(usize);
+    ///
+    /// impl S0 {
+    ///     fn foo() {
+    ///         const C: S0 = Self(0);
+    ///         fn bar() -> S0 {
+    ///             Self(0)
+    ///         }
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// The `Self` type alias should not be reachable because nested items are not associated with
+    /// the scope of the parameters from the parent item.
+    pub SELF_CONSTRUCTOR_FROM_OUTER_ITEM,
+    Warn,
+    "detect unsupported use of `Self` from outer item",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
+        reference: "issue #124186 <https://github.com/rust-lang/rust/issues/124186>",
+    };
+}
+
 declare_lint! {
     /// The `semicolon_in_expressions_from_macros` lint detects trailing semicolons
     /// in macro bodies when the macro is invoked in expression position.

compiler/rustc_middle/src/ty/generics.rs

@@ -245,20 +245,6 @@ impl<'tcx> Generics {
         }
     }
 
-    /// Returns the `GenericParamDef` with the given index if available.
-    pub fn opt_param_at(
-        &'tcx self,
-        param_index: usize,
-        tcx: TyCtxt<'tcx>,
-    ) -> Option<&'tcx GenericParamDef> {
-        if let Some(index) = param_index.checked_sub(self.parent_count) {
-            self.own_params.get(index)
-        } else {
-            tcx.generics_of(self.parent.expect("parent_count > 0 but no parent?"))
-                .opt_param_at(param_index, tcx)
-        }
-    }
-
     pub fn params_to(&'tcx self, param_index: usize, tcx: TyCtxt<'tcx>) -> &'tcx [GenericParamDef] {
         if let Some(index) = param_index.checked_sub(self.parent_count) {
             &self.own_params[..index]
@@ -290,20 +276,6 @@ impl<'tcx> Generics {
         }
     }
 
-    /// Returns the `GenericParamDef` associated with this `ParamTy` if it belongs to this
-    /// `Generics`.
-    pub fn opt_type_param(
-        &'tcx self,
-        param: ParamTy,
-        tcx: TyCtxt<'tcx>,
-    ) -> Option<&'tcx GenericParamDef> {
-        let param = self.opt_param_at(param.index as usize, tcx)?;
-        match param.kind {
-            GenericParamDefKind::Type { .. } => Some(param),
-            _ => None,
-        }
-    }
-
     /// Returns the `GenericParamDef` associated with this `ParamConst`.
     pub fn const_param(&'tcx self, param: ParamConst, tcx: TyCtxt<'tcx>) -> &GenericParamDef {
         let param = self.param_at(param.index as usize, tcx);

tests/ui/malformed/do-not-ice-on-note_and_explain.rs

@@ -1,7 +1,12 @@
 struct A<B>(B);
-impl<B>A<B>{fn d(){fn d(){Self(1)}}}
-//~^ ERROR the size for values of type `B` cannot be known at compilation time
-//~| ERROR the size for values of type `B` cannot be known at compilation time
-//~| ERROR mismatched types
-//~| ERROR mismatched types
-//~| ERROR `main` function not found in crate
+
+impl<B> A<B> {
+    fn d() {
+        fn d() {
+            Self(1)
+            //~^ ERROR can't reference `Self` constructor from outer item
+        }
+    }
+}
+
+fn main() {}