Skip to content

Commit 540d837

Browse files
petrochenkovpietroalbini
authored andcommitted
resolve: Make sure macros and imports are resolved in full parent scope
Slightly simplify `fn build_reduced_graph_for_use_tree`
1 parent feea133 commit 540d837

File tree

5 files changed

+74
-65
lines changed

5 files changed

+74
-65
lines changed

src/librustc_resolve/build_reduced_graph.rs

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -111,23 +111,23 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
111111

112112
fn build_reduced_graph_for_use_tree(
113113
&mut self,
114-
root_use_tree: &ast::UseTree,
115-
root_id: NodeId,
114+
// This particular use tree
116115
use_tree: &ast::UseTree,
117116
id: NodeId,
118-
vis: ty::Visibility,
119117
parent_prefix: &[Ident],
120-
mut uniform_paths_canary_emitted: bool,
121118
nested: bool,
122-
item: &Item,
119+
mut uniform_paths_canary_emitted: bool,
120+
// The whole `use` item
123121
parent_scope: ParentScope<'a>,
122+
item: &Item,
123+
vis: ty::Visibility,
124+
root_span: Span,
124125
) {
125126
debug!("build_reduced_graph_for_use_tree(parent_prefix={:?}, \
126127
uniform_paths_canary_emitted={}, \
127128
use_tree={:?}, nested={})",
128129
parent_prefix, uniform_paths_canary_emitted, use_tree, nested);
129130

