Rollup merge of #101098 - petrochenkov:noinvis, r=TaKO8Ki

rustc_middle: Remove `Visibility::Invisible`

It had a different meaning in the past, but now it's only used as an implementation detail of import resolution.

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

@@ -1500,24 +1500,18 @@ fn vcall_visibility_metadata<'ll, 'tcx>(
         // If there is not LTO and the visibility in public, we have to assume that the vtable can
         // be seen from anywhere. With multiple CGUs, the vtable is quasi-public.
         (Lto::No | Lto::ThinLocal, Visibility::Public, _)
-        | (Lto::No, Visibility::Restricted(_) | Visibility::Invisible, false) => {
-            VCallVisibility::Public
-        }
+        | (Lto::No, Visibility::Restricted(_), false) => VCallVisibility::Public,
         // With LTO and a quasi-public visibility, the usages of the functions of the vtable are
         // all known by the `LinkageUnit`.
         // FIXME: LLVM only supports this optimization for `Lto::Fat` currently. Once it also
         // supports `Lto::Thin` the `VCallVisibility` may have to be adjusted for those.
         (Lto::Fat | Lto::Thin, Visibility::Public, _)
-        | (
-            Lto::ThinLocal | Lto::Thin | Lto::Fat,
-            Visibility::Restricted(_) | Visibility::Invisible,
-            false,
-        ) => VCallVisibility::LinkageUnit,
+        | (Lto::ThinLocal | Lto::Thin | Lto::Fat, Visibility::Restricted(_), false) => {
+            VCallVisibility::LinkageUnit
+        }
         // If there is only one CGU, private vtables can only be seen by that CGU/translation unit
         // and therefore we know of all usages of functions in the vtable.
-        (_, Visibility::Restricted(_) | Visibility::Invisible, true) => {
-            VCallVisibility::TranslationUnit
-        }
+        (_, Visibility::Restricted(_), true) => VCallVisibility::TranslationUnit,
     };
 
     let trait_ref_typeid = typeid_for_trait_ref(cx.tcx, trait_ref);

compiler/rustc_middle/src/middle/stability.rs

@@ -293,7 +293,7 @@ fn skip_stability_check_due_to_privacy(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
 
         // These are not visible outside crate; therefore
         // stability markers are irrelevant, if even present.
-        ty::Visibility::Restricted(..) | ty::Visibility::Invisible => true,
+        ty::Visibility::Restricted(..) => true,
     }
 }
 

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

@@ -169,14 +169,10 @@ impl<'tcx> FieldDef {
         param_env: ty::ParamEnv<'tcx>,
     ) -> DefIdForest<'tcx> {
         let data_uninhabitedness = move || self.ty(tcx, substs).uninhabited_from(tcx, param_env);
