Skip to content

Commit df035a3

Browse files
borsbors
committed
Auto merge of #87487 - lambinoo:I-64762_unreachable_pub_lint, r=petrochenkov
Fixes wrong unreachable_pub lints on nested and glob public reexport Linked issues: #64762 & #82064
2 parents d63a8d9 + 3a77bb8 commit df035a3

File tree

19 files changed

+359
-218
lines changed

19 files changed

+359
-218
lines changed

‎compiler/rustc_errors/src/snippet.rs

-3
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,6 @@ pub enum AnnotationType {
6969
/// Annotation under a single line of code
7070
Singleline,
7171

72-
/// Annotation enclosing the first and last character of a multiline span
73-
Multiline(MultilineAnnotation),
74-
7572
// The Multiline type above is replaced with the following three in order
7673
// to reuse the current label drawing code.
7774
//

‎compiler/rustc_lint/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ use unused::*;
9292

9393
/// Useful for other parts of the compiler / Clippy.
9494
pub use builtin::SoftLints;
95-
pub use context::{CheckLintNameResult, EarlyContext, LateContext, LintContext, LintStore};
95+
pub use context::{CheckLintNameResult, FindLintError, LintStore};
96+
pub use context::{EarlyContext, LateContext, LintContext};
9697
pub use early::check_ast_crate;
9798
pub use late::check_crate;
9899
pub use passes::{EarlyLintPass, LateLintPass};

‎compiler/rustc_middle/src/middle/privacy.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ pub enum AccessLevel {
2828
}
2929

3030
/// Holds a map of accessibility levels for reachable HIR nodes.
31-
#[derive(Debug)]
31+
#[derive(Debug, Clone)]
3232
pub struct AccessLevels<Id = LocalDefId> {
3333
pub map: FxHashMap<Id, AccessLevel>,
3434
}

‎compiler/rustc_middle/src/ty/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ pub use generics::*;
2020
pub use vtable::*;
2121

2222
use crate::metadata::ModChild;
23+
use crate::middle::privacy::AccessLevels;
2324
use crate::mir::{Body, GeneratorLayout};
2425
use crate::traits::{self, Reveal};
2526
use crate::ty;
@@ -123,6 +124,7 @@ pub struct ResolverOutputs {
123124
pub definitions: rustc_hir::definitions::Definitions,
124125
pub cstore: Box<CrateStoreDyn>,
125126
pub visibilities: FxHashMap<LocalDefId, Visibility>,
127+
pub access_levels: AccessLevels,
126128
pub extern_crate_map: FxHashMap<LocalDefId, CrateNum>,
127129
pub maybe_unused_trait_imports: FxHashSet<LocalDefId>,
128130
pub maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,

‎compiler/rustc_privacy/src/lib.rs

+39-181
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ use rustc_data_structures::fx::FxHashSet;
1111
use rustc_errors::struct_span_err;
1212
use rustc_hir as hir;
1313
use rustc_hir::def::{DefKind, Res};
14-
use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet};
15-
use rustc_hir::def_id::{CRATE_DEF_ID, CRATE_DEF_INDEX, LOCAL_CRATE};
14+
use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet, CRATE_DEF_ID};
1615
use rustc_hir::intravisit::{self, DeepVisitor, NestedVisitorMap, Visitor};
1716
use rustc_hir::{AssocItemKind, HirIdSet, Node, PatKind};
1817
use rustc_middle::bug;
@@ -26,7 +25,7 @@ use rustc_middle::ty::subst::InternalSubsts;
2625
use rustc_middle::ty::{self, Const, GenericParamDefKind, TraitRef, Ty, TyCtxt, TypeFoldable};
2726
use rustc_session::lint;
2827
use rustc_span::hygiene::Transparency;
29-
use rustc_span::symbol::{kw, sym, Ident};
28+
use rustc_span::symbol::{kw, Ident};
3029
use rustc_span::Span;
3130
use rustc_trait_selection::traits::const_evaluatable::{self, AbstractConst};
3231

@@ -436,6 +435,15 @@ impl<'tcx> EmbargoVisitor<'tcx> {
436435
self.access_levels.map.get(&def_id).copied()
437436
}
438437

