Skip to content

Commit

Permalink
Auto merge of #124187 - compiler-errors:self-ctor, r=petrochenkov
Browse files Browse the repository at this point in the history
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 #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 #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.
  • Loading branch information
bors committed May 25, 2024
2 parents 36153f1 + a25bb5f commit 21e6de7
Show file tree
Hide file tree
Showing 13 changed files with 244 additions and 176 deletions.
5 changes: 2 additions & 3 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir_typeck/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions compiler/rustc_hir_typeck/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
40 changes: 39 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
105 changes: 43 additions & 62 deletions compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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; \
Expand All @@ -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>`.
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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 \
Expand All @@ -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), _)
Expand Down Expand Up @@ -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;
};

Expand Down
42 changes: 42 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
28 changes: 0 additions & 28 deletions compiler/rustc_middle/src/ty/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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);
Expand Down
17 changes: 11 additions & 6 deletions tests/ui/malformed/do-not-ice-on-note_and_explain.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Loading

0 comments on commit 21e6de7

Please sign in to comment.