diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index b79075ac09b9c..0de9f2ed0ceae 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -110,6 +110,7 @@ declare_lint_pass! { TYVAR_BEHIND_RAW_POINTER, UNCONDITIONAL_PANIC, UNCONDITIONAL_RECURSION, + UNCONSTRUCTABLE_PUB_STRUCT, UNCOVERED_PARAM_IN_PROJECTION, UNEXPECTED_CFGS, UNFULFILLED_LINT_EXPECTATIONS, @@ -755,6 +756,38 @@ declare_lint! { "detect unused, unexported items" } +declare_lint! { + /// The `unconstructable_pub_struct` lint detects public structs that + /// are unused locally and cannot be constructed externally. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// #![deny(unconstructable_pub_struct)] + /// + /// pub struct Foo(i32); + /// # fn main() {} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Unconstructable pub structs may signal a mistake or unfinished code. + /// To silence the warning for individual items, prefix the name with an + /// underscore such as `_Foo`. + /// + /// To preserve this lint, add a field with unit or never types that + /// indicate that the behavior is intentional, or use `PhantomData` as + /// field types if the struct is only used at the type level to check + /// things like well-formedness. + /// + /// Otherwise, consider removing it if the struct is no longer in use. + pub UNCONSTRUCTABLE_PUB_STRUCT, + Allow, + "detects pub structs that are unused locally and cannot be constructed externally" +} + declare_lint! { /// The `unused_attributes` lint detects attributes that were not used by /// the compiler. diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 2aa32dfa0d825..04a5a1bdde082 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1205,6 +1205,7 @@ rustc_queries! { query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx ( LocalDefIdSet, LocalDefIdMap>, + LocalDefIdSet, ) { arena_cache desc { "finding live symbols in crate" } diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index ab7885905a6e7..e4bfc6e954674 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -590,6 +590,10 @@ passes_trait_impl_const_stable = passes_transparent_incompatible = transparent {$target} cannot have other repr hints +passes_unconstructable_pub_struct = + pub struct `{$name}` is unconstructable externally and never constructed locally + .help = this struct may be unused locally and also externally, consider removing it + passes_unexportable_adt_with_private_fields = ADT types with private fields are not exportable .note = `{$field_name}` is private diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index fc33405d455b2..b207592cec272 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -9,7 +9,7 @@ use hir::def_id::{LocalDefIdMap, LocalDefIdSet}; use rustc_abi::FieldIdx; use rustc_data_structures::fx::FxIndexSet; use rustc_errors::MultiSpan; -use rustc_hir::def::{CtorOf, DefKind, Res}; +use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId}; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{self as hir, Node, PatKind, QPath}; @@ -18,12 +18,13 @@ use rustc_middle::middle::privacy::Level; use rustc_middle::query::Providers; use rustc_middle::ty::{self, AssocTag, TyCtxt}; use rustc_middle::{bug, span_bug}; -use rustc_session::lint::builtin::DEAD_CODE; +use rustc_session::lint::builtin::{DEAD_CODE, UNCONSTRUCTABLE_PUB_STRUCT}; use rustc_session::lint::{self, LintExpectationId}; use rustc_span::{Symbol, kw, sym}; use crate::errors::{ - ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UselessAssignment, + ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UnconstructablePubStruct, + UselessAssignment, }; /// Any local definition that may call something in its body block should be explored. For example, @@ -66,6 +67,38 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { } } +fn struct_can_be_constructed_directly(tcx: TyCtxt<'_>, id: LocalDefId) -> bool { + let adt_def = tcx.adt_def(id); + + // Skip types contain fields of unit and never type, + // it's usually intentional to make the type not constructible + if adt_def.all_fields().any(|field| { + let field_type = tcx.type_of(field.did).instantiate_identity(); + field_type.is_unit() || field_type.is_never() + }) { + return true; + } + + return adt_def.all_fields().all(|field| { + let field_type = tcx.type_of(field.did).instantiate_identity(); + // Skip fields of PhantomData, + // cause it's a common way to check things like well-formedness + if field_type.is_phantom_data() { + return true; + } + + field.vis.is_public() + }); +} + +fn method_has_no_receiver(tcx: TyCtxt<'_>, id: LocalDefId) -> bool { + if let Some(fn_decl) = tcx.hir_fn_decl_by_hir_id(tcx.local_def_id_to_hir_id(id)) { + !fn_decl.implicit_self.has_implicit_self() + } else { + true + } +} + /// Determine if a work from the worklist is coming from a `#[allow]` /// or a `#[expect]` of `dead_code` #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] @@ -370,6 +403,28 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { } } + fn mark_live_symbols_until_unsolved_items_fixed( + &mut self, + unsolved_items: &mut Vec, + ) { + self.mark_live_symbols(); + + // We have marked the primary seeds as live. We now need to process unsolved items from traits + // and trait impls: add them to the work list if the trait or the implemented type is live. + let mut items_to_check: Vec<_> = unsolved_items + .extract_if(.., |&mut local_def_id| self.check_impl_or_impl_item_live(local_def_id)) + .collect(); + + while !items_to_check.is_empty() { + self.worklist.extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No))); + self.mark_live_symbols(); + + items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| { + self.check_impl_or_impl_item_live(local_def_id) + })); + } + } + /// Automatically generated items marked with `rustc_trivial_field_reads` /// will be ignored for the purposes of dead code analysis (see PR #85200 /// for discussion). @@ -492,7 +547,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { .and_then(|def_id| def_id.as_local()), ), // impl items are live if the corresponding traits are live - DefKind::Impl { of_trait: true } => ( + DefKind::Impl { .. } => ( local_def_id, self.tcx .impl_trait_ref(local_def_id) @@ -684,6 +739,12 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> { } } +fn has_allow_unconstructable_pub_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { + let hir_id = tcx.local_def_id_to_hir_id(def_id); + let lint_level = tcx.lint_level_at_node(UNCONSTRUCTABLE_PUB_STRUCT, hir_id).level; + matches!(lint_level, lint::Allow | lint::Expect) +} + fn has_allow_dead_code_or_lang_attr( tcx: TyCtxt<'_>, def_id: LocalDefId, @@ -743,7 +804,9 @@ fn maybe_record_as_seed<'tcx>( unsolved_items: &mut Vec, ) { let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, owner_id.def_id); - if let Some(comes_from_allow) = allow_dead_code { + if let Some(comes_from_allow) = allow_dead_code + && !tcx.effective_visibilities(()).is_reachable(owner_id.def_id) + { worklist.push((owner_id.def_id, comes_from_allow)); } @@ -807,6 +870,33 @@ fn create_and_seed_worklist( effective_vis .is_public_at_level(Level::Reachable) .then_some(id) + .filter(|id| { + let (is_seed, is_impl_or_impl_item) = match tcx.def_kind(*id) { + DefKind::Impl { .. } => (false, true), + DefKind::AssocFn => ( + !matches!(tcx.def_kind(tcx.local_parent(*id)), DefKind::Impl { .. }) + || method_has_no_receiver(tcx, *id), + true, + ), + DefKind::Struct => ( + has_allow_unconstructable_pub_struct(tcx, *id) + || struct_can_be_constructed_directly(tcx, *id), + false, + ), + DefKind::Ctor(CtorOf::Struct, CtorKind::Fn) => ( + has_allow_unconstructable_pub_struct(tcx, tcx.local_parent(*id)) + || struct_can_be_constructed_directly(tcx, tcx.local_parent(*id)), + false, + ), + _ => (true, false), + }; + + if !is_seed && is_impl_or_impl_item { + unsolved_impl_item.push(*id); + } + + is_seed + }) .map(|id| (id, ComesFromAllowExpect::No)) }) // Seed entry point @@ -827,7 +917,7 @@ fn create_and_seed_worklist( fn live_symbols_and_ignored_derived_traits( tcx: TyCtxt<'_>, (): (), -) -> (LocalDefIdSet, LocalDefIdMap>) { +) -> (LocalDefIdSet, LocalDefIdMap>, LocalDefIdSet) { let (worklist, mut unsolved_items) = create_and_seed_worklist(tcx); let mut symbol_visitor = MarkSymbolVisitor { worklist, @@ -841,28 +931,29 @@ fn live_symbols_and_ignored_derived_traits( ignore_variant_stack: vec![], ignored_derived_traits: Default::default(), }; - symbol_visitor.mark_live_symbols(); + symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items); - // We have marked the primary seeds as live. We now need to process unsolved items from traits - // and trait impls: add them to the work list if the trait or the implemented type is live. - let mut items_to_check: Vec<_> = unsolved_items - .extract_if(.., |&mut local_def_id| { - symbol_visitor.check_impl_or_impl_item_live(local_def_id) - }) - .collect(); + let reachable_items = + tcx.effective_visibilities(()).iter().filter_map(|(&id, effective_vis)| { + effective_vis.is_public_at_level(Level::Reachable).then_some(id) + }); - while !items_to_check.is_empty() { - symbol_visitor - .worklist - .extend(items_to_check.drain(..).map(|id| (id, ComesFromAllowExpect::No))); - symbol_visitor.mark_live_symbols(); + let mut unstructurable_pub_structs = LocalDefIdSet::default(); + for id in reachable_items { + if symbol_visitor.live_symbols.contains(&id) { + continue; + } + + if matches!(tcx.def_kind(id), DefKind::Struct) { + unstructurable_pub_structs.insert(id); + } - items_to_check.extend(unsolved_items.extract_if(.., |&mut local_def_id| { - symbol_visitor.check_impl_or_impl_item_live(local_def_id) - })); + symbol_visitor.worklist.push((id, ComesFromAllowExpect::No)); } - (symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits) + symbol_visitor.mark_live_symbols_until_unsolved_items_fixed(&mut unsolved_items); + + (symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits, unstructurable_pub_structs) } struct DeadItem { @@ -1142,7 +1233,8 @@ impl<'tcx> DeadVisitor<'tcx> { } fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) { - let (live_symbols, ignored_derived_traits) = tcx.live_symbols_and_ignored_derived_traits(()); + let (live_symbols, ignored_derived_traits, unstructurable_pub_structs) = + tcx.live_symbols_and_ignored_derived_traits(()); let mut visitor = DeadVisitor { tcx, live_symbols, ignored_derived_traits }; let module_items = tcx.hir_module_items(module); @@ -1229,6 +1321,27 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) { for foreign_item in module_items.foreign_items() { visitor.check_definition(foreign_item.owner_id.def_id); } + + for item in module_items.free_items() { + let def_id = item.owner_id.def_id; + + if !unstructurable_pub_structs.contains(&def_id) { + continue; + } + + let Some(name) = tcx.opt_item_name(def_id.to_def_id()) else { + continue; + }; + + if name.as_str().starts_with('_') { + continue; + } + + let hir_id = tcx.local_def_id_to_hir_id(def_id); + let vis_span = tcx.hir_node(hir_id).expect_item().vis_span; + let diag = UnconstructablePubStruct { name, vis_span }; + tcx.emit_node_span_lint(UNCONSTRUCTABLE_PUB_STRUCT, hir_id, tcx.hir_span(hir_id), diag); + } } pub(crate) fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 2da4b6f52cf2c..8fc2ff445e3cf 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1268,6 +1268,14 @@ pub(crate) enum MultipleDeadCodes<'tcx> { }, } +#[derive(LintDiagnostic)] +#[diag(passes_unconstructable_pub_struct)] +pub(crate) struct UnconstructablePubStruct { + pub name: Symbol, + #[help] + pub vis_span: Span, +} + #[derive(Subdiagnostic)] #[note(passes_enum_variant_same_name)] pub(crate) struct EnumVariantSameName<'tcx> { diff --git a/tests/ui/lint/dead-code/unconstructable-pub-struct.rs b/tests/ui/lint/dead-code/unconstructable-pub-struct.rs new file mode 100644 index 0000000000000..dc9f228052f0a --- /dev/null +++ b/tests/ui/lint/dead-code/unconstructable-pub-struct.rs @@ -0,0 +1,83 @@ +#![feature(never_type)] +#![deny(unconstructable_pub_struct)] + +pub struct T1(!); +pub struct T2(()); +pub struct T3(std::marker::PhantomData); + +pub struct T4 { + _x: !, +} + +pub struct T5 { + _x: !, + _y: X, +} + +pub struct T6 { + _x: (), +} + +pub struct T7 { + _x: (), + _y: X, +} + +pub struct T8 { + _x: std::marker::PhantomData, +} + +pub struct T9 { //~ ERROR: pub struct `T9` is unconstructable externally and never constructed locally + _x: std::marker::PhantomData, + _y: i32, +} + +pub struct _T10(i32); + +mod pri { + pub struct Unreachable(i32); +} + +pub struct NeverConstructed(i32); //~ ERROR: pub struct `NeverConstructed` is unconstructable externally and never constructed locally + +impl NeverConstructed { + pub fn not_construct_self(&self) {} +} + +impl Clone for NeverConstructed { + fn clone(&self) -> NeverConstructed { + NeverConstructed(0) + } +} + +pub trait Trait { + fn not_construct_self(&self); +} + +impl Trait for NeverConstructed { + fn not_construct_self(&self) { + self.0; + } +} + +pub struct Constructed(i32); + +impl Constructed { + pub fn construct_self() -> Self { + Constructed(0) + } +} + +impl Clone for Constructed { + fn clone(&self) -> Constructed { + Constructed(0) + } +} + +impl Trait for Constructed { + fn not_construct_self(&self) { + self.0; + } +} + +fn main() {} diff --git a/tests/ui/lint/dead-code/unconstructable-pub-struct.stderr b/tests/ui/lint/dead-code/unconstructable-pub-struct.stderr new file mode 100644 index 0000000000000..9effbf3ea51e6 --- /dev/null +++ b/tests/ui/lint/dead-code/unconstructable-pub-struct.stderr @@ -0,0 +1,31 @@ +error: pub struct `T9` is unconstructable externally and never constructed locally + --> $DIR/unconstructable-pub-struct.rs:30:1 + | +LL | pub struct T9 { + | ^^^^^^^^^^^^^^^^ + | +help: this struct may be unused locally and also externally, consider removing it + --> $DIR/unconstructable-pub-struct.rs:30:1 + | +LL | pub struct T9 { + | ^^^ +note: the lint level is defined here + --> $DIR/unconstructable-pub-struct.rs:2:9 + | +LL | #![deny(unconstructable_pub_struct)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: pub struct `NeverConstructed` is unconstructable externally and never constructed locally + --> $DIR/unconstructable-pub-struct.rs:41:1 + | +LL | pub struct NeverConstructed(i32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: this struct may be unused locally and also externally, consider removing it + --> $DIR/unconstructable-pub-struct.rs:41:1 + | +LL | pub struct NeverConstructed(i32); + | ^^^ + +error: aborting due to 2 previous errors +