438+
fn update_with_hir_id(
439+
&mut self,
440+
hir_id: hir::HirId,
441+
level: Option<AccessLevel>,
442+
) -> Option<AccessLevel> {
443+
let def_id = self.tcx.hir().local_def_id(hir_id);
444+
self.update(def_id, level)
445+
}
446+
439447
/// Updates node level and returns the updated level.
440448
fn update(&mut self, def_id: LocalDefId, level: Option<AccessLevel>) -> Option<AccessLevel> {
441449
let old_level = self.get(def_id);
@@ -623,54 +631,6 @@ impl<'tcx> EmbargoVisitor<'tcx> {
623631
| DefKind::Generator => (),
624632
}
625633
}
626-
627-
/// Given the path segments of an `ItemKind::Use`, then we need
628-
/// to update the visibility of the intermediate use so that it isn't linted
629-
/// by `unreachable_pub`.
630-
///
631-
/// This isn't trivial as `path.res` has the `DefId` of the eventual target
632-
/// of the use statement not of the next intermediate use statement.
633-
///
634-
/// To do this, consider the last two segments of the path to our intermediate
635-
/// use statement. We expect the penultimate segment to be a module and the
636-
/// last segment to be the name of the item we are exporting. We can then
637-
/// look at the items contained in the module for the use statement with that
638-
/// name and update that item's visibility.
639-
///
640-
/// FIXME: This solution won't work with glob imports and doesn't respect
641-
/// namespaces. See <https://github.com/rust-lang/rust/pull/57922#discussion_r251234202>.
642-
fn update_visibility_of_intermediate_use_statements(
643-
&mut self,
644-
segments: &[hir::PathSegment<'_>],
645-
) {
646-
if let [.., module, segment] = segments {
647-
if let Some(item) = module
648-
.res
649-
.and_then(|res| res.mod_def_id())
650-
// If the module is `self`, i.e. the current crate,
651-
// there will be no corresponding item.
652-
.filter(|def_id| def_id.index != CRATE_DEF_INDEX || def_id.krate != LOCAL_CRATE)
653-
.and_then(|def_id| def_id.as_local())
654-
.map(|module_hir_id| self.tcx.hir().expect_item(module_hir_id))
655-
{
656-
if let hir::ItemKind::Mod(m) = &item.kind {
657-
for &item_id in m.item_ids {
658-
let item = self.tcx.hir().item(item_id);
659-
if !self.tcx.hygienic_eq(
660-
segment.ident,
661-
item.ident,
662-
item_id.def_id.to_def_id(),
663-
) {
664-
continue;
665-
}
666-
if let hir::ItemKind::Use(..) = item.kind {
667-
self.update(item.def_id, Some(AccessLevel::Exported));
668-
}
669-
}
670-
}
671-
}
672-
}
673-
}
674634
}
675635

676636
impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
@@ -683,120 +643,22 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
683643
}
684644

