Skip to content

Commit 21b1458

Browse files
committed
Fix order-dependent visibility diagnostics
1 parent 03c609a commit 21b1458

12 files changed

Lines changed: 196 additions & 74 deletions

compiler/rustc_resolve/src/build_reduced_graph.rs

Lines changed: 53 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ use crate::imports::{ImportData, ImportKind, OnUnknownData};
3535
use crate::macros::{MacroRulesDecl, MacroRulesScope, MacroRulesScopeRef};
3636
use crate::ref_mut::CmCell;
3737
use crate::{
38-
BindingKey, Decl, DeclData, DeclKind, ExternModule, ExternPreludeEntry, Finalize, IdentKey,
39-
LocalModule, MacroData, Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, Res,
40-
Resolver, Segment, Used, VisResolutionError, errors,
38+
BindingKey, Decl, DeclData, DeclKind, DelayedVisResolutionError, ExternModule,
39+
ExternPreludeEntry, Finalize, IdentKey, LocalModule, MacroData, Module, ModuleKind,
40+
ModuleOrUniformRoot, ParentScope, PathResult, Res, Resolver, Segment, Used, VisResolutionError,
41+
errors,
4142
};
4243

4344
impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
@@ -362,10 +363,54 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
362363
}
363364

364365
fn resolve_visibility(&mut self, vis: &ast::Visibility) -> Visibility {
365-
self.try_resolve_visibility(vis, true).unwrap_or_else(|err| {
366-
self.r.report_vis_error(err);
367-
Visibility::Public
368-
})
366+
match self.try_resolve_visibility(vis, true) {
367+
Ok(vis) => vis,
368+
Err(err) => {
369+
if !self.delay_vis_resolution(vis) {
370+
self.r.report_vis_error(err);
371+
}
372+
Visibility::Public
373+
}
374+
}
375+
}
376+
377+
fn visibility_path_segments<'ast>(
378+
&self,
379+
path: &'ast ast::Path,
380+
) -> Result<Vec<Segment>, VisResolutionError<'ast>> {
381+
let ident = path.segments.get(0).expect("empty path in visibility").ident;
382+
let crate_root = if ident.is_path_segment_keyword() {
383+
None
384+
} else if ident.span.is_rust_2015() {
385+
Some(Segment::from_ident(Ident::new(
386+
kw::PathRoot,
387+
path.span.shrink_to_lo().with_ctxt(ident.span.ctxt()),
388+
)))
389+
} else {
390+
return Err(VisResolutionError::Relative2018(ident.span, path));
391+
};
392+
393+
Ok(crate_root
394+
.into_iter()
395+
.chain(path.segments.iter().map(|seg| seg.into()))
396+
.collect::<Vec<_>>())
397+
}
398+
399+
fn delay_vis_resolution(&mut self, vis: &ast::Visibility) -> bool {
400+
let ast::VisibilityKind::Restricted { ref path, id, .. } = vis.kind else {
401+
return false;
402+
};
403+
let Ok(segments) = self.visibility_path_segments(path) else {
404+
return false;
405+
};
406+
407+
self.r.delayed_vis_resolution_errors.push(DelayedVisResolutionError {
408+
path: segments,
409+
parent_scope: self.parent_scope,
410+
id,
411+
span: path.span,
412+
});
413+
true
369414
}
370415

