From 1251b2b66759ba39d538f427cb97e00e765a5b01 Mon Sep 17 00:00:00 2001 From: James Dietz Date: Mon, 4 Nov 2024 16:38:54 -0500 Subject: [PATCH 1/2] add check and note for recursive default impl --- compiler/rustc_mir_build/messages.ftl | 2 ++ compiler/rustc_mir_build/src/errors.rs | 6 +++++ compiler/rustc_mir_build/src/lints.rs | 23 ++++++++++++++++++- .../lint/lint-unconditional-recursion.stderr | 2 ++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index 55149570dbc4d..f066967542db6 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -265,6 +265,8 @@ mir_build_pointer_pattern = function pointers and raw pointers not derived from mir_build_privately_uninhabited = pattern `{$witness_1}` is currently uninhabited, but this variant contains private fields which may become inhabited in the future +mir_build_recursive_default_impl = ..default() in the Default impl does not apply a default for each struct field + mir_build_rust_2024_incompatible_pat = patterns are not allowed to reset the default binding mode in edition 2024 mir_build_rustc_box_attribute_error = `#[rustc_box]` attribute used incorrectly diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 676f7c98b8f88..71d8e178d948d 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -18,10 +18,16 @@ use crate::fluent_generated as fluent; pub(crate) struct UnconditionalRecursion { #[label] pub(crate) span: Span, + #[subdiagnostic] + pub(crate) default_impl_note: Option, #[label(mir_build_unconditional_recursion_call_site_label)] pub(crate) call_sites: Vec, } +#[derive(Subdiagnostic)] +#[help(mir_build_recursive_default_impl)] +pub(crate) struct RecursiveDefaultImpl {} + #[derive(LintDiagnostic)] #[diag(mir_build_call_to_deprecated_safe_fn_requires_unsafe)] pub(crate) struct CallToDeprecatedSafeFnRequiresUnsafe { diff --git a/compiler/rustc_mir_build/src/lints.rs b/compiler/rustc_mir_build/src/lints.rs index 5cf33868adeee..fae4e20388056 100644 --- a/compiler/rustc_mir_build/src/lints.rs +++ b/compiler/rustc_mir_build/src/lints.rs @@ -9,7 +9,7 @@ use rustc_middle::ty::{self, GenericArg, GenericArgs, Instance, Ty, TyCtxt}; use rustc_session::lint::builtin::UNCONDITIONAL_RECURSION; use rustc_span::Span; -use crate::errors::UnconditionalRecursion; +use crate::errors::{RecursiveDefaultImpl, UnconditionalRecursion}; pub(crate) fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) { check_call_recursion(tcx, body); @@ -31,6 +31,24 @@ fn check_call_recursion<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) { check_recursion(tcx, body, CallRecursion { trait_args }) } } +use rustc_span::def_id::LocalDefId; +use rustc_span::sym; +fn is_default_impl<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> bool { + if tcx.def_kind(def_id) == DefKind::AssocFn { + // Check if this is a trait impl and get the defid of the trait if it is + if let Some(parent_def_id) = tcx.opt_parent(def_id.into()) { + if tcx.def_kind(parent_def_id) == (DefKind::Impl { of_trait: true }) { + if let Some(trait_def_id) = tcx.trait_id_of_impl(parent_def_id) { + // check if it is a default impl + if tcx.get_diagnostic_name(trait_def_id) == Some(sym::Default) { + return true; + } + } + } + } + } + false +} fn check_recursion<'tcx>( tcx: TyCtxt<'tcx>, @@ -54,9 +72,12 @@ fn check_recursion<'tcx>( let sp = tcx.def_span(def_id); let hir_id = tcx.local_def_id_to_hir_id(def_id); + let default_impl_note = + { if is_default_impl(tcx, def_id) { Some(RecursiveDefaultImpl {}) } else { None } }; tcx.emit_node_span_lint(UNCONDITIONAL_RECURSION, hir_id, sp, UnconditionalRecursion { span: sp, call_sites: vis.reachable_recursive_calls, + default_impl_note, }); } } diff --git a/tests/ui/lint/lint-unconditional-recursion.stderr b/tests/ui/lint/lint-unconditional-recursion.stderr index d75754bf9f900..d71aabca6c13f 100644 --- a/tests/ui/lint/lint-unconditional-recursion.stderr +++ b/tests/ui/lint/lint-unconditional-recursion.stderr @@ -122,6 +122,7 @@ LL | let x = Default::default(); | ------------------ recursive call site | = help: a `loop` may express intention better if this is on purpose + = help: ..default() in the Default impl does not apply a default for each struct field error: function cannot return without recursing --> $DIR/lint-unconditional-recursion.rs:102:5 @@ -196,6 +197,7 @@ LL | ..Default::default() | ------------------ recursive call site | = help: a `loop` may express intention better if this is on purpose + = help: ..default() in the Default impl does not apply a default for each struct field error: aborting due to 18 previous errors From d9ee42055d7574b2a14f9019a9a68f0ef19cb9f1 Mon Sep 17 00:00:00 2001 From: James Dietz Date: Thu, 21 Nov 2024 14:10:15 -0500 Subject: [PATCH 2/2] apply comments --- compiler/rustc_mir_build/messages.ftl | 2 +- compiler/rustc_mir_build/src/errors.rs | 8 ++--- compiler/rustc_mir_build/src/lints.rs | 32 ++++++++----------- .../lint/lint-unconditional-recursion.stderr | 4 +-- 4 files changed, 18 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl index f066967542db6..d807704a8ef6b 100644 --- a/compiler/rustc_mir_build/messages.ftl +++ b/compiler/rustc_mir_build/messages.ftl @@ -265,7 +265,7 @@ mir_build_pointer_pattern = function pointers and raw pointers not derived from mir_build_privately_uninhabited = pattern `{$witness_1}` is currently uninhabited, but this variant contains private fields which may become inhabited in the future -mir_build_recursive_default_impl = ..default() in the Default impl does not apply a default for each struct field +mir_build_recursive_default_impl = calling `..Default::default()` in a `Default` implementation leads to infinite recursion, and does not initialize the fields of a struct or enum mir_build_rust_2024_incompatible_pat = patterns are not allowed to reset the default binding mode in edition 2024 diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 71d8e178d948d..c831564363f7f 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/compiler/rustc_mir_build/src/errors.rs @@ -18,16 +18,12 @@ use crate::fluent_generated as fluent; pub(crate) struct UnconditionalRecursion { #[label] pub(crate) span: Span, - #[subdiagnostic] - pub(crate) default_impl_note: Option, + #[help(mir_build_recursive_default_impl)] + pub(crate) default_impl_note: Option<()>, #[label(mir_build_unconditional_recursion_call_site_label)] pub(crate) call_sites: Vec, } -#[derive(Subdiagnostic)] -#[help(mir_build_recursive_default_impl)] -pub(crate) struct RecursiveDefaultImpl {} - #[derive(LintDiagnostic)] #[diag(mir_build_call_to_deprecated_safe_fn_requires_unsafe)] pub(crate) struct CallToDeprecatedSafeFnRequiresUnsafe { diff --git a/compiler/rustc_mir_build/src/lints.rs b/compiler/rustc_mir_build/src/lints.rs index fae4e20388056..e4e09782f26bf 100644 --- a/compiler/rustc_mir_build/src/lints.rs +++ b/compiler/rustc_mir_build/src/lints.rs @@ -7,9 +7,10 @@ use rustc_hir::def::DefKind; use rustc_middle::mir::{self, BasicBlock, BasicBlocks, Body, Terminator, TerminatorKind}; use rustc_middle::ty::{self, GenericArg, GenericArgs, Instance, Ty, TyCtxt}; use rustc_session::lint::builtin::UNCONDITIONAL_RECURSION; -use rustc_span::Span; +use rustc_span::def_id::LocalDefId; +use rustc_span::{Span, sym}; -use crate::errors::{RecursiveDefaultImpl, UnconditionalRecursion}; +use crate::errors::UnconditionalRecursion; pub(crate) fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) { check_call_recursion(tcx, body); @@ -31,21 +32,16 @@ fn check_call_recursion<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) { check_recursion(tcx, body, CallRecursion { trait_args }) } } -use rustc_span::def_id::LocalDefId; -use rustc_span::sym; + fn is_default_impl<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> bool { - if tcx.def_kind(def_id) == DefKind::AssocFn { - // Check if this is a trait impl and get the defid of the trait if it is - if let Some(parent_def_id) = tcx.opt_parent(def_id.into()) { - if tcx.def_kind(parent_def_id) == (DefKind::Impl { of_trait: true }) { - if let Some(trait_def_id) = tcx.trait_id_of_impl(parent_def_id) { - // check if it is a default impl - if tcx.get_diagnostic_name(trait_def_id) == Some(sym::Default) { - return true; - } - } - } - } + // Check if this is a trait impl and get the defid of the trait if it is + if tcx.def_kind(def_id) == DefKind::AssocFn + && let Some(parent_def_id) = tcx.opt_parent(def_id.into()) + && tcx.def_kind(parent_def_id) == (DefKind::Impl { of_trait: true }) + && let Some(trait_def_id) = tcx.trait_id_of_impl(parent_def_id) + { + // check if it is a default impl + return tcx.get_diagnostic_name(trait_def_id) == Some(sym::Default); } false } @@ -72,12 +68,10 @@ fn check_recursion<'tcx>( let sp = tcx.def_span(def_id); let hir_id = tcx.local_def_id_to_hir_id(def_id); - let default_impl_note = - { if is_default_impl(tcx, def_id) { Some(RecursiveDefaultImpl {}) } else { None } }; tcx.emit_node_span_lint(UNCONDITIONAL_RECURSION, hir_id, sp, UnconditionalRecursion { span: sp, call_sites: vis.reachable_recursive_calls, - default_impl_note, + default_impl_note: is_default_impl(tcx, def_id).then(|| ()), }); } } diff --git a/tests/ui/lint/lint-unconditional-recursion.stderr b/tests/ui/lint/lint-unconditional-recursion.stderr index d71aabca6c13f..e913e229e2ec8 100644 --- a/tests/ui/lint/lint-unconditional-recursion.stderr +++ b/tests/ui/lint/lint-unconditional-recursion.stderr @@ -122,7 +122,7 @@ LL | let x = Default::default(); | ------------------ recursive call site | = help: a `loop` may express intention better if this is on purpose - = help: ..default() in the Default impl does not apply a default for each struct field + = help: calling `..Default::default()` in a `Default` implementation leads to infinite recursion, and does not initialize the fields of a struct or enum error: function cannot return without recursing --> $DIR/lint-unconditional-recursion.rs:102:5 @@ -197,7 +197,7 @@ LL | ..Default::default() | ------------------ recursive call site | = help: a `loop` may express intention better if this is on purpose - = help: ..default() in the Default impl does not apply a default for each struct field + = help: calling `..Default::default()` in a `Default` implementation leads to infinite recursion, and does not initialize the fields of a struct or enum error: aborting due to 18 previous errors