130-
let is_prelude = attr::contains_name(&item.attrs, "prelude_import");
131131
let uniform_paths =
132132
self.session.rust_2018() &&
133133
self.session.features_untracked().uniform_paths;
@@ -215,8 +215,8 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
215215
subclass,
216216
source.span,
217217
id,
218-
root_use_tree.span,
219-
root_id,
218+
root_span,
219+
item.id,
220220
ty::Visibility::Invisible,
221221
parent_scope.clone(),
222222
true, // is_uniform_paths_canary
@@ -339,25 +339,25 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
339339
subclass,
340340
use_tree.span,
341341
id,
342-
root_use_tree.span,
343-
root_id,
342+
root_span,
343+
item.id,
344344
vis,
345345
parent_scope,
346346
false, // is_uniform_paths_canary
347347
);
348348
}
349349
ast::UseTreeKind::Glob => {
350350
let subclass = GlobImport {
351-
is_prelude,
351+
is_prelude: attr::contains_name(&item.attrs, "prelude_import"),
352352
max_vis: Cell::new(ty::Visibility::Invisible),
353353
};
354354
self.add_import_directive(
355355
prefix,
356356
subclass,
357357
use_tree.span,
358358
id,
359-
root_use_tree.span,
360-
root_id,
359+
root_span,
360+
item.id,
361361
vis,
362362
parent_scope,
363363
false, // is_uniform_paths_canary
@@ -388,16 +388,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
388388

389389
for &(ref tree, id) in items {
390390
self.build_reduced_graph_for_use_tree(
391-
root_use_tree,
392-
root_id,
393-
tree,
394-
id,
395-
vis,
396-
&prefix,
397-
uniform_paths_canary_emitted,
398-
true,
399-
item,
400-
parent_scope.clone(),
391+
// This particular use tree
392+
tree, id, &prefix, true, uniform_paths_canary_emitted,
393+
// The whole `use` item
394+
parent_scope.clone(), item, vis, root_span,
401395
);
402396
}
403397
}
@@ -415,16 +409,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
415409
match item.node {
416410
ItemKind::Use(ref use_tree) => {
417411
self.build_reduced_graph_for_use_tree(
418-
use_tree,
419-
item.id,
420-
use_tree,
421-
item.id,
422-
vis,
423-
&[],
424-
false, // uniform_paths_canary_emitted
425-
false,
426-
item,
427-
parent_scope,
412+
// This particular use tree
413+
use_tree, item.id, &[], false, false,
414+
// The whole `use` item
415+
parent_scope, item, vis, use_tree.span,
428416
);
429417
}
430418

src/librustc_resolve/error_reporting.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
// except according to those terms.
1010

1111
use {CrateLint, PathResult};
12+
use macros::ParentScope;
1213

1314
use std::collections::BTreeSet;
1415

@@ -24,6 +25,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
2425
&mut self,
2526
span: Span,
2627
path: Vec<Ident>
28+
parent_scope: &ParentScope<'b>,
2729
) -> Option<Vec<Ident>> {
2830
debug!("make_path_suggestion: span={:?} path={:?}", span, path);
2931
// If we don't have a path to suggest changes to, then return.
@@ -40,10 +42,12 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
4042
(Some(fst), Some(snd)) if !is_special(*fst) && !is_special(*snd) => {
4143
debug!("make_path_suggestion: fst={:?} snd={:?}", fst, snd);
4244

43-
self.make_missing_self_suggestion(span, path.clone())
44-
.or_else(|| self.make_missing_crate_suggestion(span, path.clone()))
45-
.or_else(|| self.make_missing_super_suggestion(span, path.clone()))
46-
.or_else(|| self.make_external_crate_suggestion(span, path.clone()))
45+
self.make_missing_self_suggestion(span, path.clone(), parent_scope)
46+
.or_else(|| self.make_missing_crate_suggestion(span, path.clone(),
47+
parent_scope))
48+
.or_else(|| self.make_missing_super_suggestion(span, path.clone(),
49+
parent_scope))
50+
.or_else(|| self.make_external_crate_suggestion(span, path, parent_scope))
4751
},
4852
_ => None,
4953
}
@@ -60,10 +64,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
6064
&mut self,
6165
span: Span,
6266
mut path: Vec<Ident>
67+
parent_scope: &ParentScope<'b>,
6368
) -> Option<Vec<Ident>> {
6469
// Replace first ident with `self` and check if that is valid.
6570
path[0].name = keywords::SelfValue.name();
66-
let result = self.resolve_path(None, &path, None, false, span, CrateLint::No);
71+
let result = self.resolve_path(None, &path, None, parent_scope, false, span, CrateLint::No);
6772
debug!("make_missing_self_suggestion: path={:?} result={:?}", path, result);
6873
if let PathResult::Module(..) = result {
6974
Some(path)
@@ -83,10 +88,11 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
8388
&mut self,
8489
span: Span,
8590
mut path: Vec<Ident>
91+
parent_scope: &ParentScope<'b>,
8692
) -> Option<Vec<Ident>> {
8793
// Replace first ident with `crate` and check if that is valid.
8894
path[0].name = keywords::Crate.name();
89-
let result = self.resolve_path(None, &path, None, false, span, CrateLint::No);
95+
let result = self.resolve_path(None, &path, None, parent_scope, false, span, CrateLint::No);
9096
debug!("make_missing_crate_suggestion: path={:?} result={:?}", path, result);
9197
if let PathResult::Module(..) = result {
9298
Some(path)
@@ -106,10 +112,12 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
106112
&mut self,
107113
span: Span,
108114
mut path: Vec<Ident>
115+
parent_scope: &ParentScope<'b>,
109116
) -> Option<Vec<Ident>> {
110117
// Replace first ident with `crate` and check if that is valid.
111118
path[0].name = keywords::Super.name();
112119
let result = self.resolve_path(None, &path, None, false, span, CrateLint::No);
120+
let result = self.resolve_path(None, &path, None, parent_scope, false, span, CrateLint::No);
113121
debug!("make_missing_super_suggestion: path={:?} result={:?}", path, result);
114122
if let PathResult::Module(..) = result {
115123
Some(path)
@@ -132,6 +140,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
132140
&mut self,
133141
span: Span,
134142
mut path: Vec<Ident>
143+
parent_scope: &ParentScope<'b>,
135144
) -> Option<Vec<Ident>> {
136145
// Need to clone else we can't call `resolve_path` without a borrow error. We also store
137146
// into a `BTreeMap` so we can get consistent ordering (and therefore the same diagnostic)
@@ -149,7 +158,8 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
149158
// Replace the first after root (a placeholder we inserted) with a crate name
150159
// and check if that is valid.
151160
path[1].name = *name;
152-
let result = self.resolve_path(None, &path, None, false, span, CrateLint::No);
161+
let result =
162+
self.resolve_path(None, &path, None, parent_scope, false, span, CrateLint::No);
153163
debug!("make_external_crate_suggestion: name={:?} path={:?} result={:?}",
154164
name, path, result);
155165
if let PathResult::Module(..) = result {

src/librustc_resolve/lib.rs

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,7 @@ pub struct ModuleData<'a> {
10171017
resolutions: RefCell<FxHashMap<(Ident, Namespace), &'a RefCell<NameResolution<'a>>>>,
10181018
legacy_macro_resolutions: RefCell<Vec<(Ident, MacroKind, ParentScope<'a>,
10191019
Option<&'a NameBinding<'a>>)>>,
1020-
macro_resolutions: RefCell<Vec<(Box<[Ident]>, Span)>>,
1020+
macro_resolutions: RefCell<Vec<(Vec<Segment>, ParentScope<'a>, Span)>>,
10211021
builtin_attrs: RefCell<Vec<(Ident, ParentScope<'a>)>>,
10221022

10231023
// Macro invocations that can expand into items in this module.
@@ -1628,7 +1628,8 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
16281628
let hir::Path { ref segments, span, ref mut def } = *path;
16291629
let path: Vec<_> = segments.iter().map(|seg| seg.ident).collect();
16301630
// FIXME (Manishearth): Intra doc links won't get warned of epoch changes
1631-
match self.resolve_path(None, &path, Some(namespace), true, span, CrateLint::No) {
1631+
match self.resolve_path_without_parent_scope(None, &path, Some(namespace), true, span,
1632+
CrateLint::No) {
16321633
PathResult::Module(ModuleOrUniformRoot::Module(module)) =>
16331634
*def = module.def().unwrap(),
16341635
PathResult::NonModule(path_res) if path_res.unresolved_segments() == 0 =>
@@ -2472,10 +2473,10 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
24722473
new_id = Some(def.def_id());
24732474
let span = trait_ref.path.span;
24742475
if let PathResult::Module(ModuleOrUniformRoot::Module(module)) =
2475-
self.resolve_path(
2476+
self.resolve_path_without_parent_scope(
24762477
None,
24772478
&path,
2478-
None,
2479+
Some(TypeNS),
24792480
false,
24802481
span,
24812482
CrateLint::SimplePath(trait_ref.ref_id),
@@ -2993,8 +2994,9 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
29932994
(String::new(), "the crate root".to_string())
29942995
} else {
29952996
let mod_path = &path[..path.len() - 1];
2996-
let mod_prefix = match this.resolve_path(None, mod_path, Some(TypeNS),
2997-
false, span, CrateLint::No) {
2997+
let mod_prefix = match this.resolve_path_without_parent_scope(
2998+
None, mod_path, Some(TypeNS), false, span, CrateLint::No
2999+
) {
29983000
PathResult::Module(ModuleOrUniformRoot::Module(module)) =>
29993001
module.def(),
30003002
_ => None,
@@ -3478,7 +3480,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
34783480
));
34793481
}
34803482

3481-
let result = match self.resolve_path(
3483+
let result = match self.resolve_path_without_parent_scope(
34823484
None,
34833485
&path,
34843486
Some(ns),
@@ -3525,7 +3527,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
35253527
path[0].name != keywords::CrateRoot.name() &&
35263528
path[0].name != keywords::DollarCrate.name() {
35273529
let unqualified_result = {
3528-
match self.resolve_path(
3530+
match self.resolve_path_without_parent_scope(
35293531
None,
35303532
&[*path.last().unwrap()],
35313533
Some(ns),
@@ -3548,7 +3550,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
35483550
Some(result)
35493551
}
35503552

3551-
fn resolve_path(
3553+
fn resolve_path_without_parent_scope(
35523554
&mut self,
35533555
base_module: Option<ModuleOrUniformRoot<'a>>,
35543556
path: &[Ident],
@@ -3557,12 +3559,15 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
35573559
path_span: Span,
35583560
crate_lint: CrateLint,
35593561
) -> PathResult<'a> {
3562+
// Macro and import paths must have full parent scope available during resolution,
3563+
// other paths will do okay with parent module alone.
3564+
assert!(opt_ns != None && opt_ns != Some(MacroNS));
35603565
let parent_scope = ParentScope { module: self.current_module, ..self.dummy_parent_scope() };
3561-
self.resolve_path_with_parent_scope(base_module, path, opt_ns, &parent_scope,
3562-
record_used, path_span, crate_lint)
3566+
self.resolve_path(base_module, path, opt_ns, &parent_scope,
3567+
record_used, path_span, crate_lint)
35633568
}
35643569

3565-
fn resolve_path_with_parent_scope(
3570+
fn resolve_path(
35663571
&mut self,
35673572
base_module: Option<ModuleOrUniformRoot<'a>>,
35683573
path: &[Ident],
@@ -3749,7 +3754,6 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
37493754
PathResult::Module(module.unwrap_or_else(|| {
37503755
span_bug!(path_span, "resolve_path: empty(?) path {:?} has no module", path);
37513756
}))
3752-
37533757
}
37543758

37553759
fn lint_if_path_starts_with_module(
@@ -4033,8 +4037,9 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
40334037
} else {
40344038
// Search in module.
40354039
let mod_path = &path[..path.len() - 1];
4036-
if let PathResult::Module(module) = self.resolve_path(None, mod_path, Some(TypeNS),
4037-
false, span, CrateLint::No) {
4040+
if let PathResult::Module(module) = self.resolve_path_without_parent_scope(
4041+
None, mod_path, Some(TypeNS), false, span, CrateLint::No
4042+
) {
40384043
if let ModuleOrUniformRoot::Module(module) = module {
40394044
add_module_candidates(module, &mut names);
40404045
}

src/librustc_resolve/macros.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
462462
parent_scope: &ParentScope<'a>,
463463
force: bool,
464464
) -> Result<Def, Determinacy> {
465-
let ast::Path { ref segments, span } = *path;
465+
let path_span = path.span;
466466
let mut path: Vec<_> = segments.iter().map(|seg| seg.ident).collect();
467467

468468
// Possibly apply the macro helper hack
@@ -473,15 +473,15 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
473473
}
474474

475475
if path.len() > 1 {
476-
let def = match self.resolve_path_with_parent_scope(None, &path, Some(MacroNS),
477-
parent_scope, false, span,
478-
CrateLint::No) {
476+
let def = match self.resolve_path(None, &path, Some(MacroNS), parent_scope,
477+
false, path_span, CrateLint::No) {
479478
PathResult::NonModule(path_res) => match path_res.base_def() {
480479
Def::Err => Err(Determinacy::Determined),
481480
def @ _ => {
482481
if path_res.unresolved_segments() > 0 {
483482
self.found_unresolved_macro = true;
484-
self.session.span_err(span, "fail to resolve non-ident macro path");
483+
self.session.span_err(path_span,
484+
"fail to resolve non-ident macro path");
485485
Err(Determinacy::Determined)
486486
} else {
487487
Ok(def)
@@ -497,12 +497,12 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
497497
};
498498

499499
parent_scope.module.macro_resolutions.borrow_mut()
500-
.push((path.into_boxed_slice(), span));
500+
.push((path.into_boxed_slice(), parent_scope.clone(), span));
501501

502502
def
503503
} else {
504504
let binding = self.early_resolve_ident_in_lexical_scope(
505-
path[0], MacroNS, Some(kind), parent_scope, false, force, span
505+
path[0], MacroNS, Some(kind), parent_scope, false, force, path_span
506506
);
507507
match binding {
508508
Ok(..) => {}
@@ -846,8 +846,12 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
846846

847847
pub fn finalize_current_module_macro_resolutions(&mut self) {
848848
let module = self.current_module;
849-
for &(ref path, span) in module.macro_resolutions.borrow().iter() {
850-
match self.resolve_path(None, &path, Some(MacroNS), true, span, CrateLint::No) {
849+
850+
let macro_resolutions =
851+
mem::replace(&mut *module.macro_resolutions.borrow_mut(), Vec::new());
852+
for (mut path, parent_scope, path_span) in macro_resolutions {
853+
match self.resolve_path(None, &path, Some(MacroNS), &parent_scope,
854+
true, path_span, CrateLint::No) {
851855
PathResult::NonModule(_) => {},
852856
PathResult::Failed(span, msg, _) => {
853857
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));

src/librustc_resolve/resolve_imports.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -875,6 +875,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
875875
}),
876876
&directive.module_path[..],
877877
None,
878+
&directive.parent_scope,
878879
false,
879880
directive.span,
880881
directive.crate_lint(),
@@ -954,6 +955,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
954955
}),
955956
&module_path,
956957
None,
958+
&directive.parent_scope,
957959
true,
958960
span,
959961
directive.crate_lint(),
@@ -966,7 +968,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
966968
}
967969
PathResult::Failed(span, msg, true) => {
968970
return if let Some(suggested_path) = self.make_path_suggestion(
969-
span, module_path.clone()
971+
span, module_path.clone(), &directive.parent_scope
970972
) {
971973
Some((
972974
span,

0 commit comments

Comments
 (0)