Fix order-dependent visibility diagnostics

SynapLink · SynapLink · commit 4f56ffe81997 · 2026-04-29T19:51:26.000+02:00
diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -35,9 +35,10 @@ use crate::imports::{ImportData, ImportKind, OnUnknownData};
 use crate::macros::{MacroRulesDecl, MacroRulesScope, MacroRulesScopeRef};
 use crate::ref_mut::CmCell;
 use crate::{
-    BindingKey, Decl, DeclData, DeclKind, ExternModule, ExternPreludeEntry, Finalize, IdentKey,
-    LocalModule, MacroData, Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, Res,
-    Resolver, Segment, Used, VisResolutionError, errors,
+    BindingKey, Decl, DeclData, DeclKind, DelayedVisResolutionError, ExternModule,
+    ExternPreludeEntry, Finalize, IdentKey, LocalModule, MacroData, Module, ModuleKind,
+    ModuleOrUniformRoot, ParentScope, PathResult, Res, Resolver, Segment, Used, VisResolutionError,
+    errors,
 };
 
 impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
@@ -355,38 +356,50 @@ impl<'ra, 'tcx> AsMut<Resolver<'ra, 'tcx>> for DefCollector<'_, 'ra, 'tcx> {
     }
 }
 
-impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
-    fn res(&self, def_id: impl Into<DefId>) -> Res {
-        let def_id = def_id.into();
-        Res::Def(self.r.tcx.def_kind(def_id), def_id)
-    }
+impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
+    fn visibility_path_segments(
+        &self,
+        path: &ast::Path,
+    ) -> Result<Vec<Segment>, VisResolutionError> {
+        let ident = path.segments.get(0).expect("empty path in visibility").ident;
+        let crate_root = if ident.is_path_segment_keyword() {
+            None
+        } else if ident.span.is_rust_2015() {
+            Some(Segment::from_ident(Ident::new(
+                kw::PathRoot,
+                path.span.shrink_to_lo().with_ctxt(ident.span.ctxt()),
+            )))
+        } else {
+            return Err(VisResolutionError::Relative2018(ident.span, path.clone()));
+        };
 
-    fn resolve_visibility(&mut self, vis: &ast::Visibility) -> Visibility {
-        self.try_resolve_visibility(vis, true).unwrap_or_else(|err| {
-            self.r.report_vis_error(err);
-            Visibility::Public
-        })
+        Ok(crate_root
+            .into_iter()
+            .chain(path.segments.iter().map(|seg| seg.into()))
+            .collect::<Vec<_>>())
     }
 
-    fn try_resolve_visibility<'ast>(
+    pub(crate) fn try_resolve_visibility(
         &mut self,
-        vis: &'ast ast::Visibility,
+        parent_scope: ParentScope<'ra>,
+        vis: &ast::Visibility,
         finalize: bool,
-    ) -> Result<Visibility, VisResolutionError<'ast>> {
-        let parent_scope = &self.parent_scope;
+        record_partial_res: bool,
+    ) -> Result<Visibility, VisResolutionError> {
+        let parent_scope_ref = &parent_scope;
         match vis.kind {
             ast::VisibilityKind::Public => Ok(Visibility::Public),
             ast::VisibilityKind::Inherited => {
-                Ok(match self.parent_scope.module.kind {
+                Ok(match parent_scope.module.kind {
                     // Any inherited visibility resolved directly inside an enum or trait
                     // (i.e. variants, fields, and trait items) inherits from the visibility
                     // of the enum or trait.
                     ModuleKind::Def(DefKind::Enum | DefKind::Trait, def_id, _) => {
-                        self.r.tcx.visibility(def_id).expect_local()
+                        self.tcx.visibility(def_id).expect_local()
                     }
                     // Otherwise, the visibility is restricted to the nearest parent `mod` item.
                     _ => Visibility::Restricted(
-                        self.parent_scope.module.nearest_parent_mod().expect_local(),
+                        parent_scope.module.nearest_parent_mod().expect_local(),
                     ),
                 })
             }
@@ -395,48 +408,33 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
                 // paths" right now, so on 2018 edition we only allow module-relative paths for now.
                 // On 2015 edition visibilities are resolved as crate-relative by default,
                 // so we are prepending a root segment if necessary.
-                let ident = path.segments.get(0).expect("empty path in visibility").ident;
-                let crate_root = if ident.is_path_segment_keyword() {
-                    None
-                } else if ident.span.is_rust_2015() {
-                    Some(Segment::from_ident(Ident::new(
-                        kw::PathRoot,
-                        path.span.shrink_to_lo().with_ctxt(ident.span.ctxt()),
-                    )))
-                } else {
-                    return Err(VisResolutionError::Relative2018(ident.span, path));
-                };
-
-                let segments = crate_root
-                    .into_iter()
-                    .chain(path.segments.iter().map(|seg| seg.into()))
-                    .collect::<Vec<_>>();
+                let segments = self.visibility_path_segments(path)?;
                 let expected_found_error = |res| {
                     Err(VisResolutionError::ExpectedFound(
                         path.span,
                         Segment::names_to_string(&segments),
                         res,
                     ))
                 };
-                match self.r.cm().resolve_path(
+                match self.cm().resolve_path(
                     &segments,
                     None,
-                    parent_scope,
+                    parent_scope_ref,
                     finalize.then(|| Finalize::new(id, path.span)),
                     None,
                     None,
                 ) {
                     PathResult::Module(ModuleOrUniformRoot::Module(module)) => {
                         let res = module.res().expect("visibility resolved to unnamed block");
-                        if finalize {
-                            self.r.record_partial_res(id, PartialRes::new(res));
+                        if record_partial_res {
+                            self.record_partial_res(id, PartialRes::new(res));
                         }
                         if module.is_normal() {
                             match res {
                                 Res::Err => Ok(Visibility::Public),
                                 _ => {
                                     let vis = Visibility::Restricted(res.def_id());
-                                    if self.r.is_accessible_from(vis, parent_scope.module) {
+                                    if self.is_accessible_from(vis, parent_scope.module) {
                                         Ok(vis.expect_local())
                                     } else {
                                         Err(VisResolutionError::AncestorOnly(path.span))
@@ -465,6 +463,27 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
             }
         }
     }
+}
+
+impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
+    fn res(&self, def_id: impl Into<DefId>) -> Res {
+        let def_id = def_id.into();
+        Res::Def(self.r.tcx.def_kind(def_id), def_id)
+    }
+
+    fn resolve_visibility(&mut self, vis: &ast::Visibility) -> Visibility {
+        match self.r.try_resolve_visibility(self.parent_scope, vis, true, true) {
+            Ok(vis) => vis,
+            Err(error) => {
+                self.r.delayed_vis_resolution_errors.push(DelayedVisResolutionError {
+                    vis: vis.clone(),
+                    parent_scope: self.parent_scope,
+                    error,
+                });
+                Visibility::Public
+            }
+        }
+    }
 
     fn insert_field_idents(&mut self, def_id: LocalDefId, fields: &[ast::FieldDef]) {
         if fields.iter().any(|field| field.is_placeholder) {
@@ -904,7 +923,8 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
                         // correct visibilities for unnamed field placeholders specifically, so the
                         // constructor visibility should still be determined correctly.
                         let field_vis = self
-                            .try_resolve_visibility(&field.vis, false)
+                            .r
+                            .try_resolve_visibility(self.parent_scope, &field.vis, false, false)
                             .unwrap_or(Visibility::Public);
                         if ctor_vis.greater_than(field_vis, self.r.tcx) {
                             ctor_vis = field_vis;
@@ -1339,9 +1359,10 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
             let vis = match item.kind {
                 // Visibilities must not be resolved non-speculatively twice
                 // and we already resolved this one as a `fn` item visibility.
-                ItemKind::Fn(..) => {
-                    self.try_resolve_visibility(&item.vis, false).unwrap_or(Visibility::Public)
-                }
+                ItemKind::Fn(..) => self
+                    .r
+                    .try_resolve_visibility(self.parent_scope, &item.vis, false, false)
+                    .unwrap_or(Visibility::Public),
                 _ => self.resolve_visibility(&item.vis),
             };
             if !vis.is_public() {
diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs
@@ -48,10 +48,10 @@ use crate::imports::{Import, ImportKind};
 use crate::late::{DiagMetadata, PatternSource, Rib};
 use crate::{
     AmbiguityError, AmbiguityKind, AmbiguityWarning, BindingError, BindingKey, Decl, DeclKind,
-    Finalize, ForwardGenericParamBanReason, HasGenericParams, IdentKey, LateDecl, MacroRulesScope,
-    Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, PrivacyError, Res,
-    ResolutionError, Resolver, Scope, ScopeSet, Segment, UseError, Used, VisResolutionError,
-    errors as errs, path_names_to_string,
+    DelayedVisResolutionError, Finalize, ForwardGenericParamBanReason, HasGenericParams, IdentKey,
+    LateDecl, MacroRulesScope, Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult,
+    PrivacyError, Res, ResolutionError, Resolver, Scope, ScopeSet, Segment, UseError, Used,
+    VisResolutionError, errors as errs, path_names_to_string,
 };
 
 /// A vector of spans and replacements, a message and applicability.
@@ -137,6 +137,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
     }
 
     pub(crate) fn report_errors(&mut self, krate: &Crate) {
+        self.report_delayed_vis_resolution_errors();
         self.report_with_use_injections(krate);
 
         for &(span_use, span_def) in &self.macro_expanded_macro_export_errors {
@@ -178,6 +179,21 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         }
     }
 
+    fn report_delayed_vis_resolution_errors(&mut self) {
+        for DelayedVisResolutionError { vis, parent_scope, error } in
+            std::mem::take(&mut self.delayed_vis_resolution_errors)
+        {
+            match self.try_resolve_visibility(parent_scope, &vis, true, false) {
+                Ok(_) => {
+                    self.report_vis_error(error);
+                }
+                Err(error) => {
+                    self.report_vis_error(error);
+                }
+            }
+        }
+    }
+
     fn report_with_use_injections(&mut self, krate: &Crate) {
         for UseError { mut err, candidates, def_id, instead, suggestion, path, is_call } in
             std::mem::take(&mut self.use_injections)
@@ -1127,7 +1143,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
 
     pub(crate) fn report_vis_error(
         &mut self,
-        vis_resolution_error: VisResolutionError<'_>,
+        vis_resolution_error: VisResolutionError,
     ) -> ErrorGuaranteed {
         match vis_resolution_error {
             VisResolutionError::Relative2018(span, path) => {
@@ -1136,7 +1152,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     path_span: path.span,
                     // intentionally converting to String, as the text would also be used as
                     // in suggestion context
-                    path_str: pprust::path_to_string(path),
+                    path_str: pprust::path_to_string(&path),
                 })
             }
             VisResolutionError::AncestorOnly(span) => {
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -336,8 +336,9 @@ enum ResolutionError<'ra> {
     BindingInNeverPattern,
 }
 
-enum VisResolutionError<'a> {
-    Relative2018(Span, &'a ast::Path),
+#[derive(Debug)]
+enum VisResolutionError {
+    Relative2018(Span, ast::Path),
     AncestorOnly(Span),
     FailedToResolve(Span, Symbol, String, Option<Suggestion>, String),
     ExpectedFound(Span, String, Res),
@@ -997,6 +998,13 @@ struct UseError<'a> {
     is_call: bool,
 }
 
+#[derive(Debug)]
+struct DelayedVisResolutionError<'ra> {
+    vis: ast::Visibility,
+    parent_scope: ParentScope<'ra>,
+    error: VisResolutionError,
+}
+
 #[derive(Clone, Copy, PartialEq, Debug)]
 enum AmbiguityKind {
     BuiltinAttr,
@@ -1351,6 +1359,8 @@ pub struct Resolver<'ra, 'tcx> {
     issue_145575_hack_applied: bool = false,
     /// `use` injections are delayed for better placement and deduplication.
     use_injections: Vec<UseError<'tcx>> = Vec::new(),
+    /// Visibility path resolution failures are delayed until all modules are collected.
+    delayed_vis_resolution_errors: Vec<DelayedVisResolutionError<'ra>> = Vec::new(),
     /// Crate-local macro expanded `macro_export` referred to by a module-relative path.
     macro_expanded_macro_export_errors: BTreeSet<(Span, Span)> = BTreeSet::new(),
 
diff --git a/tests/ui/privacy/restricted/test.stderr b/tests/ui/privacy/restricted/test.stderr
@@ -1,3 +1,15 @@
+error[E0364]: `f` is private, and cannot be re-exported
+  --> $DIR/test.rs:22:24
+   |
+LL |         pub(super) use foo::bar::f as g;
+   |                        ^^^^^^^^^^^^^^^^
+   |
+note: consider marking `f` as `pub` in the imported module
+  --> $DIR/test.rs:22:24
+   |
+LL |         pub(super) use foo::bar::f as g;
+   |                        ^^^^^^^^^^^^^^^^
+
 error[E0433]: cannot find module or crate `bad` in the crate root
   --> $DIR/test.rs:51:12
    |
@@ -15,18 +27,6 @@ error[E0742]: visibilities can only be restricted to ancestor modules
 LL |     pub(in foo) mod m2 {}
    |            ^^^
 
-error[E0364]: `f` is private, and cannot be re-exported
-  --> $DIR/test.rs:22:24
-   |
-LL |         pub(super) use foo::bar::f as g;
-   |                        ^^^^^^^^^^^^^^^^
-   |
-note: consider marking `f` as `pub` in the imported module
-  --> $DIR/test.rs:22:24
-   |
-LL |         pub(super) use foo::bar::f as g;
-   |                        ^^^^^^^^^^^^^^^^
-
 error[E0603]: struct `Crate` is private
   --> $DIR/test.rs:39:25
    |
diff --git a/tests/ui/resolve/resolve-bad-visibility.rs b/tests/ui/resolve/resolve-bad-visibility.rs
@@ -6,11 +6,9 @@ pub(in E) struct S; //~ ERROR expected module, found enum `::E`
 pub(in Tr) struct Z; //~ ERROR expected module, found trait `::Tr`
 pub(in std::vec) struct F; //~ ERROR visibilities can only be restricted to ancestor modules
 pub(in nonexistent) struct G; //~ ERROR cannot find
-pub(in too_soon) struct H; //~ ERROR cannot find
+pub(in too_soon) struct H; //~ ERROR visibilities can only be restricted
 
-// Visibilities are resolved eagerly without waiting for modules becoming fully populated.
-// Visibilities can only use ancestor modules legally which are always available in time,
-// so the worst thing that can happen due to eager resolution is a suboptimal error message.
+// The module exists, but it is not an ancestor module.
 mod too_soon {}
 
 fn main () {}
diff --git a/tests/ui/resolve/resolve-bad-visibility.stderr b/tests/ui/resolve/resolve-bad-visibility.stderr
@@ -27,16 +27,11 @@ help: you might be missing a crate named `nonexistent`, add it to your project a
 LL + extern crate nonexistent;
    |
 
-error[E0433]: cannot find module or crate `too_soon` in the crate root
+error[E0742]: visibilities can only be restricted to ancestor modules
   --> $DIR/resolve-bad-visibility.rs:9:8
    |
 LL | pub(in too_soon) struct H;
-   |        ^^^^^^^^ use of unresolved module or unlinked crate `too_soon`
-   |
-help: you might be missing a crate named `too_soon`, add it to your project and import it in your code
-   |
-LL + extern crate too_soon;
-   |
+   |        ^^^^^^^^
 
 error: aborting due to 5 previous errors
 
diff --git a/tests/ui/resolve/visibility-indeterminate.stderr b/tests/ui/resolve/visibility-indeterminate.stderr
@@ -1,15 +1,15 @@
-error[E0433]: cannot find `bar` in the crate root
-  --> $DIR/visibility-indeterminate.rs:5:10
-   |
-LL | pub(in ::bar) struct Baz {}
-   |          ^^^ could not find `bar` in the list of imported crates
-
 error: cannot find macro `foo` in this scope
   --> $DIR/visibility-indeterminate.rs:3:1
    |
 LL | foo!();
    | ^^^
 
+error[E0433]: cannot find `bar` in the crate root
+  --> $DIR/visibility-indeterminate.rs:5:10
+   |
+LL | pub(in ::bar) struct Baz {}
+   |          ^^^ could not find `bar` in the list of imported crates
+
 error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0433`.
diff --git a/tests/ui/resolve/visibility-order-dependent.rs b/tests/ui/resolve/visibility-order-dependent.rs
@@ -0,0 +1,14 @@
+//@ edition: 2021
+// Regression test for <https://github.com/rust-lang/rust/issues/40066>.
+
+mod foo {
+    pub(in crate::bar) struct Foo;
+    //~^ ERROR visibilities can only be restricted to ancestor modules
+}
+
+mod bar {
+    pub(in crate::foo) struct Bar;
+    //~^ ERROR visibilities can only be restricted to ancestor modules
+}
+
+fn main() {}
diff --git a/tests/ui/resolve/visibility-order-dependent.stderr b/tests/ui/resolve/visibility-order-dependent.stderr
diff --git a/tests/ui/span/visibility-ty-params.stderr b/tests/ui/span/visibility-ty-params.stderr
diff --git a/tests/ui/use/use-self-type.stderr b/tests/ui/use/use-self-type.stderr
