Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
May 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1162,6 +1163,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 @@ -244,20 +244,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 @@ -289,20 +275,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
Loading