-        // FIXME(canndrew): Currently enum fields are (incorrectly) stored with
-        // `Visibility::Invisible` so we need to override `self.vis` if we're
-        // dealing with an enum.
         if is_enum {
             data_uninhabitedness()
         } else {
             match self.vis {
-                Visibility::Invisible => DefIdForest::empty(),
                 Visibility::Restricted(from) => {
                     let forest = DefIdForest::from_id(from);
                     let iter = Some(forest).into_iter().chain(Some(data_uninhabitedness()));

compiler/rustc_middle/src/ty/mod.rs

@@ -268,8 +268,6 @@ pub enum Visibility {
     Public,
     /// Visible only in the given crate-local module.
     Restricted(DefId),
-    /// Not visible anywhere in the local crate. This is the visibility of private external items.
-    Invisible,
 }
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable, TyEncodable, TyDecodable)]
@@ -366,8 +364,6 @@ impl Visibility {
         let restriction = match self {
             // Public items are visible everywhere.
             Visibility::Public => return true,
-            // Private items from other crates are visible nowhere.
-            Visibility::Invisible => return false,
             // Restricted items are visible in an arbitrary local module.
             Visibility::Restricted(other) if other.krate != module.krate => return false,
             Visibility::Restricted(module) => module,
@@ -380,7 +376,6 @@ impl Visibility {
     pub fn is_at_least<T: DefIdTree>(self, vis: Visibility, tree: T) -> bool {
         let vis_restriction = match vis {
             Visibility::Public => return self == Visibility::Public,
-            Visibility::Invisible => return true,
             Visibility::Restricted(module) => module,
         };
 
@@ -392,7 +387,6 @@ impl Visibility {
         match self {
             Visibility::Public => true,
             Visibility::Restricted(def_id) => def_id.is_local(),
-            Visibility::Invisible => false,
         }
     }
 

compiler/rustc_privacy/src/lib.rs

@@ -1731,7 +1731,6 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> {
         if !vis.is_at_least(self.required_visibility, self.tcx) {
             let vis_descr = match vis {
                 ty::Visibility::Public => "public",
-                ty::Visibility::Invisible => "private",
                 ty::Visibility::Restricted(vis_def_id) => {
                     if vis_def_id == self.tcx.parent_module(hir_id).to_def_id() {
                         "private"

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -380,7 +380,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
             has_attributes: !item.attrs.is_empty(),
             root_span,
             root_id,
-            vis: Cell::new(vis),
+            vis: Cell::new(Some(vis)),
             used: Cell::new(false),
         });
 
@@ -588,7 +588,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
             ast::UseTreeKind::Glob => {
                 let kind = ImportKind::Glob {
                     is_prelude: self.r.session.contains_name(&item.attrs, sym::prelude_import),
-                    max_vis: Cell::new(ty::Visibility::Invisible),
+                    max_vis: Cell::new(None),
                 };
                 self.r.visibilities.insert(self.r.local_def_id(id), vis);
                 self.add_import(prefix, kind, use_tree.span, id, item, root_span, item.id, vis);
@@ -650,7 +650,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                         true,
                         // The whole `use` item
                         item,
-                        ty::Visibility::Invisible,
+                        ty::Visibility::Restricted(self.parent_scope.module.nearest_parent_mod()),
                         root_span,
                     );
                 }
@@ -885,7 +885,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
             root_span: item.span,
             span: item.span,
             module_path: Vec::new(),
-            vis: Cell::new(vis),
+            vis: Cell::new(Some(vis)),
             used: Cell::new(used),
         });
         self.r.potentially_unused_imports.push(import);
@@ -1118,7 +1118,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
                 root_span: span,
                 span,
                 module_path: Vec::new(),
-                vis: Cell::new(ty::Visibility::Restricted(CRATE_DEF_ID.to_def_id())),
+                vis: Cell::new(Some(ty::Visibility::Restricted(CRATE_DEF_ID.to_def_id()))),
                 used: Cell::new(false),
             })
         };

compiler/rustc_resolve/src/check_unused.rs

@@ -227,7 +227,7 @@ impl Resolver<'_> {
         for import in self.potentially_unused_imports.iter() {
             match import.kind {
                 _ if import.used.get()
-                    || import.vis.get().is_public()
+                    || import.expect_vis().is_public()
                     || import.span.is_dummy() =>
                 {
                     if let ImportKind::MacroUse = import.kind {

compiler/rustc_resolve/src/ident.rs

@@ -953,7 +953,10 @@ impl<'a> Resolver<'a> {
         // Check if one of single imports can still define the name,
         // if it can then our result is not determined and can be invalidated.
         for single_import in &resolution.single_imports {
-            if !self.is_accessible_from(single_import.vis.get(), parent_scope.module) {
+            let Some(import_vis) = single_import.vis.get() else {
+                continue;
+            };
+            if !self.is_accessible_from(import_vis, parent_scope.module) {
                 continue;
             }
             let Some(module) = single_import.imported_module.get() else {
@@ -1018,7 +1021,10 @@ impl<'a> Resolver<'a> {
         // Check if one of glob imports can still define the name,
         // if it can then our "no resolution" result is not determined and can be invalidated.
         for glob_import in module.globs.borrow().iter() {
-            if !self.is_accessible_from(glob_import.vis.get(), parent_scope.module) {
+            let Some(import_vis) = glob_import.vis.get() else {
+                continue;
+            };
+            if !self.is_accessible_from(import_vis, parent_scope.module) {
                 continue;
             }
             let module = match glob_import.imported_module.get() {

compiler/rustc_resolve/src/imports.rs

@@ -52,8 +52,8 @@ pub enum ImportKind<'a> {
     },
     Glob {
         is_prelude: bool,
-        max_vis: Cell<ty::Visibility>, // The visibility of the greatest re-export.
-                                       // n.b. `max_vis` is only used in `finalize_import` to check for re-export errors.
+        max_vis: Cell<Option<ty::Visibility>>, // The visibility of the greatest re-export.
+                                               // n.b. `max_vis` is only used in `finalize_import` to check for re-export errors.
     },
     ExternCrate {
         source: Option<Symbol>,
@@ -144,7 +144,7 @@ pub(crate) struct Import<'a> {
     pub module_path: Vec<Segment>,
     /// The resolution of `module_path`.
     pub imported_module: Cell<Option<ModuleOrUniformRoot<'a>>>,
-    pub vis: Cell<ty::Visibility>,
+    pub vis: Cell<Option<ty::Visibility>>,
     pub used: Cell<bool>,
 }
 
@@ -159,6 +159,10 @@ impl<'a> Import<'a> {
             _ => false,
         }
     }
+
+    pub(crate) fn expect_vis(&self) -> ty::Visibility {
+        self.vis.get().expect("encountered cleared import visibility")
+    }
 }
 
 /// Records information about the resolution of a name in a namespace of a module.
@@ -199,7 +203,7 @@ fn pub_use_of_private_extern_crate_hack(import: &Import<'_>, binding: &NameBindi
                 import: Import { kind: ImportKind::ExternCrate { .. }, .. },
                 ..
             },
-        ) => import.vis.get().is_public(),
+        ) => import.expect_vis().is_public(),
         _ => false,
     }
 }
@@ -212,17 +216,20 @@ impl<'a> Resolver<'a> {
         binding: &'a NameBinding<'a>,
         import: &'a Import<'a>,
     ) -> &'a NameBinding<'a> {
-        let vis = if binding.vis.is_at_least(import.vis.get(), self)
+        let import_vis = import.expect_vis();
+        let vis = if binding.vis.is_at_least(import_vis, self)
             || pub_use_of_private_extern_crate_hack(import, binding)
         {
-            import.vis.get()
+            import_vis
         } else {
             binding.vis
         };
 
         if let ImportKind::Glob { ref max_vis, .. } = import.kind {
-            if vis == import.vis.get() || vis.is_at_least(max_vis.get(), self) {
-                max_vis.set(vis)
+            if vis == import_vis
+                || max_vis.get().map_or(true, |max_vis| vis.is_at_least(max_vis, self))
+            {
+                max_vis.set(Some(vis))
             }
         }
 
@@ -536,7 +543,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         } else {
             // For better failure detection, pretend that the import will
             // not define any names while resolving its module path.
-            let orig_vis = import.vis.replace(ty::Visibility::Invisible);
+            let orig_vis = import.vis.take();
             let path_res =
                 self.r.maybe_resolve_path(&import.module_path, None, &import.parent_scope);
             import.vis.set(orig_vis);
@@ -571,7 +578,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                 if let Err(Undetermined) = source_bindings[ns].get() {
                     // For better failure detection, pretend that the import will
                     // not define any names while resolving its module path.
-                    let orig_vis = import.vis.replace(ty::Visibility::Invisible);
+                    let orig_vis = import.vis.take();
                     let binding = this.resolve_ident_in_module(
                         module,
                         source,
@@ -620,7 +627,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
     /// Optionally returns an unresolved import error. This error is buffered and used to
     /// consolidate multiple unresolved import errors into a single diagnostic.
     fn finalize_import(&mut self, import: &'b Import<'b>) -> Option<UnresolvedImportError> {
-        let orig_vis = import.vis.replace(ty::Visibility::Invisible);
+        let orig_vis = import.vis.take();
         let ignore_binding = match &import.kind {
             ImportKind::Single { target_bindings, .. } => target_bindings[TypeNS].get(),
             _ => None,
@@ -727,9 +734,9 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                         });
                     }
                 }
-                if !is_prelude &&
-                   max_vis.get() != ty::Visibility::Invisible && // Allow empty globs.
-                   !max_vis.get().is_at_least(import.vis.get(), &*self.r)
+                if !is_prelude
+                    && let Some(max_vis) = max_vis.get()
+                    && !max_vis.is_at_least(import.expect_vis(), &*self.r)
                 {
                     let msg = "glob import doesn't reexport anything because no candidate is public enough";
                     self.r.lint_buffer.buffer_lint(UNUSED_IMPORTS, import.id, import.span, msg);
@@ -742,7 +749,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         let mut all_ns_err = true;
         self.r.per_ns(|this, ns| {
             if !type_ns_only || ns == TypeNS {
-                let orig_vis = import.vis.replace(ty::Visibility::Invisible);
+                let orig_vis = import.vis.take();
                 let binding = this.resolve_ident_in_module(
                     module,
                     ident,
@@ -906,8 +913,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         let mut crate_private_reexport = false;
         self.r.per_ns(|this, ns| {
             if let Ok(binding) = source_bindings[ns].get() {
-                let vis = import.vis.get();
-                if !binding.vis.is_at_least(vis, &*this) {
+                if !binding.vis.is_at_least(import.expect_vis(), &*this) {
                     reexport_error = Some((ns, binding));
                     if let ty::Visibility::Restricted(binding_def_id) = binding.vis {
                         if binding_def_id.is_top_level_module() {

src/librustdoc/clean/mod.rs

@@ -1776,11 +1776,6 @@ fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
 pub(crate) fn clean_visibility(vis: ty::Visibility) -> Visibility {
     match vis {
         ty::Visibility::Public => Visibility::Public,
-        // NOTE: this is not quite right: `ty` uses `Invisible` to mean 'private',
-        // while rustdoc really does mean inherited. That means that for enum variants, such as
-        // `pub enum E { V }`, `V` will be marked as `Public` by `ty`, but as `Inherited` by rustdoc.
-        // Various parts of clean override `tcx.visibility` explicitly to make sure this distinction is captured.
-        ty::Visibility::Invisible => Visibility::Inherited,
         ty::Visibility::Restricted(module) => Visibility::Restricted(module),
     }
 }