From 423a712a16233277e22fec707d9aca8a0a83aa2d Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 13 Feb 2022 19:38:36 +0100 Subject: [PATCH] Fix lints. --- compiler/rustc_lint/src/builtin.rs | 69 ++++++++----------- compiler/rustc_passes/src/dead.rs | 49 ++++--------- compiler/rustc_privacy/src/lib.rs | 2 +- .../rustc_resolve/src/build_reduced_graph.rs | 9 ++- src/test/ui/lint/unreachable_pub-pub_crate.rs | 5 ++ .../ui/lint/unreachable_pub-pub_crate.stderr | 18 ++--- src/test/ui/lint/unreachable_pub.rs | 5 ++ src/test/ui/lint/unreachable_pub.stderr | 18 ++--- 8 files changed, 79 insertions(+), 96 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 7facb8e1b58..68658e2616e 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -486,9 +486,6 @@ fn check_fn(&mut self, cx: &EarlyContext<'_>, fk: FnKind<'_>, span: Span, _: ast pub struct MissingDoc { /// Stack of whether `#[doc(hidden)]` is set at each level which has lint attributes. doc_hidden_stack: Vec, - - /// Private traits or trait items that leaked through. Don't check their methods. - private_traits: FxHashSet, } impl_lint_pass!(MissingDoc => [MISSING_DOCS]); @@ -519,7 +516,7 @@ fn has_doc(attr: &ast::Attribute) -> bool { impl MissingDoc { pub fn new() -> MissingDoc { - MissingDoc { doc_hidden_stack: vec![false], private_traits: FxHashSet::default() } + MissingDoc { doc_hidden_stack: vec![false] } } fn doc_hidden(&self) -> bool { @@ -597,36 +594,16 @@ fn check_crate(&mut self, cx: &LateContext<'_>) { fn check_item(&mut self, cx: &LateContext<'_>, it: &hir::Item<'_>) { match it.kind { - hir::ItemKind::Trait(.., trait_item_refs) => { + hir::ItemKind::Trait(..) => { // Issue #11592: traits are always considered exported, even when private. if cx.tcx.visibility(it.def_id) == ty::Visibility::Restricted( cx.tcx.parent_module_from_def_id(it.def_id).to_def_id(), ) { - self.private_traits.insert(it.hir_id()); - for trait_item_ref in trait_item_refs { - self.private_traits.insert(trait_item_ref.id.hir_id()); - } return; } } - hir::ItemKind::Impl(hir::Impl { of_trait: Some(ref trait_ref), items, .. }) => { - // If the trait is private, add the impl items to `private_traits` so they don't get - // reported for missing docs. - let real_trait = trait_ref.path.res.def_id(); - let Some(def_id) = real_trait.as_local() else { return }; - if cx.tcx.visibility(def_id) - == ty::Visibility::Restricted( - cx.tcx.parent_module_from_def_id(it.def_id).to_def_id(), - ) - { - for impl_item_ref in items { - self.private_traits.insert(impl_item_ref.id.hir_id()); - } - } - return; - } hir::ItemKind::TyAlias(..) | hir::ItemKind::Fn(..) | hir::ItemKind::Macro(..) @@ -646,10 +623,6 @@ fn check_item(&mut self, cx: &LateContext<'_>, it: &hir::Item<'_>) { } fn check_trait_item(&mut self, cx: &LateContext<'_>, trait_item: &hir::TraitItem<'_>) { - if self.private_traits.contains(&trait_item.hir_id()) { - return; - } - let (article, desc) = cx.tcx.article_and_description(trait_item.def_id.to_def_id()); self.check_missing_docs_attrs(cx, trait_item.def_id, trait_item.span, article, desc); @@ -1389,15 +1362,16 @@ fn perform_lint( cx: &LateContext<'_>, what: &str, def_id: LocalDefId, + span: Span, vis_span: Span, exportable: bool, ) { let mut applicability = Applicability::MachineApplicable; - if !cx.access_levels.is_reachable(def_id) { + if cx.tcx.visibility(def_id).is_public() && !cx.access_levels.is_reachable(def_id) { if vis_span.from_expansion() { applicability = Applicability::MaybeIncorrect; } - let def_span = cx.tcx.def_span(def_id); + let def_span = cx.tcx.sess.source_map().guess_head_span(span); cx.struct_span_lint(UNREACHABLE_PUB, def_span, |lint| { let mut err = lint.build(&format!("unreachable `pub` {}", what)); let replacement = if cx.tcx.features().crate_visibility_modifier { @@ -1424,27 +1398,40 @@ fn perform_lint( impl<'tcx> LateLintPass<'tcx> for UnreachablePub { fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) { - if cx.tcx.visibility(item.def_id).is_public() { - self.perform_lint(cx, "item", item.def_id, item.vis_span, true); + // Do not warn for fake `use` statements. + if let hir::ItemKind::Use(_, hir::UseKind::ListStem) = &item.kind { + return; } + self.perform_lint(cx, "item", item.def_id, item.span, item.vis_span, true); } fn check_foreign_item(&mut self, cx: &LateContext<'_>, foreign_item: &hir::ForeignItem<'tcx>) { - if cx.tcx.visibility(foreign_item.def_id).is_public() { - self.perform_lint(cx, "item", foreign_item.def_id, foreign_item.vis_span, true); - } + self.perform_lint( + cx, + "item", + foreign_item.def_id, + foreign_item.span, + foreign_item.vis_span, + true, + ); } fn check_field_def(&mut self, cx: &LateContext<'_>, field: &hir::FieldDef<'_>) { let def_id = cx.tcx.hir().local_def_id(field.hir_id); - if cx.tcx.visibility(def_id).is_public() { - self.perform_lint(cx, "field", def_id, field.vis_span, false); - } + self.perform_lint(cx, "field", def_id, field.span, field.vis_span, false); } fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) { - if cx.tcx.visibility(impl_item.def_id).is_public() { - self.perform_lint(cx, "item", impl_item.def_id, impl_item.vis_span, false); + // Only lint inherent impl items. + if cx.tcx.associated_item(impl_item.def_id).trait_item_def_id.is_none() { + self.perform_lint( + cx, + "item", + impl_item.def_id, + impl_item.span, + impl_item.vis_span, + false, + ); } } } diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 1da802961fe..b661f6f9d72 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -45,8 +45,6 @@ struct MarkSymbolVisitor<'tcx> { live_symbols: FxHashSet, repr_has_repr_c: bool, in_pat: bool, - inherited_pub_visibility: bool, - pub_visibility: bool, ignore_variant_stack: Vec, // maps from tuple struct constructors to tuple struct items struct_constructors: FxHashMap, @@ -284,33 +282,23 @@ fn visit_node(&mut self, node: Node<'tcx>) { } let had_repr_c = self.repr_has_repr_c; - let had_inherited_pub_visibility = self.inherited_pub_visibility; - let had_pub_visibility = self.pub_visibility; self.repr_has_repr_c = false; - self.inherited_pub_visibility = false; - self.pub_visibility = false; match node { - Node::Item(item) => { - self.pub_visibility = self.tcx.visibility(item.def_id).is_public(); + Node::Item(item) => match item.kind { + hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => { + let def = self.tcx.adt_def(item.def_id); + self.repr_has_repr_c = def.repr().c(); - match item.kind { - hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => { - let def = self.tcx.adt_def(item.def_id); - self.repr_has_repr_c = def.repr().c(); - - intravisit::walk_item(self, &item); - } - hir::ItemKind::Enum(..) => { - self.inherited_pub_visibility = self.pub_visibility; - - intravisit::walk_item(self, &item); - } - hir::ItemKind::ForeignMod { .. } => {} - _ => { - intravisit::walk_item(self, &item); - } + intravisit::walk_item(self, &item); } - } + hir::ItemKind::Enum(..) => { + intravisit::walk_item(self, &item); + } + hir::ItemKind::ForeignMod { .. } => {} + _ => { + intravisit::walk_item(self, &item); + } + }, Node::TraitItem(trait_item) => { intravisit::walk_trait_item(self, trait_item); } @@ -322,8 +310,6 @@ fn visit_node(&mut self, node: Node<'tcx>) { } _ => {} } - self.pub_visibility = had_pub_visibility; - self.inherited_pub_visibility = had_inherited_pub_visibility; self.repr_has_repr_c = had_repr_c; } @@ -356,19 +342,14 @@ fn visit_variant_data( ) { let tcx = self.tcx; let has_repr_c = self.repr_has_repr_c; - let inherited_pub_visibility = self.inherited_pub_visibility; - let pub_visibility = self.pub_visibility; let live_fields = def.fields().iter().filter_map(|f| { let def_id = tcx.hir().local_def_id(f.hir_id); if has_repr_c { return Some(def_id); } - if !pub_visibility { + if !tcx.visibility(f.hir_id.owner).is_public() { return None; } - if inherited_pub_visibility { - return Some(def_id); - } if tcx.visibility(def_id).is_public() { Some(def_id) } else { None } }); self.live_symbols.extend(live_fields); @@ -612,8 +593,6 @@ fn live_symbols_and_ignored_derived_traits<'tcx>( live_symbols: Default::default(), repr_has_repr_c: false, in_pat: false, - inherited_pub_visibility: false, - pub_visibility: false, ignore_variant_stack: vec![], struct_constructors, ignored_derived_traits: FxHashMap::default(), diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 2501d182a20..27b42a8ac15 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -1982,7 +1982,7 @@ fn visibility(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Visibility { // Visibility on them should have no effect, but to avoid the visibility // query failing on some items, we provide it for opaque types as well. | Node::Item(hir::Item { - kind: hir::ItemKind::Use(..) | hir::ItemKind::OpaqueTy(..), + kind: hir::ItemKind::Use(_, hir::UseKind::ListStem) | hir::ItemKind::OpaqueTy(..), .. }) => ty::Visibility::Restricted(tcx.parent_module(hir_id).to_def_id()), // Visibilities of trait impl items are inherited from their traits diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 8dade5f87ee..3cb38d94009 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -458,6 +458,14 @@ fn build_reduced_graph_for_use_tree( let mut source = module_path.pop().unwrap(); let mut type_ns_only = false; + self.r.visibilities.insert(self.r.local_def_id(id), vis); + if id1 != ast::DUMMY_NODE_ID { + self.r.visibilities.insert(self.r.local_def_id(id1), vis); + } + if id2 != ast::DUMMY_NODE_ID { + self.r.visibilities.insert(self.r.local_def_id(id2), vis); + } + if nested { // Correctly handle `self` if source.ident.name == kw::SelfLower { @@ -564,7 +572,6 @@ fn build_reduced_graph_for_use_tree( additional_ids: (id1, id2), }; - self.r.visibilities.insert(self.r.local_def_id(id), vis); self.add_import( module_path, kind, diff --git a/src/test/ui/lint/unreachable_pub-pub_crate.rs b/src/test/ui/lint/unreachable_pub-pub_crate.rs index 27c31c22311..4dc951985ae 100644 --- a/src/test/ui/lint/unreachable_pub-pub_crate.rs +++ b/src/test/ui/lint/unreachable_pub-pub_crate.rs @@ -26,6 +26,11 @@ impl Hydrogen { pub fn count_neutrons(&self) -> usize { self.neutrons } //~ WARNING unreachable_pub pub(crate) fn count_electrons(&self) -> usize { self.electrons } } + impl Clone for Hydrogen { + fn clone(&self) -> Hydrogen { + Hydrogen { neutrons: self.neutrons, electrons: self.electrons } + } + } pub enum Helium {} //~ WARNING unreachable_pub pub union Lithium { c1: usize, c2: u8 } //~ WARNING unreachable_pub diff --git a/src/test/ui/lint/unreachable_pub-pub_crate.stderr b/src/test/ui/lint/unreachable_pub-pub_crate.stderr index f284db80ff9..6c05a030138 100644 --- a/src/test/ui/lint/unreachable_pub-pub_crate.stderr +++ b/src/test/ui/lint/unreachable_pub-pub_crate.stderr @@ -50,7 +50,7 @@ LL | pub fn count_neutrons(&self) -> usize { self.neutrons } | help: consider restricting its visibility: `pub(crate)` warning: unreachable `pub` item - --> $DIR/unreachable_pub-pub_crate.rs:30:5 + --> $DIR/unreachable_pub-pub_crate.rs:35:5 | LL | pub enum Helium {} | ---^^^^^^^^^^^^ @@ -60,7 +60,7 @@ LL | pub enum Helium {} = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub-pub_crate.rs:31:5 + --> $DIR/unreachable_pub-pub_crate.rs:36:5 | LL | pub union Lithium { c1: usize, c2: u8 } | ---^^^^^^^^^^^^^^ @@ -70,7 +70,7 @@ LL | pub union Lithium { c1: usize, c2: u8 } = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub-pub_crate.rs:32:5 + --> $DIR/unreachable_pub-pub_crate.rs:37:5 | LL | pub fn beryllium() {} | ---^^^^^^^^^^^^^^^ @@ -80,7 +80,7 @@ LL | pub fn beryllium() {} = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub-pub_crate.rs:33:5 + --> $DIR/unreachable_pub-pub_crate.rs:38:5 | LL | pub trait Boron {} | ---^^^^^^^^^^^^ @@ -90,7 +90,7 @@ LL | pub trait Boron {} = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub-pub_crate.rs:34:5 + --> $DIR/unreachable_pub-pub_crate.rs:39:5 | LL | pub const CARBON: usize = 1; | ---^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -100,7 +100,7 @@ LL | pub const CARBON: usize = 1; = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub-pub_crate.rs:35:5 + --> $DIR/unreachable_pub-pub_crate.rs:40:5 | LL | pub static NITROGEN: usize = 2; | ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -110,7 +110,7 @@ LL | pub static NITROGEN: usize = 2; = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub-pub_crate.rs:36:5 + --> $DIR/unreachable_pub-pub_crate.rs:41:5 | LL | pub type Oxygen = bool; | ---^^^^^^^^^^^^^^^^^^^^ @@ -120,7 +120,7 @@ LL | pub type Oxygen = bool; = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub-pub_crate.rs:39:47 + --> $DIR/unreachable_pub-pub_crate.rs:44:47 | LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} } | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -135,7 +135,7 @@ LL | define_empty_struct_with_visibility!(pub, Fluorine); = note: this warning originates in the macro `define_empty_struct_with_visibility` (in Nightly builds, run with -Z macro-backtrace for more info) warning: unreachable `pub` item - --> $DIR/unreachable_pub-pub_crate.rs:45:9 + --> $DIR/unreachable_pub-pub_crate.rs:50:9 | LL | pub fn catalyze() -> bool; | ---^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/lint/unreachable_pub.rs b/src/test/ui/lint/unreachable_pub.rs index 6bfec0ec5e8..39e2b596156 100644 --- a/src/test/ui/lint/unreachable_pub.rs +++ b/src/test/ui/lint/unreachable_pub.rs @@ -22,6 +22,11 @@ impl Hydrogen { pub fn count_neutrons(&self) -> usize { self.neutrons } //~ WARNING unreachable_pub crate fn count_electrons(&self) -> usize { self.electrons } } + impl Clone for Hydrogen { + fn clone(&self) -> Hydrogen { + Hydrogen { neutrons: self.neutrons, electrons: self.electrons } + } + } pub enum Helium {} //~ WARNING unreachable_pub pub union Lithium { c1: usize, c2: u8 } //~ WARNING unreachable_pub diff --git a/src/test/ui/lint/unreachable_pub.stderr b/src/test/ui/lint/unreachable_pub.stderr index 61c9582287c..e8e55be5a47 100644 --- a/src/test/ui/lint/unreachable_pub.stderr +++ b/src/test/ui/lint/unreachable_pub.stderr @@ -50,7 +50,7 @@ LL | pub fn count_neutrons(&self) -> usize { self.neutrons } | help: consider restricting its visibility: `crate` warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:26:5 + --> $DIR/unreachable_pub.rs:31:5 | LL | pub enum Helium {} | ---^^^^^^^^^^^^ @@ -60,7 +60,7 @@ LL | pub enum Helium {} = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:27:5 + --> $DIR/unreachable_pub.rs:32:5 | LL | pub union Lithium { c1: usize, c2: u8 } | ---^^^^^^^^^^^^^^ @@ -70,7 +70,7 @@ LL | pub union Lithium { c1: usize, c2: u8 } = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:28:5 + --> $DIR/unreachable_pub.rs:33:5 | LL | pub fn beryllium() {} | ---^^^^^^^^^^^^^^^ @@ -80,7 +80,7 @@ LL | pub fn beryllium() {} = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:29:5 + --> $DIR/unreachable_pub.rs:34:5 | LL | pub trait Boron {} | ---^^^^^^^^^^^^ @@ -90,7 +90,7 @@ LL | pub trait Boron {} = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:30:5 + --> $DIR/unreachable_pub.rs:35:5 | LL | pub const CARBON: usize = 1; | ---^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -100,7 +100,7 @@ LL | pub const CARBON: usize = 1; = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:31:5 + --> $DIR/unreachable_pub.rs:36:5 | LL | pub static NITROGEN: usize = 2; | ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -110,7 +110,7 @@ LL | pub static NITROGEN: usize = 2; = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:32:5 + --> $DIR/unreachable_pub.rs:37:5 | LL | pub type Oxygen = bool; | ---^^^^^^^^^^^^^^^^^^^^ @@ -120,7 +120,7 @@ LL | pub type Oxygen = bool; = help: or consider exporting it for use by other crates warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:35:47 + --> $DIR/unreachable_pub.rs:40:47 | LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} } | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -135,7 +135,7 @@ LL | define_empty_struct_with_visibility!(pub, Fluorine); = note: this warning originates in the macro `define_empty_struct_with_visibility` (in Nightly builds, run with -Z macro-backtrace for more info) warning: unreachable `pub` item - --> $DIR/unreachable_pub.rs:41:9 + --> $DIR/unreachable_pub.rs:46:9 | LL | pub fn catalyze() -> bool; | ---^^^^^^^^^^^^^^^^^^^^^^^