Skip to content

Commit 3ac4076

Browse files
committed
Auto merge of #32097 - jseyfried:fix_resolution_regression, r=nikomatsakis
Fix a regression in import resolution This fixes #32089 (caused by #31726) by deducing that name resolution has failed (as opposed to being determinate) in more cases. r? @nikomatsakis
2 parents bcda58f + 4dc4cae commit 3ac4076

File tree

3 files changed

+65
-21
lines changed

3 files changed

+65
-21
lines changed

src/librustc_resolve/build_reduced_graph.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -667,8 +667,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
667667

668668
match subclass {
669669
SingleImport { target, .. } => {
670-
module_.increment_outstanding_references_for(target, ValueNS);
671-
module_.increment_outstanding_references_for(target, TypeNS);
670+
module_.increment_outstanding_references_for(target, ValueNS, is_public);
671+
module_.increment_outstanding_references_for(target, TypeNS, is_public);
672672
}
673673
GlobImport => {
674674
// Set the glob flag. This tells us that we don't know the

src/librustc_resolve/resolve_imports.rs

+40-19
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,10 @@ impl ImportDirective {
124124
#[derive(Clone, Default)]
125125
/// Records information about the resolution of a name in a module.
126126
pub struct NameResolution<'a> {
127-
/// The number of unresolved single imports that could define the name.
128-
outstanding_references: usize,
127+
/// The number of unresolved single imports of any visibility that could define the name.
128+
outstanding_references: u32,
129+
/// The number of unresolved `pub` single imports that could define the name.
130+
pub_outstanding_references: u32,
129131
/// The least shadowable known binding for this name, or None if there are no known bindings.
130132
pub binding: Option<&'a NameBinding<'a>>,
131133
duplicate_globs: Vec<&'a NameBinding<'a>>,
@@ -151,9 +153,12 @@ impl<'a> NameResolution<'a> {
151153
}
152154

153155
// Returns the resolution of the name assuming no more globs will define it.
154-
fn result(&self) -> ResolveResult<&'a NameBinding<'a>> {
156+
fn result(&self, allow_private_imports: bool) -> ResolveResult<&'a NameBinding<'a>> {
155157
match self.binding {
156158
Some(binding) if !binding.defined_with(DefModifiers::GLOB_IMPORTED) => Success(binding),
159+
// If we don't allow private imports and no public imports can define the name, fail.
160+
_ if !allow_private_imports && self.pub_outstanding_references == 0 &&
161+
!self.binding.map(NameBinding::is_public).unwrap_or(false) => Failed(None),
157162
_ if self.outstanding_references > 0 => Indeterminate,
158163
Some(binding) => Success(binding),
159164
None => Failed(None),
@@ -162,8 +167,9 @@ impl<'a> NameResolution<'a> {
162167

163168
// Returns Some(the resolution of the name), or None if the resolution depends
164169
// on whether more globs can define the name.
165-
fn try_result(&self) -> Option<ResolveResult<&'a NameBinding<'a>>> {
166-
match self.result() {
170+
fn try_result(&self, allow_private_imports: bool)
171+
-> Option<ResolveResult<&'a NameBinding<'a>>> {
172+
match self.result(allow_private_imports) {
167173
Success(binding) if binding.defined_with(DefModifiers::PRELUDE) => None,
168174
Failed(_) => None,
169175
result @ _ => Some(result),
@@ -200,7 +206,7 @@ impl<'a> ::ModuleS<'a> {
200206
};
201207

202208
let resolution = resolutions.get(&(name, ns)).cloned().unwrap_or_default();
203-
if let Some(result) = resolution.try_result() {
209+
if let Some(result) = resolution.try_result(allow_private_imports) {
204210
// If the resolution doesn't depend on glob definability, check privacy and return.
205211
return result.and_then(|binding| {
206212
let allowed = allow_private_imports || !binding.is_import() || binding.is_public();
@@ -234,7 +240,7 @@ impl<'a> ::ModuleS<'a> {
234240
}
235241
}
236242

237-
resolution.result()
243+
resolution.result(true)
238244
}
239245

240246
// Define the name or return the existing binding if there is a collision.
@@ -246,15 +252,26 @@ impl<'a> ::ModuleS<'a> {
246252
})
247253
}
248254

249-
pub fn increment_outstanding_references_for(&self, name: Name, ns: Namespace) {
255+
pub fn increment_outstanding_references_for(&self, name: Name, ns: Namespace, is_public: bool) {
250256
let mut resolutions = self.resolutions.borrow_mut();
251-
resolutions.entry((name, ns)).or_insert_with(Default::default).outstanding_references += 1;
257+
let resolution = resolutions.entry((name, ns)).or_insert_with(Default::default);
258+
resolution.outstanding_references += 1;
259+
if is_public {
260+
resolution.pub_outstanding_references += 1;
261+
}
252262
}
253263

254-
fn decrement_outstanding_references_for(&self, name: Name, ns: Namespace) {
255-
self.update_resolution(name, ns, |resolution| match resolution.outstanding_references {
256-
0 => panic!("No more outstanding references!"),
257-
ref mut outstanding_references => *outstanding_references -= 1,
264+
fn decrement_outstanding_references_for(&self, name: Name, ns: Namespace, is_public: bool) {
265+
let decrement_references = |count: &mut _| {
266+
assert!(*count > 0);
267+
*count -= 1;
268+
};
269+
270+
self.update_resolution(name, ns, |resolution| {
271+
decrement_references(&mut resolution.outstanding_references);
272+
if is_public {
273+
decrement_references(&mut resolution.pub_outstanding_references);
274+
}
258275
})
259276
}
260277

@@ -265,11 +282,11 @@ impl<'a> ::ModuleS<'a> {
265282
{
266283
let mut resolutions = self.resolutions.borrow_mut();
267284
let resolution = resolutions.entry((name, ns)).or_insert_with(Default::default);
268-
let was_success = resolution.try_result().and_then(ResolveResult::success).is_some();
285+
let was_success = resolution.try_result(false).and_then(ResolveResult::success).is_some();
269286

270287
let t = update(resolution);
271288
if !was_success {
272-
if let Some(Success(binding)) = resolution.try_result() {
289+
if let Some(Success(binding)) = resolution.try_result(false) {
273290
self.define_in_glob_importers(name, ns, binding);
274291
}
275292
}
@@ -460,10 +477,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
460477
let mut resolve_in_ns = |ns, determined: bool| {
461478
// Temporarily count the directive as determined so that the resolution fails
462479
// (as opposed to being indeterminate) when it can only be defined by the directive.
463-
if !determined { module_.decrement_outstanding_references_for(target, ns) }
480+
if !determined {
481+
module_.decrement_outstanding_references_for(target, ns, directive.is_public)
482+
}
464483
let result =
465484
self.resolver.resolve_name_in_module(target_module, source, ns, false, true);
466-
if !determined { module_.increment_outstanding_references_for(target, ns) }
485+
if !determined {
486+
module_.increment_outstanding_references_for(target, ns, directive.is_public)
487+
}
467488
result
468489
};
469490
(resolve_in_ns(ValueNS, value_determined.get()),
@@ -494,7 +515,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
494515
self.report_conflict(target, ns, &directive.import(binding, None), old_binding);
495516
}
496517
}
497-
module_.decrement_outstanding_references_for(target, ns);
518+
module_.decrement_outstanding_references_for(target, ns, directive.is_public);
498519
}
499520

500521
match (&value_result, &type_result) {
@@ -601,7 +622,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
601622
}
602623

603624
for (&(name, ns), resolution) in target_module.resolutions.borrow().iter() {
604-
if let Some(Success(binding)) = resolution.try_result() {
625+
if let Some(Success(binding)) = resolution.try_result(false) {
605626
if binding.defined_with(DefModifiers::IMPORTABLE | DefModifiers::PUBLIC) {
606627
let _ = module_.try_define_child(name, ns, directive.import(binding, None));
607628
}

src/test/compile-fail/issue-32089.rs

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(rustc_attrs)]
12+
#![allow(unused_imports)]
13+
14+
pub type Type = i32;
15+
16+
mod one { use super::Type; }
17+
pub use self::one::*;
18+
19+
mod two { use super::Type; }
20+
pub use self::two::*;
21+
22+
#[rustc_error]
23+
fn main() {} //~ ERROR compilation successful

0 commit comments

Comments
 (0)