685645
fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
686-
let inherited_item_level = match item.kind {
646+
let item_level = match item.kind {
687647
hir::ItemKind::Impl { .. } => {
688-
Option::<AccessLevel>::of_impl(item.def_id, self.tcx, &self.access_levels)
689-
}
690-
// Only exported `macro_rules!` items are public, but they always are.
691-
hir::ItemKind::Macro(MacroDef { macro_rules: true, .. }) => {
692-
let def_id = item.def_id.to_def_id();
693-
let is_macro_export = self.tcx.has_attr(def_id, sym::macro_export);
694-
if is_macro_export { Some(AccessLevel::Public) } else { None }
695-
}
696-
// Foreign modules inherit level from parents.
697-
hir::ItemKind::ForeignMod { .. } => self.prev_level,
698-
// Other `pub` items inherit levels from parents.
699-
hir::ItemKind::Const(..)
700-
| hir::ItemKind::Enum(..)
701-
| hir::ItemKind::ExternCrate(..)
702-
| hir::ItemKind::GlobalAsm(..)
703-
| hir::ItemKind::Fn(..)
704-
| hir::ItemKind::Macro(..)
705-
| hir::ItemKind::Mod(..)
706-
| hir::ItemKind::Static(..)
707-
| hir::ItemKind::Struct(..)
708-
| hir::ItemKind::Trait(..)
709-
| hir::ItemKind::TraitAlias(..)
710-
| hir::ItemKind::OpaqueTy(..)
711-
| hir::ItemKind::TyAlias(..)
712-
| hir::ItemKind::Union(..)
713-
| hir::ItemKind::Use(..) => {
714-
if item.vis.node.is_pub() {
715-
self.prev_level
716-
} else {
717-
None
718-
}
648+
let impl_level =
649+
Option::<AccessLevel>::of_impl(item.def_id, self.tcx, &self.access_levels);
650+
self.update(item.def_id, impl_level)
719651
}
652+
_ => self.get(item.def_id),
720653
};
721654

722-
// Update level of the item itself.
723-
let item_level = self.update(item.def_id, inherited_item_level);
724-
725-
// Update levels of nested things.
726-
match item.kind {
727-
hir::ItemKind::Enum(ref def, _) => {
728-
for variant in def.variants {
729-
let variant_level =
730-
self.update(self.tcx.hir().local_def_id(variant.id), item_level);
731-
if let Some(ctor_hir_id) = variant.data.ctor_hir_id() {
732-
self.update(self.tcx.hir().local_def_id(ctor_hir_id), item_level);
733-
}
734-
for field in variant.data.fields() {
735-
self.update(self.tcx.hir().local_def_id(field.hir_id), variant_level);
736-
}
737-
}
738-
}
739-
hir::ItemKind::Impl(ref impl_) => {
740-
for impl_item_ref in impl_.items {
741-
if impl_.of_trait.is_some()
742-
|| self.tcx.visibility(impl_item_ref.id.def_id) == ty::Visibility::Public
743-
{
744-
self.update(impl_item_ref.id.def_id, item_level);
745-
}
746-
}
747-
}
748-
hir::ItemKind::Trait(.., trait_item_refs) => {
749-
for trait_item_ref in trait_item_refs {
750-
self.update(trait_item_ref.id.def_id, item_level);
751-
}
752-
}
753-
hir::ItemKind::Struct(ref def, _) | hir::ItemKind::Union(ref def, _) => {
754-
if let Some(ctor_hir_id) = def.ctor_hir_id() {
755-
self.update(self.tcx.hir().local_def_id(ctor_hir_id), item_level);
756-
}
757-
for field in def.fields() {
758-
if field.vis.node.is_pub() {
759-
self.update(self.tcx.hir().local_def_id(field.hir_id), item_level);
760-
}
761-
}
762-
}
763-
hir::ItemKind::Macro(ref macro_def) => {
764-
self.update_reachability_from_macro(item.def_id, macro_def);
765-
}
766-
hir::ItemKind::ForeignMod { items, .. } => {
767-
for foreign_item in items {
768-
if self.tcx.visibility(foreign_item.id.def_id) == ty::Visibility::Public {
769-
self.update(foreign_item.id.def_id, item_level);
770-
}
771-
}
772-
}
773-
774-
hir::ItemKind::OpaqueTy(..)
775-
| hir::ItemKind::Use(..)
776-
| hir::ItemKind::Static(..)
777-
| hir::ItemKind::Const(..)
778-
| hir::ItemKind::GlobalAsm(..)
779-
| hir::ItemKind::TyAlias(..)
780-
| hir::ItemKind::Mod(..)
781-
| hir::ItemKind::TraitAlias(..)
782-
| hir::ItemKind::Fn(..)
783-
| hir::ItemKind::ExternCrate(..) => {}
784-
}
785-
786655
// Mark all items in interfaces of reachable items as reachable.
787656
match item.kind {
788657
// The interface is empty.
789-
hir::ItemKind::Macro(..) | hir::ItemKind::ExternCrate(..) => {}
658+
hir::ItemKind::ExternCrate(..) => {}
790659
// All nested items are checked by `visit_item`.
791660
hir::ItemKind::Mod(..) => {}
792-
// Re-exports are handled in `visit_mod`. However, in order to avoid looping over
793-
// all of the items of a mod in `visit_mod` looking for use statements, we handle
794-
// making sure that intermediate use statements have their visibilities updated here.
795-
hir::ItemKind::Use(path, _) => {
796-
if item_level.is_some() {
797-
self.update_visibility_of_intermediate_use_statements(path.segments.as_ref());
798-
}
799-
}
661+
hir::ItemKind::Use(..) => {}
800662
// The interface is empty.
801663
hir::ItemKind::GlobalAsm(..) => {}
802664
hir::ItemKind::OpaqueTy(..) => {
@@ -847,6 +709,14 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
847709
}
848710
// Visit everything except for private impl items.
849711
hir::ItemKind::Impl(ref impl_) => {
712+
for impl_item_ref in impl_.items {
713+
if impl_.of_trait.is_some()
714+
|| self.tcx.visibility(impl_item_ref.id.def_id) == ty::Visibility::Public
715+
{
716+
self.update(impl_item_ref.id.def_id, item_level);
717+
}
718+
}
719+
850720
if item_level.is_some() {
851721
self.reach(item.def_id, item_level).generics().predicates().ty().trait_ref();
852722

@@ -861,15 +731,21 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
861731
}
862732
}
863733
}
864-
865734
// Visit everything, but enum variants have their own levels.
866735
hir::ItemKind::Enum(ref def, _) => {
867736
if item_level.is_some() {
868737
self.reach(item.def_id, item_level).generics().predicates();
869738
}
739+
740+
let enum_level = self.get(item.def_id);
870741
for variant in def.variants {
871-
let variant_level = self.get(self.tcx.hir().local_def_id(variant.id));
742+
let variant_level = self.update_with_hir_id(variant.id, enum_level);
743+
872744
if variant_level.is_some() {
745+
if let Some(ctor_id) = variant.data.ctor_hir_id() {
746+
self.update_with_hir_id(ctor_id, variant_level);
747+
}
748+
873749
for field in variant.data.fields() {
874750
self.reach(self.tcx.hir().local_def_id(field.hir_id), variant_level)
875751
.ty();
@@ -880,6 +756,9 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
880756
}
881757
}
882758
}
759+
hir::ItemKind::Macro(ref macro_def) => {
760+
self.update_reachability_from_macro(item.def_id, macro_def);
761+
}
883762
// Visit everything, but foreign items have their own levels.
884763
hir::ItemKind::ForeignMod { items, .. } => {
885764
for foreign_item in items {
@@ -920,27 +799,6 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
920799
intravisit::walk_block(self, b);
921800
self.prev_level = orig_level;
922801
}
923-
924-
fn visit_mod(&mut self, m: &'tcx hir::Mod<'tcx>, _sp: Span, id: hir::HirId) {
925-
// This code is here instead of in visit_item so that the
926-
// crate module gets processed as well.
927-
if self.prev_level.is_some() {
928-
let def_id = self.tcx.hir().local_def_id(id);
929-
if let Some(exports) = self.tcx.module_reexports(def_id) {
930-
for export in exports.iter() {
931-
if export.vis.is_public() {
932-
if let Some(def_id) = export.res.opt_def_id() {
933-
if let Some(def_id) = def_id.as_local() {
934-
self.update(def_id, Some(AccessLevel::Exported));
935-
}
936-
}
937-
}
938-
}
939-
}
940-
}
941-
942-
intravisit::walk_mod(self, m, id);
943-
}
944802
}
945803

946804
impl ReachEverythingInTheInterfaceVisitor<'_, '_> {
@@ -2166,11 +2024,12 @@ fn privacy_access_levels(tcx: TyCtxt<'_>, (): ()) -> &AccessLevels {
21662024
// items which are reachable from external crates based on visibility.
21672025
let mut visitor = EmbargoVisitor {
21682026
tcx,
2169-
access_levels: Default::default(),
2027+
access_levels: tcx.resolutions(()).access_levels.clone(),
21702028
macro_reachable: Default::default(),
21712029
prev_level: Some(AccessLevel::Public),
21722030
changed: false,
21732031
};
2032+
21742033
loop {
21752034
tcx.hir().walk_toplevel_module(&mut visitor);
21762035
if visitor.changed {
@@ -2179,7 +2038,6 @@ fn privacy_access_levels(tcx: TyCtxt<'_>, (): ()) -> &AccessLevels {
21792038
break;
21802039
}
21812040
}
2182-
visitor.update(CRATE_DEF_ID, Some(AccessLevel::Public));
21832041

21842042
tcx.arena.alloc(visitor.access_levels)
21852043
}

0 commit comments

Comments
 (0)