From 5ee4db4e05ecb845fa99b8863a080014f7ada9cb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 19 Apr 2024 22:54:53 -0400 Subject: [PATCH] Warn/error on self ctor from outer item in inner item --- compiler/rustc_hir_typeck/messages.ftl | 4 + compiler/rustc_hir_typeck/src/errors.rs | 28 ++++++ .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 40 ++++++++- compiler/rustc_lint_defs/src/builtin.rs | 42 +++++++++ .../do-not-ice-on-note_and_explain.rs | 17 ++-- .../do-not-ice-on-note_and_explain.stderr | 85 ++----------------- tests/ui/self/self-ctor-nongeneric.rs | 4 + tests/ui/self/self-ctor-nongeneric.stderr | 27 ++++++ tests/ui/self/self-ctor.rs | 14 +++ tests/ui/self/self-ctor.stderr | 21 +++++ 10 files changed, 199 insertions(+), 83 deletions(-) create mode 100644 tests/ui/self/self-ctor-nongeneric.stderr create mode 100644 tests/ui/self/self-ctor.rs create mode 100644 tests/ui/self/self-ctor.stderr diff --git a/compiler/rustc_hir_typeck/messages.ftl b/compiler/rustc_hir_typeck/messages.ftl index 72b95a9603d..6f499947d5c 100644 --- a/compiler/rustc_hir_typeck/messages.ftl +++ b/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 diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs index f250b909596..3f12f252654 100644 --- a/compiler/rustc_hir_typeck/src/errors.rs +++ b/compiler/rustc_hir_typeck/src/errors.rs @@ -651,3 +651,31 @@ pub enum SuggestBoxingForReturnImplTrait { ends: Vec, }, } + +#[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, +} + +#[derive(LintDiagnostic)] +#[diag(hir_typeck_self_ctor_from_outer_item)] +pub struct SelfCtorFromOuterItemLint { + #[label] + pub impl_span: Span, + #[subdiagnostic] + pub sugg: Option, +} + +#[derive(Subdiagnostic)] +#[suggestion(hir_typeck_suggestion, code = "{name}", applicability = "machine-applicable")] +pub struct ReplaceWithName { + #[primary_span] + pub span: Span, + pub name: String, +} diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 6e8ef044452..dc927e66765 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/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; @@ -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(); diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 53694545772..3ddf7f9e6c0 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/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 ", + }; +} + declare_lint! { /// The `semicolon_in_expressions_from_macros` lint detects trailing semicolons /// in macro bodies when the macro is invoked in expression position. diff --git a/tests/ui/malformed/do-not-ice-on-note_and_explain.rs b/tests/ui/malformed/do-not-ice-on-note_and_explain.rs index e65276fb738..be0b18a00d2 100644 --- a/tests/ui/malformed/do-not-ice-on-note_and_explain.rs +++ b/tests/ui/malformed/do-not-ice-on-note_and_explain.rs @@ -1,7 +1,12 @@ struct A(B); -implA{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 A { + fn d() { + fn d() { + Self(1) + //~^ ERROR can't reference `Self` constructor from outer item + } + } +} + +fn main() {} diff --git a/tests/ui/malformed/do-not-ice-on-note_and_explain.stderr b/tests/ui/malformed/do-not-ice-on-note_and_explain.stderr index 41d0f17366b..11a8c01e490 100644 --- a/tests/ui/malformed/do-not-ice-on-note_and_explain.stderr +++ b/tests/ui/malformed/do-not-ice-on-note_and_explain.stderr @@ -1,79 +1,12 @@ -error[E0601]: `main` function not found in crate `do_not_ice_on_note_and_explain` - --> $DIR/do-not-ice-on-note_and_explain.rs:2:37 +error[E0401]: can't reference `Self` constructor from outer item + --> $DIR/do-not-ice-on-note_and_explain.rs:6:13 | -LL | implA{fn d(){fn d(){Self(1)}}} - | ^ consider adding a `main` function to `$DIR/do-not-ice-on-note_and_explain.rs` +LL | impl A { + | ------------ the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +... +LL | Self(1) + | ^^^^ help: replace `Self` with the actual type: `A` -error[E0277]: the size for values of type `B` cannot be known at compilation time - --> $DIR/do-not-ice-on-note_and_explain.rs:2:32 - | -LL | implA{fn d(){fn d(){Self(1)}}} - | - ---- ^ doesn't have a size known at compile-time - | | | - | | required by a bound introduced by this call - | this type parameter needs to be `Sized` - | -note: required by a bound in `A` - --> $DIR/do-not-ice-on-note_and_explain.rs:1:10 - | -LL | struct A(B); - | ^ required by this bound in `A` +error: aborting due to 1 previous error -error[E0308]: mismatched types - --> $DIR/do-not-ice-on-note_and_explain.rs:2:32 - | -LL | implA{fn d(){fn d(){Self(1)}}} - | ---- ^ expected type parameter `B`, found integer - | | - | arguments to this function are incorrect - | - = note: expected type parameter `B` - found type `{integer}` -note: tuple struct defined here - --> $DIR/do-not-ice-on-note_and_explain.rs:1:8 - | -LL | struct A(B); - | ^ - -error[E0308]: mismatched types - --> $DIR/do-not-ice-on-note_and_explain.rs:2:27 - | -LL | implA{fn d(){fn d(){Self(1)}}} - | ^^^^^^^ expected `()`, found `A` - | - = note: expected unit type `()` - found struct `A` -help: consider using a semicolon here - | -LL | implA{fn d(){fn d(){Self(1);}}} - | + -help: try adding a return type - | -LL | implA{fn d(){fn d() -> A{Self(1)}}} - | +++++++ - -error[E0277]: the size for values of type `B` cannot be known at compilation time - --> $DIR/do-not-ice-on-note_and_explain.rs:2:27 - | -LL | implA{fn d(){fn d(){Self(1)}}} - | - ^^^^^^^ doesn't have a size known at compile-time - | | - | this type parameter needs to be `Sized` - | -note: required by an implicit `Sized` bound in `A` - --> $DIR/do-not-ice-on-note_and_explain.rs:1:10 - | -LL | struct A(B); - | ^ required by the implicit `Sized` requirement on this type parameter in `A` -help: you could relax the implicit `Sized` bound on `B` if it were used through indirection like `&B` or `Box` - --> $DIR/do-not-ice-on-note_and_explain.rs:1:10 - | -LL | struct A(B); - | ^ - ...if indirection were used here: `Box` - | | - | this could be changed to `B: ?Sized`... - -error: aborting due to 5 previous errors - -Some errors have detailed explanations: E0277, E0308, E0601. -For more information about an error, try `rustc --explain E0277`. +For more information about this error, try `rustc --explain E0401`. diff --git a/tests/ui/self/self-ctor-nongeneric.rs b/tests/ui/self/self-ctor-nongeneric.rs index 0594e87a0a4..c32cf9df694 100644 --- a/tests/ui/self/self-ctor-nongeneric.rs +++ b/tests/ui/self/self-ctor-nongeneric.rs @@ -6,8 +6,12 @@ struct S0(usize); impl S0 { fn foo() { const C: S0 = Self(0); + //~^ WARN can't reference `Self` constructor from outer item + //~| WARN this was previously accepted by the compiler but is being phased out fn bar() -> S0 { Self(0) + //~^ WARN can't reference `Self` constructor from outer item + //~| WARN this was previously accepted by the compiler but is being phased out } } } diff --git a/tests/ui/self/self-ctor-nongeneric.stderr b/tests/ui/self/self-ctor-nongeneric.stderr new file mode 100644 index 00000000000..6c03c6f3e38 --- /dev/null +++ b/tests/ui/self/self-ctor-nongeneric.stderr @@ -0,0 +1,27 @@ +warning: can't reference `Self` constructor from outer item + --> $DIR/self-ctor-nongeneric.rs:8:23 + | +LL | impl S0 { + | ------- the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +LL | fn foo() { +LL | const C: S0 = Self(0); + | ^^^^ help: replace `Self` with the actual type: `S0` + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #124186 + = note: `#[warn(self_constructor_from_outer_item)]` on by default + +warning: can't reference `Self` constructor from outer item + --> $DIR/self-ctor-nongeneric.rs:12:13 + | +LL | impl S0 { + | ------- the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +... +LL | Self(0) + | ^^^^ help: replace `Self` with the actual type: `S0` + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #124186 + +warning: 2 warnings emitted + diff --git a/tests/ui/self/self-ctor.rs b/tests/ui/self/self-ctor.rs new file mode 100644 index 00000000000..d166499f884 --- /dev/null +++ b/tests/ui/self/self-ctor.rs @@ -0,0 +1,14 @@ +struct S0(T); + +impl S0 { + fn foo() { + const C: S0 = Self(0); + //~^ ERROR can't reference `Self` constructor from outer item + fn bar() -> S0 { + Self(0) + //~^ ERROR can't reference `Self` constructor from outer item + } + } +} + +fn main() {} diff --git a/tests/ui/self/self-ctor.stderr b/tests/ui/self/self-ctor.stderr new file mode 100644 index 00000000000..0cb22baaa1a --- /dev/null +++ b/tests/ui/self/self-ctor.stderr @@ -0,0 +1,21 @@ +error[E0401]: can't reference `Self` constructor from outer item + --> $DIR/self-ctor.rs:5:28 + | +LL | impl S0 { + | ------------- the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +LL | fn foo() { +LL | const C: S0 = Self(0); + | ^^^^ help: replace `Self` with the actual type: `S0` + +error[E0401]: can't reference `Self` constructor from outer item + --> $DIR/self-ctor.rs:8:13 + | +LL | impl S0 { + | ------------- the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +... +LL | Self(0) + | ^^^^ help: replace `Self` with the actual type: `S0` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0401`.