Skip to content

Fix order-dependent visibility diagnostics #155948

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rust-bors merged 1 commit into from
May 1, 2026
Merged

Fix order-dependent visibility diagnostics #155948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
231 changes: 124 additions & 107 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -235,6 +236,111 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}
}

pub(crate) fn try_resolve_visibility(
&mut self,
parent_scope: &ParentScope<'ra>,
vis: &ast::Visibility,
finalize: bool,
) -> Result<Visibility, VisResolutionError> {
match vis.kind {
ast::VisibilityKind::Public => Ok(Visibility::Public),
ast::VisibilityKind::Inherited => {
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.tcx.visibility(def_id).expect_local()
}
// Otherwise, the visibility is restricted to the nearest parent `mod` item.
_ => Visibility::Restricted(
parent_scope.module.nearest_parent_mod().expect_local(),
),
})
}
ast::VisibilityKind::Restricted { ref path, id, .. } => {
// For visibilities we are not ready to provide correct implementation of "uniform
// 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.as_ref().clone(),
));
};
let segments = crate_root
.into_iter()
.chain(path.segments.iter().map(|seg| seg.into()))
.collect::<Vec<_>>();
let expected_found_error = |res| {
Err(VisResolutionError::ExpectedFound(
path.span,
Segment::names_to_string(&segments),
res,
))
};
match self.cm().resolve_path(
&segments,
None,
parent_scope,
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 module.is_normal() {
match res {
Res::Err => {
if finalize {
self.record_partial_res(id, PartialRes::new(res));
}
Ok(Visibility::Public)
}
_ => {
let vis = Visibility::Restricted(res.def_id());
if self.is_accessible_from(vis, parent_scope.module) {
if finalize {
self.record_partial_res(id, PartialRes::new(res));
}
Ok(vis.expect_local())
} else {
Err(VisResolutionError::AncestorOnly(path.span))
}
}
}
} else {
expected_found_error(res)
}
}
PathResult::Module(..) => Err(VisResolutionError::ModuleOnly(path.span)),
PathResult::NonModule(partial_res) => {
expected_found_error(partial_res.expect_full_res())
}
PathResult::Failed {
span, label, suggestion, message, segment_name, ..
} => Err(VisResolutionError::FailedToResolve(
span,
segment_name,
label,
suggestion,
message,
)),
PathResult::Indeterminate => Err(VisResolutionError::Indeterminate(path.span)),
}
}
}
}

pub(crate) fn build_reduced_graph_external(&self, module: ExternModule<'ra>) {
let def_id = module.def_id();
let children = self.tcx.module_children(def_id);
Expand Down Expand Up @@ -362,106 +468,15 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
}

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
})
}

fn try_resolve_visibility<'ast>(
&mut self,
vis: &'ast ast::Visibility,
finalize: bool,
) -> Result<Visibility, VisResolutionError<'ast>> {
let parent_scope = &self.parent_scope;
match vis.kind {
ast::VisibilityKind::Public => Ok(Visibility::Public),
ast::VisibilityKind::Inherited => {
Ok(match self.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()
}
// Otherwise, the visibility is restricted to the nearest parent `mod` item.
_ => Visibility::Restricted(
self.parent_scope.module.nearest_parent_mod().expect_local(),
),
})
}
ast::VisibilityKind::Restricted { ref path, id, .. } => {
// For visibilities we are not ready to provide correct implementation of "uniform
// 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 expected_found_error = |res| {
Err(VisResolutionError::ExpectedFound(
path.span,
Segment::names_to_string(&segments),
res,
))
};
match self.r.cm().resolve_path(
&segments,
None,
parent_scope,
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 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) {
Ok(vis.expect_local())
} else {
Err(VisResolutionError::AncestorOnly(path.span))
}
}
}
} else {
expected_found_error(res)
}
}
PathResult::Module(..) => Err(VisResolutionError::ModuleOnly(path.span)),
PathResult::NonModule(partial_res) => {
expected_found_error(partial_res.expect_full_res())
}
PathResult::Failed {
span, label, suggestion, message, segment_name, ..
} => Err(VisResolutionError::FailedToResolve(
span,
segment_name,
label,
suggestion,
message,
)),
PathResult::Indeterminate => Err(VisResolutionError::Indeterminate(path.span)),
}
match self.r.try_resolve_visibility(&self.parent_scope, vis, 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
}
}
}
Expand Down Expand Up @@ -904,7 +919,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)
.unwrap_or(Visibility::Public);
if ctor_vis.greater_than(field_vis, self.r.tcx) {
ctor_vis = field_vis;
Expand Down Expand Up @@ -1339,9 +1355,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)
.unwrap_or(Visibility::Public),
_ => self.resolve_visibility(&item.vis),
};
if !vis.is_public() {
Expand Down
29 changes: 21 additions & 8 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// ignore-tidy-filelength
use std::mem;
use std::ops::ControlFlow;

use itertools::Itertools as _;
Expand Down Expand Up @@ -48,10 +49,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.
Expand Down Expand Up @@ -137,6 +138,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 {
Expand Down Expand Up @@ -171,16 +173,27 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}

let mut reported_spans = FxHashSet::default();
for error in std::mem::take(&mut self.privacy_errors) {
for error in mem::take(&mut self.privacy_errors) {
if reported_spans.insert(error.dedup_span) {
self.report_privacy_error(&error);
}
}
}

fn report_delayed_vis_resolution_errors(&mut self) {
for DelayedVisResolutionError { vis, parent_scope, error } in
mem::take(&mut self.delayed_vis_resolution_errors)
{
match self.try_resolve_visibility(&parent_scope, &vis, true) {
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)
mem::take(&mut self.use_injections)
{
let (span, found_use) = if let Some(def_id) = def_id.as_local() {
UsePlacementFinder::check(krate, self.def_id_to_node_id(def_id))
Expand Down Expand Up @@ -1127,7 +1140,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) => {
Expand All @@ -1136,7 +1149,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) => {
Expand Down
14 changes: 12 additions & 2 deletions compiler/rustc_resolve/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),

Expand Down
Loading
Loading