371416
fn try_resolve_visibility<'ast>(
@@ -395,22 +440,7 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> {
395440
// paths" right now, so on 2018 edition we only allow module-relative paths for now.
396441
// On 2015 edition visibilities are resolved as crate-relative by default,
397442
// so we are prepending a root segment if necessary.
398-
let ident = path.segments.get(0).expect("empty path in visibility").ident;
399-
let crate_root = if ident.is_path_segment_keyword() {
400-
None
401-
} else if ident.span.is_rust_2015() {
402-
Some(Segment::from_ident(Ident::new(
403-
kw::PathRoot,
404-
path.span.shrink_to_lo().with_ctxt(ident.span.ctxt()),
405-
)))
406-
} else {
407-
return Err(VisResolutionError::Relative2018(ident.span, path));
408-
};
409-
410-
let segments = crate_root
411-
.into_iter()
412-
.chain(path.segments.iter().map(|seg| seg.into()))
413-
.collect::<Vec<_>>();
443+
let segments = self.visibility_path_segments(path)?;
414444
let expected_found_error = |res| {
415445
Err(VisResolutionError::ExpectedFound(
416446
path.span,

compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,10 @@ use crate::imports::{Import, ImportKind};
4848
use crate::late::{DiagMetadata, PatternSource, Rib};
4949
use crate::{
5050
AmbiguityError, AmbiguityKind, AmbiguityWarning, BindingError, BindingKey, Decl, DeclKind,
51-
Finalize, ForwardGenericParamBanReason, HasGenericParams, IdentKey, LateDecl, MacroRulesScope,
52-
Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult, PrivacyError, Res,
53-
ResolutionError, Resolver, Scope, ScopeSet, Segment, UseError, Used, VisResolutionError,
54-
errors as errs, path_names_to_string,
51+
DelayedVisResolutionError, Finalize, ForwardGenericParamBanReason, HasGenericParams, IdentKey,
52+
LateDecl, MacroRulesScope, Module, ModuleKind, ModuleOrUniformRoot, ParentScope, PathResult,
53+
PrivacyError, Res, ResolutionError, Resolver, Scope, ScopeSet, Segment, UseError, Used,
54+
VisResolutionError, errors as errs, path_names_to_string,
5555
};
5656

5757
/// A vector of spans and replacements, a message and applicability.
@@ -137,6 +137,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
137137
}
138138

139139
pub(crate) fn report_errors(&mut self, krate: &Crate) {
140+
self.report_delayed_vis_resolution_errors();
140141
self.report_with_use_injections(krate);
141142

142143
for &(span_use, span_def) in &self.macro_expanded_macro_export_errors {
@@ -178,6 +179,65 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
178179
}
179180
}
180181

182+
fn report_delayed_vis_resolution_errors(&mut self) {
183+
for DelayedVisResolutionError { path, parent_scope, id, span } in
184+
std::mem::take(&mut self.delayed_vis_resolution_errors)
185+
{
186+
let path_result = self.cm().resolve_path(
187+
&path,
188+
None,
189+
&parent_scope,
190+
Some(Finalize::new(id, span)),
191+
None,
192+
None,
193+
);
194+
match path_result {
195+
PathResult::Module(ModuleOrUniformRoot::Module(module)) => {
196+
let res = module.res().expect("visibility resolved to unnamed block");
197+
if module.is_normal() {
198+
match res {
199+
Res::Err => {}
200+
_ => {
201+
let vis = Visibility::Restricted(res.def_id());
202+
if !self.is_accessible_from(vis, parent_scope.module) {
203+
self.report_vis_error(VisResolutionError::AncestorOnly(span));
204+
}
205+
}
206+
}
207+
} else {
208+
self.report_vis_error(VisResolutionError::ExpectedFound(
209+
span,
210+
Segment::names_to_string(&path),
211+
res,
212+
));
213+
}
214+
}
215+
PathResult::Module(..) => {
216+
self.report_vis_error(VisResolutionError::ModuleOnly(span));
217+
}
218+
PathResult::NonModule(partial_res) => {
219+
self.report_vis_error(VisResolutionError::ExpectedFound(
220+
span,
221+
Segment::names_to_string(&path),
222+
partial_res.expect_full_res(),
223+
));
224+
}
225+
PathResult::Failed { span, segment_name, label, suggestion, message, .. } => {
226+
self.report_vis_error(VisResolutionError::FailedToResolve(
227+
span,
228+
segment_name,
229+
label,
230+
suggestion,
231+
message,
232+
));
233+
}
234+
PathResult::Indeterminate => {
235+
self.report_vis_error(VisResolutionError::Indeterminate(span));
236+
}
237+
}
238+
}
239+
}
240+
181241
fn report_with_use_injections(&mut self, krate: &Crate) {
182242
for UseError { mut err, candidates, def_id, instead, suggestion, path, is_call } in
183243
std::mem::take(&mut self.use_injections)

compiler/rustc_resolve/src/lib.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -997,6 +997,14 @@ struct UseError<'a> {
997997
is_call: bool,
998998
}
999999

1000+
#[derive(Debug)]
1001+
struct DelayedVisResolutionError<'ra> {
1002+
path: Vec<Segment>,
1003+
parent_scope: ParentScope<'ra>,
1004+
id: NodeId,
1005+
span: Span,
1006+
}
1007+
10001008
#[derive(Clone, Copy, PartialEq, Debug)]
10011009
enum AmbiguityKind {
10021010
BuiltinAttr,
@@ -1351,6 +1359,8 @@ pub struct Resolver<'ra, 'tcx> {
13511359
issue_145575_hack_applied: bool = false,
13521360
/// `use` injections are delayed for better placement and deduplication.
13531361
use_injections: Vec<UseError<'tcx>> = Vec::new(),
1362+
/// Visibility path resolution failures are delayed until all modules are collected.
1363+
delayed_vis_resolution_errors: Vec<DelayedVisResolutionError<'ra>> = Vec::new(),
13541364
/// Crate-local macro expanded `macro_export` referred to by a module-relative path.
13551365
macro_expanded_macro_export_errors: BTreeSet<(Span, Span)> = BTreeSet::new(),
13561366

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,3 @@
1-
error[E0742]: visibilities can only be restricted to ancestor modules
2-
--> $DIR/relative-2018.rs:7:12
3-
|
4-
LL | pub(in ::core) struct S4;
5-
| ^^^^^^
6-
71
error: relative paths are not supported in visibilities in 2018 edition or later
82
--> $DIR/relative-2018.rs:9:12
93
|
@@ -12,6 +6,12 @@ LL | pub(in a::b) struct S5;
126
| |
137
| help: try: `crate::a::b`
148

9+
error[E0742]: visibilities can only be restricted to ancestor modules
10+
--> $DIR/relative-2018.rs:7:12
11+
|
12+
LL | pub(in ::core) struct S4;
13+
| ^^^^^^
14+
1515
error: aborting due to 2 previous errors
1616

1717
For more information about this error, try `rustc --explain E0742`.

tests/ui/privacy/restricted/test.stderr

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
error[E0364]: `f` is private, and cannot be re-exported
2+
--> $DIR/test.rs:22:24
3+
|
4+
LL | pub(super) use foo::bar::f as g;
5+
| ^^^^^^^^^^^^^^^^
6+
|
7+
note: consider marking `f` as `pub` in the imported module
8+
--> $DIR/test.rs:22:24
9+
|
10+
LL | pub(super) use foo::bar::f as g;
11+
| ^^^^^^^^^^^^^^^^
12+
113
error[E0433]: cannot find module or crate `bad` in the crate root
214
--> $DIR/test.rs:51:12
315
|
@@ -15,18 +27,6 @@ error[E0742]: visibilities can only be restricted to ancestor modules
1527
LL | pub(in foo) mod m2 {}
1628
| ^^^
1729

18-
error[E0364]: `f` is private, and cannot be re-exported
19-
--> $DIR/test.rs:22:24
20-
|
21-
LL | pub(super) use foo::bar::f as g;
22-
| ^^^^^^^^^^^^^^^^
23-
|
24-
note: consider marking `f` as `pub` in the imported module
25-
--> $DIR/test.rs:22:24
26-
|
27-
LL | pub(super) use foo::bar::f as g;
28-
| ^^^^^^^^^^^^^^^^
29-
3030
error[E0603]: struct `Crate` is private
3131
--> $DIR/test.rs:39:25
3232
|

tests/ui/resolve/resolve-bad-visibility.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,9 @@ pub(in E) struct S; //~ ERROR expected module, found enum `::E`
66
pub(in Tr) struct Z; //~ ERROR expected module, found trait `::Tr`
77
pub(in std::vec) struct F; //~ ERROR visibilities can only be restricted to ancestor modules
88
pub(in nonexistent) struct G; //~ ERROR cannot find
9-
pub(in too_soon) struct H; //~ ERROR cannot find
9+
pub(in too_soon) struct H; //~ ERROR visibilities can only be restricted
1010

11-
// Visibilities are resolved eagerly without waiting for modules becoming fully populated.
12-
// Visibilities can only use ancestor modules legally which are always available in time,
13-
// so the worst thing that can happen due to eager resolution is a suboptimal error message.
11+
// The module exists, but it is not an ancestor module.
1412
mod too_soon {}
1513

1614
fn main () {}

tests/ui/resolve/resolve-bad-visibility.stderr

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,11 @@ help: you might be missing a crate named `nonexistent`, add it to your project a
2727
LL + extern crate nonexistent;
2828
|
2929

30-
error[E0433]: cannot find module or crate `too_soon` in the crate root
30+
error[E0742]: visibilities can only be restricted to ancestor modules
3131
--> $DIR/resolve-bad-visibility.rs:9:8
3232
|
3333
LL | pub(in too_soon) struct H;
34-
| ^^^^^^^^ use of unresolved module or unlinked crate `too_soon`
35-
|
36-
help: you might be missing a crate named `too_soon`, add it to your project and import it in your code
37-
|
38-
LL + extern crate too_soon;
39-
|
34+
| ^^^^^^^^
4035

4136
error: aborting due to 5 previous errors
4237

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
error[E0433]: cannot find `bar` in the crate root
2-
--> $DIR/visibility-indeterminate.rs:5:10
3-
|
4-
LL | pub(in ::bar) struct Baz {}
5-
| ^^^ could not find `bar` in the list of imported crates
6-
71
error: cannot find macro `foo` in this scope
82
--> $DIR/visibility-indeterminate.rs:3:1
93
|
104
LL | foo!();
115
| ^^^
126

7+
error[E0433]: cannot find `bar` in the crate root
8+
--> $DIR/visibility-indeterminate.rs:5:10
9+
|
10+
LL | pub(in ::bar) struct Baz {}
11+
| ^^^ could not find `bar` in the list of imported crates
12+
1313
error: aborting due to 2 previous errors
1414

1515
For more information about this error, try `rustc --explain E0433`.
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//@ edition: 2021
2+
// Regression test for <https://github.com/rust-lang/rust/issues/40066>.
3+
4+
mod foo {
5+
pub(in crate::bar) struct Foo;
6+
//~^ ERROR visibilities can only be restricted to ancestor modules
7+
}
8+
9+
mod bar {
10+
pub(in crate::foo) struct Bar;
11+
//~^ ERROR visibilities can only be restricted to ancestor modules
12+
}
13+
14+
fn main() {}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error[E0742]: visibilities can only be restricted to ancestor modules
2+
--> $DIR/visibility-order-dependent.rs:5:12
3+
|
4+
LL | pub(in crate::bar) struct Foo;
5+
| ^^^^^^^^^^
6+
7+
error[E0742]: visibilities can only be restricted to ancestor modules
8+
--> $DIR/visibility-order-dependent.rs:10:12
9+
|
10+
LL | pub(in crate::foo) struct Bar;
11+
| ^^^^^^^^^^
12+
13+
error: aborting due to 2 previous errors
14+
15+
For more information about this error, try `rustc --explain E0742`.

0 commit comments

Comments
 (0)